Skip to content

Commit

Permalink
Merge pull request #129 from openbase/bugfix/128-fix-permission-and-m…
Browse files Browse the repository at this point in the history
…odification-issues-with-the-message-registry

Fix permission and update handling of the message registry.
  • Loading branch information
lhuxohl authored Feb 27, 2024
2 parents 656f260 + 9a0f523 commit cce6d35
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.openbase.jul.storage.registry.RegistryRemote;
import org.openbase.type.domotic.activity.ActivityConfigType.ActivityConfig;
import org.openbase.type.domotic.activity.ActivityTemplateType.ActivityTemplate;
import org.openbase.type.domotic.communication.UserMessageType.UserMessage;
import org.openbase.type.domotic.service.ServiceTemplateType.ServiceTemplate;
import org.openbase.type.domotic.unit.UnitConfigType.UnitConfig;
import org.openbase.type.domotic.unit.UnitTemplateType.UnitTemplate;
Expand Down Expand Up @@ -83,7 +84,8 @@ public class Registries {
UnitTemplate.getDefaultInstance(),
ServiceTemplate.getDefaultInstance(),
ActivityTemplate.getDefaultInstance(),
ActivityConfig.getDefaultInstance()
ActivityConfig.getDefaultInstance(),
UserMessage.getDefaultInstance()
};

/**
Expand All @@ -102,6 +104,7 @@ public static List<RegistryRemote<?>> getRegistries(final boolean waitForData) t
registryList.add(getClassRegistry(waitForData));
registryList.add(getActivityRegistry(waitForData));
registryList.add(getUnitRegistry(waitForData));
registryList.add(getMessageRegistry(waitForData));
return registryList;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,56 +131,60 @@ object AuthorizationWithTokenHelper {
userMessage: UserMessageType.UserMessage,
permissionType: AuthorizationHelper.PermissionType,
unitRegistry: UnitRegistry,
): AuthPair = try {
// validate sender
canDo(
authenticationBaseData,
unitRegistry.getUnitConfigById(userMessage.senderId),
permissionType,
unitRegistry,
null,
null
)
} catch (ex: CouldNotPerformException) {
): AuthPair {
var exceptionStack: ExceptionStack? = null
exceptionStack = MultiException.push(AuthorizationWithTokenHelper::class.java, ex, exceptionStack)

// validate receiver if sender validation failed.
try {
return try {
// validate sender
canDo(
authenticationBaseData,
unitRegistry.getUnitConfigById(userMessage.recipientId),
tryOrNull { unitRegistry.getUnitConfigById(userMessage.senderId) },
permissionType,
unitRegistry,
null,
null
)
} catch (exx: CouldNotPerformException) {
exceptionStack = MultiException.push(AuthorizationWithTokenHelper::class.java, exx, exceptionStack)
} catch (ex: CouldNotPerformException) {
exceptionStack = MultiException.push(AuthorizationWithTokenHelper::class.java, ex, exceptionStack)

userMessage.conditionList.firstNotNullOfOrNull { condition ->
try {
canDo(
authenticationBaseData,
unitRegistry.getUnitConfigById(condition.unitId),
permissionType,
unitRegistry,
null,
null
)
} catch (exxx: CouldNotPerformException) {
exceptionStack = MultiException.push(AuthorizationWithTokenHelper::class.java, exxx, exceptionStack)
null
} ?: let {
MultiException.checkAndThrow({ "Permission denied!" }, exceptionStack)
// validate receiver if sender validation failed.
try {
canDo(
authenticationBaseData,
tryOrNull { unitRegistry.getUnitConfigById(userMessage.recipientId) },
permissionType,
unitRegistry,
null,
null
)
} catch (exx: CouldNotPerformException) {
exceptionStack = MultiException.push(AuthorizationWithTokenHelper::class.java, exx, exceptionStack)

userMessage.conditionList.firstNotNullOfOrNull { condition ->
try {
canDo(
authenticationBaseData,
unitRegistry.getUnitConfigById(condition.unitId),
permissionType,
unitRegistry,
null,
null
)
} catch (exxx: CouldNotPerformException) {
exceptionStack =
MultiException.push(AuthorizationWithTokenHelper::class.java, exxx, exceptionStack)
null
}
}
}
} ?: run {

MultiException.checkAndThrow({ "Permission denied!" }, exceptionStack)

throw FatalImplementationErrorException(
"ExceptionStack empty in error case.", AuthorizationWithTokenHelper::class.java
)
}
} ?: throw FatalImplementationErrorException(
"ExceptionStack empty in error case.",
AuthorizationWithTokenHelper::class.java
)
}

/**
* Perform a permission check for authentication data including tokens.
Expand All @@ -204,7 +208,7 @@ object AuthorizationWithTokenHelper {
@Throws(CouldNotPerformException::class)
fun canDo(

Check warning

Code scanning / detekt

The more parameters a function has the more complex it is. Long parameter lists are often used to control complex algorithms and violate the Single Responsibility Principle. Prefer functions with short parameter lists. Warning

The function canDo(authenticationBaseData: AuthenticationBaseData?, unitConfig: UnitConfigType.UnitConfig?, permissionType: AuthorizationHelper.PermissionType, unitRegistry: UnitRegistry, unitType: UnitTemplateType.UnitTemplate.UnitType?, serviceType: ServiceTemplateType.ServiceTemplate.ServiceType?) has too many parameters. The current threshold is set to 6.

Check warning

Code scanning / detekt

One method should have one responsibility. Long methods tend to handle many things at once. Prefer smaller methods to make them easier to understand. Warning

The function canDo is too long (79). The maximum length is 60.

Check warning

Code scanning / detekt

Restrict the number of return statements in methods. Warning

Function canDo has 5 return statements which exceeds the limit of 2.
authenticationBaseData: AuthenticationBaseData?,
unitConfig: UnitConfigType.UnitConfig,
unitConfig: UnitConfigType.UnitConfig?,
permissionType: AuthorizationHelper.PermissionType,
unitRegistry: UnitRegistry,
unitType: UnitTemplateType.UnitTemplate.UnitType? = null,
Expand All @@ -227,27 +231,6 @@ object AuthorizationWithTokenHelper {
}
}

// check if authenticated user has needed permissions
if (AuthorizationHelper.canDo(
unitConfig,
userClientPair.getUserId(),
unitRegistry.getAuthorizationGroupMap(),
unitRegistry.getLocationMap(),
permissionType
)
) {
return AuthPair(userClientPair, userClientPair.getUserId())
}
if (AuthorizationHelper.canDo(
unitConfig,
userClientPair.getClientId(),
unitRegistry.getAuthorizationGroupMap(),
unitRegistry.getLocationMap(),
permissionType
)
) {
return AuthPair(userClientPair, userClientPair.getClientId())
}
try {
// test if user is part of the admin group
val memberIdList =
Expand All @@ -262,25 +245,53 @@ object AuthorizationWithTokenHelper {
// continue with the checks, admin group is not available
}

// authenticated user does not have permissions so check if the authorization token grants them
if (authenticationBaseData != null && authenticationBaseData.authorizationToken != null) {
val authorizationToken = authenticationBaseData.authorizationToken
// verify that the authorization token is valid
verifyAuthorizationToken(authorizationToken, unitRegistry)
if (unitConfig != null) {

// verify if the token grants the necessary permissions
return authorizedByToken(
authorizationToken,
userClientPair,
unitConfig,
permissionType,
unitRegistry,
unitType,
serviceType
)
// check if authenticated user has needed permissions
if (AuthorizationHelper.canDo(
unitConfig,
userClientPair.getUserId(),
unitRegistry.getAuthorizationGroupMap(),
unitRegistry.getLocationMap(),
permissionType
)
) {
return AuthPair(userClientPair, userClientPair.getUserId())
}

if (AuthorizationHelper.canDo(
unitConfig,
userClientPair.getClientId(),
unitRegistry.getAuthorizationGroupMap(),
unitRegistry.getLocationMap(),
permissionType
)
) {
return AuthPair(userClientPair, userClientPair.getClientId())
}

// authenticated user does not have permissions so check if the authorization token grants them
if (authenticationBaseData != null
&& authenticationBaseData.authorizationToken != null
) {
val authorizationToken = authenticationBaseData.authorizationToken
// verify that the authorization token is valid
verifyAuthorizationToken(authorizationToken, unitRegistry)

// verify if the token grants the necessary permissions
return authorizedByToken(
authorizationToken,
userClientPair,
unitConfig,
permissionType,
unitRegistry,
unitType,
serviceType
)
}
}
var userRepresentation = userClientPair.getUserId()
if (!userRepresentation.isEmpty()) {
if (userRepresentation.isNotEmpty()) {
userRepresentation += "@"
}
userRepresentation += userClientPair.getClientId()
Expand Down

0 comments on commit cce6d35

Please sign in to comment.