From 01c9c4e4db301f1e6268b5fd50f11eae4beadd10 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 6 Apr 2011 13:54:32 +0100 Subject: [PATCH] SEC-1697: Don't publish authorization success events in AbstractSecurityInterceptor by default. --- .../intercept/AbstractSecurityInterceptor.java | 15 ++++++++++++++- .../MethodSecurityInterceptorTests.java | 17 +++++++++++++++-- .../FilterSecurityInterceptorTests.java | 3 ++- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java b/core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java index 8eba41366d..e86cbb6f21 100644 --- a/core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java +++ b/core/src/main/java/org/springframework/security/access/intercept/AbstractSecurityInterceptor.java @@ -109,6 +109,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A private boolean alwaysReauthenticate = false; private boolean rejectPublicInvocations = false; private boolean validateConfigAttributes = true; + private boolean publishAuthorizationSuccess = false; //~ Methods ======================================================================================================== @@ -212,7 +213,9 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A logger.debug("Authorization successful"); } - publishEvent(new AuthorizedEvent(object, attributes, authenticated)); + if (publishAuthorizationSuccess) { + publishEvent(new AuthorizedEvent(object, attributes, authenticated)); + } // Attempt to run as a different user Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attributes); @@ -402,6 +405,16 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A this.messages = new MessageSourceAccessor(messageSource); } + /** + * Only {@code AuthorizationFailureEvent} will be published. + * If you set this property to {@code true}, {@code AuthorizedEvent}s will also be published. + * + * @param publishAuthorizationSuccess default value is {@code false} + */ + public void setPublishAuthorizationSuccess(boolean publishAuthorizationSuccess) { + this.publishAuthorizationSuccess = publishAuthorizationSuccess; + } + /** * By rejecting public invocations (and setting this property to true), essentially you are ensuring * that every secure object invocation advised by AbstractSecurityInterceptor has a configuration diff --git a/core/src/test/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptorTests.java index 3b5dd4c235..0c7a120c7c 100644 --- a/core/src/test/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/access/intercept/aopalliance/MethodSecurityInterceptorTests.java @@ -26,12 +26,15 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.springframework.aop.framework.ProxyFactory; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.security.ITargetObject; import org.springframework.security.TargetObject; import org.springframework.security.access.AccessDecisionManager; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.SecurityConfig; +import org.springframework.security.access.event.AuthorizationFailureEvent; +import org.springframework.security.access.event.AuthorizedEvent; import org.springframework.security.access.intercept.AfterInvocationManager; import org.springframework.security.access.intercept.RunAsManager; import org.springframework.security.access.intercept.RunAsUserToken; @@ -58,6 +61,7 @@ public class MethodSecurityInterceptorTests { private AccessDecisionManager adm; private MethodSecurityMetadataSource mds; private AuthenticationManager authman; + private ApplicationEventPublisher eventPublisher; //~ Methods ======================================================================================================== @@ -69,9 +73,11 @@ public class MethodSecurityInterceptorTests { adm = mock(AccessDecisionManager.class); authman = mock(AuthenticationManager.class); mds = mock(MethodSecurityMetadataSource.class); + eventPublisher = mock(ApplicationEventPublisher.class); interceptor.setAccessDecisionManager(adm); interceptor.setAuthenticationManager(authman); interceptor.setSecurityMetadataSource(mds); + interceptor.setApplicationEventPublisher(eventPublisher); createTarget(false); } @@ -210,6 +216,7 @@ public class MethodSecurityInterceptorTests { @Test public void callSucceedsIfAccessDecisionManagerGrantsAccess() throws Exception { token.setAuthenticated(true); + interceptor.setPublishAuthorizationSuccess(true); SecurityContextHolder.getContext().setAuthentication(token); mdsReturnsUserRole(); @@ -217,9 +224,10 @@ public class MethodSecurityInterceptorTests { // Note we check the isAuthenticated remained true in following line assertEquals("hello org.springframework.security.authentication.TestingAuthenticationToken true", result); + verify(eventPublisher).publishEvent(any(AuthorizedEvent.class)); } - @Test(expected=AccessDeniedException.class) + @Test public void callIsntMadeWhenAccessDecisionManagerRejectsAccess() throws Exception { SecurityContextHolder.getContext().setAuthentication(token); // Use mocked target to make sure invocation doesn't happen (not in expectations so test would fail) @@ -228,7 +236,12 @@ public class MethodSecurityInterceptorTests { when(authman.authenticate(token)).thenReturn(token); doThrow(new AccessDeniedException("rejected")).when(adm).decide(any(Authentication.class), any(MethodInvocation.class), any(List.class)); - advisedTarget.makeUpperCase("HELLO"); + try { + advisedTarget.makeUpperCase("HELLO"); + fail(); + } catch (AccessDeniedException expected) { + } + verify(eventPublisher).publishEvent(any(AuthorizationFailureEvent.class)); } @Test(expected=IllegalArgumentException.class) diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/FilterSecurityInterceptorTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/FilterSecurityInterceptorTests.java index cfc6eaffb6..9b7effb3ca 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/FilterSecurityInterceptorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/FilterSecurityInterceptorTests.java @@ -105,7 +105,8 @@ public class FilterSecurityInterceptorTests { interceptor.invoke(fi); - verify(publisher).publishEvent(any(AuthorizedEvent.class)); + // SEC-1697 + verify(publisher, never()).publishEvent(any(AuthorizedEvent.class)); } @Test