Forráskód Böngészése

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
Rob Winch 3 éve
szülő
commit
415a674edc

+ 24 - 8
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<SecurityContext> deferredContext = this.securityContextHolderStrategy.getDeferredContext();
+		this.securityContextHolderStrategy
+				.setDeferredContext(defaultWithAnonymous((HttpServletRequest) req, deferredContext));
+		chain.doFilter(req, res);
+	}
+
+	private Supplier<SecurityContext> defaultWithAnonymous(HttpServletRequest request,
+			Supplier<SecurityContext> 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) {

+ 25 - 8
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<SecurityContext> 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;