From f77d21b58956342acf102b71ab166eb1995e4503 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Fri, 6 Feb 2015 23:09:37 +1100 Subject: [PATCH] fix unit test --- .../interceptors/PermissionsInterceptor.java | 37 +++++++++++----- .../rest/services/UserServiceTest.java | 19 +++++++-- .../src/test/resources/log4j2-test.xml | 3 +- .../system/DefaultSecuritySession.java | 12 +++++- .../redback/system/DefaultSecuritySystem.java | 42 +++++++++++++------ .../redback/system/SecuritySystem.java | 4 ++ 6 files changed, 90 insertions(+), 27 deletions(-) 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 f2d4284a..fc2a11fa 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 @@ -22,6 +22,7 @@ package org.apache.archiva.redback.rest.services.interceptors; import org.apache.archiva.redback.authentication.AuthenticationException; import org.apache.archiva.redback.authentication.AuthenticationResult; import org.apache.archiva.redback.authorization.AuthorizationException; +import org.apache.archiva.redback.authorization.AuthorizationResult; import org.apache.archiva.redback.authorization.RedbackAuthorization; import org.apache.archiva.redback.integration.filter.authentication.basic.HttpBasicAuthentication; import org.apache.archiva.redback.policy.AccountLockedException; @@ -75,6 +76,7 @@ public class PermissionsInterceptor { if ( redbackAuthorization.noRestriction() ) { + log.debug( "redbackAuthorization.noRestriction() so skip permission check" ); // we are fine this services is marked as non restrictive access return; } @@ -84,48 +86,64 @@ public class PermissionsInterceptor && !( permissions.length == 1 && StringUtils.isEmpty( permissions[0] ) ) ) { HttpServletRequest request = getHttpServletRequest( message ); - SecuritySession securitySession = httpAuthenticator.getSecuritySession( request.getSession( true ) ); + SecuritySession securitySession = httpAuthenticator.getSecuritySession( request.getSession() ); AuthenticationResult authenticationResult = message.get( AuthenticationResult.class ); + log.debug( "authenticationResult from message: {}", authenticationResult ); + if ( authenticationResult == null ) { try { authenticationResult = httpAuthenticator.getAuthenticationResult( request, getHttpServletResponse( message ) ); + + log.debug( "authenticationResult from request: {}", authenticationResult ); } catch ( AuthenticationException e ) { log.debug( "failed to authenticate for path {}", message.get( Message.REQUEST_URI ) ); containerRequestContext.abortWith( Response.status( Response.Status.FORBIDDEN ).build() ); + return; } catch ( AccountLockedException e ) { log.debug( "account locked for path {}", message.get( Message.REQUEST_URI ) ); containerRequestContext.abortWith( Response.status( Response.Status.FORBIDDEN ).build() ); + return; } catch ( MustChangePasswordException e ) { log.debug( "must change password for path {}", message.get( Message.REQUEST_URI ) ); containerRequestContext.abortWith( Response.status( Response.Status.FORBIDDEN ).build() ); + return; } } if ( authenticationResult != null && authenticationResult.isAuthenticated() ) { + message.put( AuthenticationResult.class, authenticationResult ); for ( String permission : permissions ) { + log.debug( "check permission: {} with securitySession {}", permission, securitySession ); if ( StringUtils.isBlank( permission ) ) { continue; } try { - if ( securitySystem.isAuthorized( securitySession, permission, - StringUtils.isBlank( redbackAuthorization.resource() ) - ? null - : redbackAuthorization.resource() ) ) + AuthorizationResult authorizationResult = + securitySystem.authorize( authenticationResult.getUser(), permission, // + StringUtils.isBlank( redbackAuthorization.resource() ) // + ? null : redbackAuthorization.resource() ); + /* + if ( securitySystem.isAuthorized( securitySession, permission, // + StringUtils.isBlank( redbackAuthorization.resource() ) // + ? null : redbackAuthorization.resource() ) ) + */ + if ( authenticationResult != null && authorizationResult.isAuthorized() ) { + log.debug( "isAuthorized for permission {}", permission ); return; } else @@ -140,13 +158,12 @@ public class PermissionsInterceptor } catch ( AuthorizationException e ) { - log.debug( e.getMessage(), e ); - + log.debug( " AuthorizationException " + e.getMessage() // + + " checking permission " + permission, e ); + containerRequestContext.abortWith( Response.status( Response.Status.FORBIDDEN ).build() ); + return; } } - containerRequestContext.abortWith( Response.status( Response.Status.FORBIDDEN ).build() ); - return; - } else { diff --git a/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/UserServiceTest.java b/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/UserServiceTest.java index 629e8fc0..b5a80d27 100644 --- a/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/UserServiceTest.java +++ b/redback-integrations/redback-rest/redback-rest-services/src/test/java/org/apache/archiva/redback/rest/services/UserServiceTest.java @@ -25,6 +25,7 @@ import org.apache.archiva.redback.rest.api.model.Permission; import org.apache.archiva.redback.rest.api.model.ResetPasswordRequest; import org.apache.archiva.redback.rest.api.model.User; import org.apache.archiva.redback.rest.api.model.UserRegistrationRequest; +import org.apache.archiva.redback.rest.api.services.RedbackServiceException; import org.apache.archiva.redback.rest.api.services.UserService; import org.apache.archiva.redback.rest.services.mock.EmailMessage; import org.apache.archiva.redback.rest.services.mock.ServicesAssert; @@ -178,7 +179,7 @@ public class UserServiceTest } finally { - getUserService( authorizationHeader ).deleteUser( "toto" ); + deleteUserQuietly( "toto" ); } } @@ -237,7 +238,7 @@ public class UserServiceTest } finally { - getUserService( authorizationHeader ).deleteUser( "toto" ); + deleteUserQuietly( "toto" ); } } @@ -307,11 +308,23 @@ public class UserServiceTest } finally { - getUserService( authorizationHeader ).deleteUser( "toto" ); + deleteUserQuietly( "toto" ); } } + private void deleteUserQuietly( String userName ) + { + try + { + getUserService( authorizationHeader ).deleteUser( userName ); + } + catch ( Exception e ) + { + log.warn( "ignore fail to delete user " + e.getMessage(), e ); + } + } + @Test public void getAdminPermissions() throws Exception diff --git a/redback-integrations/redback-rest/redback-rest-services/src/test/resources/log4j2-test.xml b/redback-integrations/redback-rest/redback-rest-services/src/test/resources/log4j2-test.xml index eb55d394..e058bdb7 100644 --- a/redback-integrations/redback-rest/redback-rest-services/src/test/resources/log4j2-test.xml +++ b/redback-integrations/redback-rest/redback-rest-services/src/test/resources/log4j2-test.xml @@ -21,7 +21,7 @@ - + @@ -32,6 +32,7 @@ + diff --git a/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySession.java b/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySession.java index 2fc40c2f..9acbec4c 100644 --- a/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySession.java +++ b/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySession.java @@ -19,8 +19,8 @@ package org.apache.archiva.redback.system; * under the License. */ -import org.apache.archiva.redback.users.User; import org.apache.archiva.redback.authentication.AuthenticationResult; +import org.apache.archiva.redback.users.User; import org.springframework.stereotype.Service; import java.io.Serializable; @@ -74,4 +74,14 @@ public class DefaultSecuritySession { return ( ( user != null ) && authenticated ); } + + @Override + public String toString() + { + return "DefaultSecuritySession{" + + "authenticationResult=" + authenticationResult + + ", user=" + user + + ", authenticated=" + authenticated + + '}'; + } } diff --git a/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySystem.java b/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySystem.java index f5fc35c8..c5720890 100644 --- a/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySystem.java +++ b/redback-system/src/main/java/org/apache/archiva/redback/system/DefaultSecuritySystem.java @@ -19,13 +19,6 @@ package org.apache.archiva.redback.system; * under the License. */ -import org.apache.archiva.redback.keys.KeyManager; -import org.apache.archiva.redback.policy.AccountLockedException; -import org.apache.archiva.redback.policy.UserSecurityPolicy; -import org.apache.archiva.redback.users.User; -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.authentication.AuthenticationDataSource; import org.apache.archiva.redback.authentication.AuthenticationException; import org.apache.archiva.redback.authentication.AuthenticationManager; @@ -34,7 +27,14 @@ import org.apache.archiva.redback.authorization.AuthorizationDataSource; import org.apache.archiva.redback.authorization.AuthorizationException; import org.apache.archiva.redback.authorization.AuthorizationResult; import org.apache.archiva.redback.authorization.Authorizer; +import org.apache.archiva.redback.keys.KeyManager; +import org.apache.archiva.redback.policy.AccountLockedException; import org.apache.archiva.redback.policy.MustChangePasswordException; +import org.apache.archiva.redback.policy.UserSecurityPolicy; +import org.apache.archiva.redback.users.User; +import org.apache.archiva.redback.users.UserManager; +import org.apache.archiva.redback.users.UserManagerException; +import org.apache.archiva.redback.users.UserNotFoundException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -47,7 +47,7 @@ import javax.inject.Named; * * @author: Jesse McConnell */ -@Service("securitySystem") +@Service( "securitySystem" ) public class DefaultSecuritySystem implements SecuritySystem { @@ -57,15 +57,15 @@ public class DefaultSecuritySystem private AuthenticationManager authnManager; @Inject - @Named(value = "authorizer#default") + @Named( value = "authorizer#default" ) private Authorizer authorizer; @Inject - @Named(value = "userManager#default") + @Named( value = "userManager#default" ) private UserManager userManager; @Inject - @Named(value = "keyManager#cached") + @Named( value = "keyManager#cached" ) private KeyManager keyManager; @Inject @@ -92,7 +92,6 @@ public class DefaultSecuritySystem * @throws UserNotFoundException * @throws MustChangePasswordException * @throws org.apache.archiva.redback.policy.AccountLockedException - * * @throws MustChangePasswordException */ public SecuritySession authenticate( AuthenticationDataSource source ) @@ -176,6 +175,24 @@ public class DefaultSecuritySystem return authorizer.isAuthorized( source ); } + public AuthorizationResult authorize( User user, String permission, String resource ) + throws AuthorizationException + { + AuthorizationDataSource source = null; + + if ( user != null ) + { + source = new AuthorizationDataSource( user.getUsername(), user, permission, resource ); + } + + if ( source == null ) + { + source = new AuthorizationDataSource( null, null, permission, resource ); + } + + return authorizer.isAuthorized( source ); + } + public boolean isAuthorized( SecuritySession session, String permission ) throws AuthorizationException { @@ -287,4 +304,5 @@ public class DefaultSecuritySystem { return userManager.isReadOnly(); } + } diff --git a/redback-system/src/main/java/org/apache/archiva/redback/system/SecuritySystem.java b/redback-system/src/main/java/org/apache/archiva/redback/system/SecuritySystem.java index 66f775e7..ad751cde 100644 --- a/redback-system/src/main/java/org/apache/archiva/redback/system/SecuritySystem.java +++ b/redback-system/src/main/java/org/apache/archiva/redback/system/SecuritySystem.java @@ -22,6 +22,7 @@ package org.apache.archiva.redback.system; import org.apache.archiva.redback.policy.AccountLockedException; import org.apache.archiva.redback.policy.MustChangePasswordException; import org.apache.archiva.redback.policy.UserSecurityPolicy; +import org.apache.archiva.redback.users.User; import org.apache.archiva.redback.users.UserManagerException; import org.apache.archiva.redback.users.UserNotFoundException; import org.apache.archiva.redback.authentication.AuthenticationDataSource; @@ -73,6 +74,9 @@ public interface SecuritySystem AuthorizationResult authorize( SecuritySession session, String permission, String resource ) throws AuthorizationException; + AuthorizationResult authorize( User user, String permission, String resource ) + throws AuthorizationException; + boolean isAuthorized( SecuritySession session, String permission, String resource ) throws AuthorizationException;