diff --git a/redback-authorization/redback-authorization-api/src/main/java/org/apache/archiva/redback/authorization/RedbackAuthorization.java b/redback-authorization/redback-authorization-api/src/main/java/org/apache/archiva/redback/authorization/RedbackAuthorization.java index c680f0fb..e686cbf9 100644 --- a/redback-authorization/redback-authorization-api/src/main/java/org/apache/archiva/redback/authorization/RedbackAuthorization.java +++ b/redback-authorization/redback-authorization-api/src/main/java/org/apache/archiva/redback/authorization/RedbackAuthorization.java @@ -25,6 +25,9 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** + * Authorization annotation. The annotation can be defined for methods and describes + * the permissions necessary to execute the method. + * * @author Olivier Lamy * @since 1.3 */ @@ -34,17 +37,25 @@ public @interface RedbackAuthorization { /** - * @return at least one of those redback roles is needed + * The list of permissions that are needed for executing the method. + * The strings refer to defined permission ids. + * The accessing user must have at least one of the given permissions to execute + * the method. + * @return the array of permission ids. */ String[] permissions() default ( "" ); /** + * The resource is used to restrict access by using information from + * the method parameters or call environment. + * Resource annotations have to be in line with the defined permissions. * @return the redback ressource karma needed */ String resource() default ( "" ); /** - * @return doc + * A description of the authorization definition. + * @return the description string */ String description() default ( "" ); diff --git a/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/main/java/org/apache/archiva/redback/authorization/rbac/evaluator/DefaultPermissionEvaluator.java b/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/main/java/org/apache/archiva/redback/authorization/rbac/evaluator/DefaultPermissionEvaluator.java index b62efe78..f33ec7cf 100644 --- a/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/main/java/org/apache/archiva/redback/authorization/rbac/evaluator/DefaultPermissionEvaluator.java +++ b/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/main/java/org/apache/archiva/redback/authorization/rbac/evaluator/DefaultPermissionEvaluator.java @@ -24,6 +24,7 @@ import org.apache.archiva.redback.users.UserManager; import org.apache.archiva.redback.users.UserManagerException; import org.apache.archiva.redback.users.UserNotFoundException; import org.apache.archiva.redback.rbac.Permission; +import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Service; import javax.inject.Inject; @@ -81,8 +82,10 @@ public class DefaultPermissionEvaluator return true; } - // if we are not checking a specific resource, the operation is enough - if ( resource == null ) + // Resource settings on the permission object and on the annotation + // should be in line. If not, we use the least privilege, which means + // if one of both is set, we will check for equality. + if ( StringUtils.isEmpty( permissionResource ) && resource == null ) { return true; } diff --git a/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/test/java/org/apache/archiva/redback/authorization/rbac/evaluator/PermissionEvaluatorTest.java b/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/test/java/org/apache/archiva/redback/authorization/rbac/evaluator/PermissionEvaluatorTest.java index fdff955c..36454d73 100644 --- a/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/test/java/org/apache/archiva/redback/authorization/rbac/evaluator/PermissionEvaluatorTest.java +++ b/redback-authorization/redback-authorization-providers/redback-authorization-rbac/src/test/java/org/apache/archiva/redback/authorization/rbac/evaluator/PermissionEvaluatorTest.java @@ -26,6 +26,7 @@ import org.apache.archiva.redback.rbac.Resource; import org.apache.archiva.redback.rbac.memory.MemoryOperation; import org.apache.archiva.redback.rbac.memory.MemoryPermission; import org.apache.archiva.redback.rbac.memory.MemoryResource; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.test.context.ContextConfiguration; @@ -46,9 +47,11 @@ public class PermissionEvaluatorTest public void testNullResource() throws PermissionEvaluationException { - // null resources should be considered as matching if any resource is obtained. - // we do this instead of using "global" as that is the inverse - you are allocated global rights, - // which is right to everything. null is the right to anything. + + // if the permission has explicitly a resource set, we should explicitly + // provide resources during access check (e.g. define a resource in the annotations). + // This is for consistency and least privilege permission. So, e.g. if you forget to define the resource + // on a method annotation, it should deny access for non-global roles. Resource resource = new MemoryResource(); resource.setIdentifier( "Resource" ); @@ -61,6 +64,6 @@ public class PermissionEvaluatorTest permission.setOperation( operation ); permission.setResource( resource ); - assertTrue( permissionEvaluator.evaluate( permission, "Operation", null, "brett" ) ); + assertFalse( permissionEvaluator.evaluate( permission, "Operation", null, "brett" ) ); } } diff --git a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/UserService.java b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/UserService.java index acfe4ed7..bb103e3e 100644 --- a/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/UserService.java +++ b/redback-integrations/redback-rest/redback-rest-api/src/main/java/org/apache/archiva/redback/rest/api/services/v2/UserService.java @@ -63,7 +63,8 @@ public interface UserService @Path( "{userId}" ) @GET @Produces( { MediaType.APPLICATION_JSON } ) - @RedbackAuthorization( permissions = RedbackRoleConstants.USER_MANAGEMENT_USER_EDIT_OPERATION ) + @RedbackAuthorization( permissions = RedbackRoleConstants.USER_MANAGEMENT_USER_EDIT_OPERATION, + resource = "{userId}" ) User getUser( @PathParam( "userId" ) String userId ) throws RedbackServiceException; diff --git a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/PermissionsInterceptor.java b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/PermissionsInterceptor.java index e19a5426..1cbff25d 100644 --- a/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/PermissionsInterceptor.java +++ b/redback-integrations/redback-rest/redback-rest-services/src/main/java/org/apache/archiva/redback/rest/services/interceptors/PermissionsInterceptor.java @@ -162,6 +162,7 @@ public class PermissionsInterceptor } else { + // The noPermission is only valid, if the user is authenticated if ( redbackAuthorization.noPermission() ) { AuthenticationResult authenticationResult = getAuthenticationResult( containerRequestContext, httpAuthenticator, request ); diff --git a/redback-rbac/redback-rbac-model/src/main/java/org/apache/archiva/redback/rbac/Resource.java b/redback-rbac/redback-rbac-model/src/main/java/org/apache/archiva/redback/rbac/Resource.java index 3a3980a9..9ec08767 100644 --- a/redback-rbac/redback-rbac-model/src/main/java/org/apache/archiva/redback/rbac/Resource.java +++ b/redback-rbac/redback-rbac-model/src/main/java/org/apache/archiva/redback/rbac/Resource.java @@ -31,6 +31,8 @@ package org.apache.archiva.redback.rbac; * wildcards can be used on the resource definition to streamline the assigning of * permissions for _large_ sets of things. * + * The GLOBAL resource is a catch-all for all resource strings. + * * @author Jesse McConnell * @author Joakim Erdfelt *