Switching to least privilege for undefined resources on method annotations

This commit is contained in:
Martin Stockhammer 2020-09-09 12:50:13 +02:00
parent a4b273cf33
commit 7a13ce4f72
6 changed files with 30 additions and 9 deletions

View File

@ -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 ( "" );

View File

@ -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;
}

View File

@ -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" ) );
}
}

View File

@ -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;

View File

@ -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 );

View File

@ -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 <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>
*