SEC-1522: Treat empty attribute collection the same as null when returned by SecurityMetadataSource. Both are now treated as public invocations.

This commit is contained in:
Luke Taylor 2010-07-27 02:18:18 +01:00
parent a74077f9b1
commit b854e67952
3 changed files with 31 additions and 43 deletions

View File

@ -31,13 +31,12 @@ public interface SecurityMetadataSource extends AopInfrastructureBean {
//~ Methods ======================================================================================================== //~ Methods ========================================================================================================
/** /**
* Accesses the <code>ConfigAttribute</code>s that apply to a given secure object. * Accesses the {@code ConfigAttribute}s that apply to a given secure object.
* <p>
* Returns <code>null</code> if no attributes apply.
* *
* @param object the object being secured * @param object the object being secured
* *
* @return the attributes that apply to the passed in secured object or null if there are no applicable attributes. * @return the attributes that apply to the passed in secured object. Can return either {@code null} or an
* empty collection if there are no applicable attributes.
* *
* @throws IllegalArgumentException if the passed object is not of a type supported by the * @throws IllegalArgumentException if the passed object is not of a type supported by the
* <code>SecurityMetadataSource</code> implementation * <code>SecurityMetadataSource</code> implementation
@ -45,18 +44,18 @@ public interface SecurityMetadataSource extends AopInfrastructureBean {
Collection<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException; Collection<ConfigAttribute> getAttributes(Object object) throws IllegalArgumentException;
/** /**
* If available, returns all of the <code>ConfigAttribute</code>s defined by the implementing class. * If available, returns all of the {@code ConfigAttribute}s defined by the implementing class.
* <p> * <p>
* This is used by the {@link AbstractSecurityInterceptor} to perform startup time validation of each * This is used by the {@link AbstractSecurityInterceptor} to perform startup time validation of each
* <code>ConfigAttribute</code> configured against it. * {@code ConfigAttribute} configured against it.
* *
* @return the <code>ConfigAttribute</code>s or <code>null</code> if unsupported * @return the {@code ConfigAttribute}s or {@code null} if unsupported
*/ */
Collection<ConfigAttribute> getAllConfigAttributes(); Collection<ConfigAttribute> getAllConfigAttributes();
/** /**
* Indicates whether the <code>SecurityMetadataSource</code> implementation is able to provide * Indicates whether the {@code SecurityMetadataSource} implementation is able to provide
* <code>ConfigAttribute</code>s for the indicated secure object type. * {@code ConfigAttribute}s for the indicated secure object type.
* *
* @param clazz the class that is being queried * @param clazz the class that is being queried
* *

View File

@ -170,7 +170,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
Collection<ConfigAttribute> attributes = this.obtainSecurityMetadataSource().getAttributes(object); Collection<ConfigAttribute> attributes = this.obtainSecurityMetadataSource().getAttributes(object);
if (attributes == null) { if (attributes == null || attributes.isEmpty()) {
if (rejectPublicInvocations) { if (rejectPublicInvocations) {
throw new IllegalArgumentException("Secure object invocation " + object + throw new IllegalArgumentException("Secure object invocation " + object +
" was denied as public invocations are not allowed via this interceptor. " " was denied as public invocations are not allowed via this interceptor. "
@ -203,8 +203,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
this.accessDecisionManager.decide(authenticated, object, attributes); this.accessDecisionManager.decide(authenticated, object, attributes);
} }
catch (AccessDeniedException accessDeniedException) { catch (AccessDeniedException accessDeniedException) {
publishEvent(new AuthorizationFailureEvent(object, attributes, authenticated, publishEvent(new AuthorizationFailureEvent(object, attributes, authenticated, accessDeniedException));
accessDeniedException));
throw accessDeniedException; throw accessDeniedException;
} }
@ -266,8 +265,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
token.getAttributes(), returnedObject); token.getAttributes(), returnedObject);
} }
catch (AccessDeniedException accessDeniedException) { catch (AccessDeniedException accessDeniedException) {
AuthorizationFailureEvent event = new AuthorizationFailureEvent(token.getSecureObject(), token AuthorizationFailureEvent event = new AuthorizationFailureEvent(token.getSecureObject(),
.getAttributes(), token.getAuthentication(), accessDeniedException); token.getAttributes(), token.getAuthentication(), accessDeniedException);
publishEvent(event); publishEvent(event);
throw accessDeniedException; throw accessDeniedException;
@ -310,7 +309,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
/** /**
* Helper method which generates an exception containing the passed reason, * Helper method which generates an exception containing the passed reason,
* and publishes an event to the application context. * and publishes an event to the application context.
* <p/> * <p>
* Always throws an exception. * Always throws an exception.
* *
* @param reason to be provided in the exception detail * @param reason to be provided in the exception detail
@ -346,12 +345,12 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
/** /**
* Indicates the type of secure objects the subclass will be presenting to * Indicates the type of secure objects the subclass will be presenting to
* the abstract parent for processing. This is used to ensure collaborators * the abstract parent for processing. This is used to ensure collaborators
* wired to the <code>AbstractSecurityInterceptor</code> all support the * wired to the {@code AbstractSecurityInterceptor} all support the
* indicated secure object class. * indicated secure object class.
* *
* @return the type of secure object the subclass provides services for * @return the type of secure object the subclass provides services for
*/ */
public abstract Class<? extends Object> getSecureObjectClass(); public abstract Class<?> getSecureObjectClass();
public boolean isAlwaysReauthenticate() { public boolean isAlwaysReauthenticate() {
return alwaysReauthenticate; return alwaysReauthenticate;

View File

@ -15,54 +15,44 @@
package org.springframework.security.access.intercept; package org.springframework.security.access.intercept;
import org.jmock.Expectations; import static org.mockito.Mockito.mock;
import org.jmock.Mockery;
import org.jmock.integration.junit4.JUnit4Mockery;
import org.junit.Test; import org.junit.Test;
import org.springframework.security.access.AccessDecisionManager; import org.springframework.security.access.AccessDecisionManager;
import org.springframework.security.access.SecurityMetadataSource; import org.springframework.security.access.SecurityMetadataSource;
import org.springframework.security.access.intercept.AbstractSecurityInterceptor;
import org.springframework.security.access.intercept.RunAsManager;
import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.util.SimpleMethodInvocation; import org.springframework.security.util.SimpleMethodInvocation;
/** /**
* Tests some {@link AbstractSecurityInterceptor} methods. Most of the testing for this class is found in the * Tests some {@link AbstractSecurityInterceptor} methods. Most of the testing for this class is found in the
* <code>MethodSecurityInterceptorTests</code> class. * {@code MethodSecurityInterceptorTests} class.
* *
* @author Ben Alex * @author Ben Alex
*/ */
public class AbstractSecurityInterceptorTests { public class AbstractSecurityInterceptorTests {
private Mockery jmock = new JUnit4Mockery();
//~ Methods ======================================================================================================== //~ Methods ========================================================================================================
@Test(expected=IllegalArgumentException.class) @Test(expected=IllegalArgumentException.class)
public void detectsIfInvocationPassedIncompatibleSecureObject() throws Exception { public void detectsIfInvocationPassedIncompatibleSecureObject() throws Exception {
MockSecurityInterceptorWhichOnlySupportsStrings si = new MockSecurityInterceptorWhichOnlySupportsStrings(); MockSecurityInterceptorWhichOnlySupportsStrings si = new MockSecurityInterceptorWhichOnlySupportsStrings();
si.setRunAsManager(jmock.mock(RunAsManager.class)); si.setRunAsManager(mock(RunAsManager.class));
si.setAuthenticationManager(jmock.mock(AuthenticationManager.class)); si.setAuthenticationManager(mock(AuthenticationManager.class));
si.setAfterInvocationManager(jmock.mock(AfterInvocationManager.class)); si.setAfterInvocationManager(mock(AfterInvocationManager.class));
si.setAccessDecisionManager(jmock.mock(AccessDecisionManager.class)); si.setAccessDecisionManager(mock(AccessDecisionManager.class));
si.setSecurityMetadataSource(jmock.mock(SecurityMetadataSource.class)); si.setSecurityMetadataSource(mock(SecurityMetadataSource.class));
jmock.checking(new Expectations() {{ ignoring(anything()); }});
si.beforeInvocation(new SimpleMethodInvocation()); si.beforeInvocation(new SimpleMethodInvocation());
} }
@Test(expected=IllegalArgumentException.class) @Test(expected=IllegalArgumentException.class)
public void detectsViolationOfGetSecureObjectClassMethod() throws Exception { public void detectsViolationOfGetSecureObjectClassMethod() throws Exception {
MockSecurityInterceptorReturnsNull si = new MockSecurityInterceptorReturnsNull(); MockSecurityInterceptorReturnsNull si = new MockSecurityInterceptorReturnsNull();
si.setRunAsManager(jmock.mock(RunAsManager.class)); si.setRunAsManager(mock(RunAsManager.class));
si.setAuthenticationManager(jmock.mock(AuthenticationManager.class)); si.setAuthenticationManager(mock(AuthenticationManager.class));
si.setAfterInvocationManager(jmock.mock(AfterInvocationManager.class)); si.setAfterInvocationManager(mock(AfterInvocationManager.class));
si.setAccessDecisionManager(jmock.mock(AccessDecisionManager.class)); si.setAccessDecisionManager(mock(AccessDecisionManager.class));
si.setSecurityMetadataSource(jmock.mock(SecurityMetadataSource.class)); si.setSecurityMetadataSource(mock(SecurityMetadataSource.class));
jmock.checking(new Expectations() {{ ignoring(anything()); }});
si.afterPropertiesSet(); si.afterPropertiesSet();
} }
@ -71,7 +61,7 @@ public class AbstractSecurityInterceptorTests {
private class MockSecurityInterceptorReturnsNull extends AbstractSecurityInterceptor { private class MockSecurityInterceptorReturnsNull extends AbstractSecurityInterceptor {
private SecurityMetadataSource securityMetadataSource; private SecurityMetadataSource securityMetadataSource;
public Class<? extends Object> getSecureObjectClass() { public Class<?> getSecureObjectClass() {
return null; return null;
} }
@ -87,7 +77,7 @@ public class AbstractSecurityInterceptorTests {
private class MockSecurityInterceptorWhichOnlySupportsStrings extends AbstractSecurityInterceptor { private class MockSecurityInterceptorWhichOnlySupportsStrings extends AbstractSecurityInterceptor {
private SecurityMetadataSource securityMetadataSource; private SecurityMetadataSource securityMetadataSource;
public Class<? extends Object> getSecureObjectClass() { public Class<?> getSecureObjectClass() {
return String.class; return String.class;
} }