From 0bf985ed7cab5b3c8a08c50b5a733569cdee6d0b Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 5 Jul 2022 14:59:42 -0500 Subject: [PATCH] AnonymousAuthenticationFilter Avoids Eager SecurityContext Access Previously AnonymousAuthenticationFilter accessed the SecurityContext to determine if anonymous authentication needed setup eagerly. Now this is done lazily to avoid unnecessary access to the SecurityContext which in turn avoids unnecessary HTTP Session access. Closes gh-11457 --- .../AnonymousAuthenticationFilter.java | 32 +++++++++++++----- .../AnonymousAuthenticationFilterTests.java | 33 ++++++++++++++----- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java index ea141bf0da..55f3b9edf0 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilter.java @@ -18,6 +18,7 @@ package org.springframework.security.web.authentication; import java.io.IOException; import java.util.List; +import java.util.function.Supplier; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; @@ -91,18 +92,33 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean implements @Override public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { - if (this.securityContextHolderStrategy.getContext().getAuthentication() == null) { - Authentication authentication = createAuthentication((HttpServletRequest) req); - SecurityContext context = this.securityContextHolderStrategy.createEmptyContext(); - context.setAuthentication(authentication); - this.securityContextHolderStrategy.setContext(context); + Supplier deferredContext = this.securityContextHolderStrategy.getDeferredContext(); + this.securityContextHolderStrategy + .setDeferredContext(defaultWithAnonymous((HttpServletRequest) req, deferredContext)); + chain.doFilter(req, res); + } + + private Supplier defaultWithAnonymous(HttpServletRequest request, + Supplier currentDeferredContext) { + return () -> { + SecurityContext currentContext = currentDeferredContext.get(); + return defaultWithAnonymous(request, currentContext); + }; + } + + private SecurityContext defaultWithAnonymous(HttpServletRequest request, SecurityContext currentContext) { + Authentication currentAuthentication = currentContext.getAuthentication(); + if (currentAuthentication == null) { + Authentication anonymous = createAuthentication(request); if (this.logger.isTraceEnabled()) { - this.logger.trace(LogMessage.of(() -> "Set SecurityContextHolder to " - + this.securityContextHolderStrategy.getContext().getAuthentication())); + this.logger.trace(LogMessage.of(() -> "Set SecurityContextHolder to " + anonymous)); } else { this.logger.debug("Set SecurityContextHolder to anonymous SecurityContext"); } + SecurityContext anonymousContext = this.securityContextHolderStrategy.createEmptyContext(); + anonymousContext.setAuthentication(anonymous); + return anonymousContext; } else { if (this.logger.isTraceEnabled()) { @@ -110,7 +126,7 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean implements + this.securityContextHolderStrategy.getContext().getAuthentication())); } } - chain.doFilter(req, res); + return currentContext; } protected Authentication createAuthentication(HttpServletRequest request) { diff --git a/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java index f45788fefb..40ce32df5e 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/AnonymousAuthenticationFilterTests.java @@ -17,6 +17,7 @@ package org.springframework.security.web.authentication; import java.io.IOException; +import java.util.function.Supplier; import jakarta.servlet.Filter; import jakarta.servlet.FilterChain; @@ -33,6 +34,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.context.SecurityContextImpl; @@ -40,7 +42,6 @@ import org.springframework.security.core.context.SecurityContextImpl; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.fail; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -79,19 +80,16 @@ public class AnonymousAuthenticationFilterTests { public void testOperationWhenAuthenticationExistsInContextHolder() throws Exception { // Put an Authentication object into the SecurityContextHolder Authentication originalAuth = new TestingAuthenticationToken("user", "password", "ROLE_A"); - SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class); - given(strategy.getContext()).willReturn(new SecurityContextImpl(originalAuth)); + SecurityContext originalContext = new SecurityContextImpl(originalAuth); + SecurityContextHolder.setContext(originalContext); AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter("qwerty", "anonymousUsername", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")); - filter.setSecurityContextHolderStrategy(strategy); - // Test MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("x"); executeFilterInContainerSimulator(mock(FilterConfig.class), filter, request, new MockHttpServletResponse(), new MockFilterChain(true)); - // Ensure filter didn't change our original object - verify(strategy).getContext(); - verify(strategy, never()).setContext(any()); + // Ensure getDeferredContext still + assertThat(SecurityContextHolder.getContext()).isEqualTo(originalContext); } @Test @@ -110,6 +108,25 @@ public class AnonymousAuthenticationFilterTests { // again } + @Test + public void doFilterDoesNotGetContext() throws Exception { + Supplier originalSupplier = mock(Supplier.class); + Authentication originalAuth = new TestingAuthenticationToken("user", "password", "ROLE_A"); + SecurityContext originalContext = new SecurityContextImpl(originalAuth); + SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class); + given(strategy.getDeferredContext()).willReturn(originalSupplier); + given(strategy.getContext()).willReturn(originalContext); + AnonymousAuthenticationFilter filter = new AnonymousAuthenticationFilter("qwerty", "anonymousUsername", + AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")); + filter.setSecurityContextHolderStrategy(strategy); + filter.afterPropertiesSet(); + + executeFilterInContainerSimulator(mock(FilterConfig.class), filter, new MockHttpServletRequest(), + new MockHttpServletResponse(), new MockFilterChain(true)); + verify(strategy, never()).getContext(); + verify(originalSupplier, never()).get(); + } + private class MockFilterChain implements FilterChain { private boolean expectToProceed;