Switching to least privilege for undefined resources on method annotations
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 c680f0f..e686cbf 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.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 @@
{
/**
- * @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 b62efe7..f33ec7c 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.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 @@
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 fdff955..36454d7 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.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 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 @@
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 acfe4ed..bb103e3 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 @@
@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 e19a542..1cbff25 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 @@
}
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 3a3980a..9ec0876 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 @@
* wildcards can be used on the resource definition to streamline the assigning of
* permissions for _large_ sets of things.
*
+ * The <code>GLOBAL</code> resource is a catch-all for all resource strings.
+ *
* @author Jesse McConnell
* @author <a href="mailto:joakim@erdfelt.com">Joakim Erdfelt</a>
*
