From 415a674edc339a83f15c04b08ba24d44884004f8 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 8b8e76d5b6..0ed501e48d 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 javax.servlet.FilterChain; import javax.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 655c363ea0..2dd4c2e30e 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 javax.servlet.Filter; import javax.servlet.FilterChain; @@ -34,6 +35,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; @@ -41,7 +43,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; @@ -80,19 +81,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 @@ -111,6 +109,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;