From b8d51725c78174141407725587ff0322ff158192 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 28 Jul 2021 12:44:46 -0600 Subject: [PATCH] Immutable SecurityContext Issue gh-10032 --- .../security/cas/web/CasAuthenticationFilter.java | 5 ++++- .../access/intercept/AbstractSecurityInterceptor.java | 9 ++++++--- .../springframework/security/core/Authentication.java | 4 +++- .../security/provisioning/JdbcUserDetailsManager.java | 8 ++++++-- .../remoting/rmi/ContextPropagatingRemoteInvocation.java | 5 ++++- .../security/web/access/ExceptionTranslationFilter.java | 6 ++++-- .../AbstractAuthenticationProcessingFilter.java | 5 ++++- .../authentication/AnonymousAuthenticationFilter.java | 6 +++++- .../logout/SecurityContextLogoutHandler.java | 5 ++++- .../AbstractPreAuthenticatedProcessingFilter.java | 7 +++++-- .../rememberme/RememberMeAuthenticationFilter.java | 5 ++++- .../web/authentication/switchuser/SwitchUserFilter.java | 9 +++++++-- .../authentication/www/BasicAuthenticationFilter.java | 5 ++++- .../authentication/www/DigestAuthenticationFilter.java | 3 ++- .../web/servletapi/HttpServlet3RequestFactory.java | 6 ++++-- 15 files changed, 66 insertions(+), 22 deletions(-) diff --git a/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java b/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java index 42339c0c9d..2352887b1f 100644 --- a/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java +++ b/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java @@ -37,6 +37,7 @@ import org.springframework.security.cas.web.authentication.ServiceAuthentication import org.springframework.security.cas.web.authentication.ServiceAuthenticationDetailsSource; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter; import org.springframework.security.web.authentication.AuthenticationFailureHandler; @@ -219,7 +220,9 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil } this.logger.debug( LogMessage.format("Authentication success. Updating SecurityContextHolder to contain: %s", authResult)); - SecurityContextHolder.getContext().setAuthentication(authResult); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authResult); + SecurityContextHolder.setContext(context); if (this.eventPublisher != null) { this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass())); } 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 8b385fbae6..0870fe2da6 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 @@ -217,8 +217,9 @@ public abstract class AbstractSecurityInterceptor Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attributes); if (runAs != null) { SecurityContext origCtx = SecurityContextHolder.getContext(); - SecurityContextHolder.setContext(SecurityContextHolder.createEmptyContext()); - SecurityContextHolder.getContext().setAuthentication(runAs); + SecurityContext newCtx = SecurityContextHolder.createEmptyContext(); + newCtx.setAuthentication(runAs); + SecurityContextHolder.setContext(newCtx); if (this.logger.isDebugEnabled()) { this.logger.debug(LogMessage.format("Switched to RunAs authentication %s", runAs)); @@ -316,7 +317,9 @@ public abstract class AbstractSecurityInterceptor if (this.logger.isDebugEnabled()) { this.logger.debug(LogMessage.format("Re-authenticated %s before authorizing", authentication)); } - SecurityContextHolder.getContext().setAuthentication(authentication); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authentication); + SecurityContextHolder.setContext(context); return authentication; } diff --git a/core/src/main/java/org/springframework/security/core/Authentication.java b/core/src/main/java/org/springframework/security/core/Authentication.java index 12e04fdeb2..d3f38a5c11 100644 --- a/core/src/main/java/org/springframework/security/core/Authentication.java +++ b/core/src/main/java/org/springframework/security/core/Authentication.java @@ -36,7 +36,9 @@ import org.springframework.security.core.context.SecurityContextHolder; * the code: * * 
- * SecurityContextHolder.getContext().setAuthentication(anAuthentication);
+ * SecurityContext context = SecurityContextHolder.createEmptyContext();
+ * context.setAuthentication(anAuthentication);
+ * SecurityContextHolder.setContext(context);
  *
* * Note that unless the Authentication has the authenticated property diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java index 75a96b478a..2cfda0ab06 100644 --- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java +++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,6 +38,7 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserCache; @@ -277,7 +278,10 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa } this.logger.debug("Changing password for user '" + username + "'"); getJdbcTemplate().update(this.changePasswordSql, newPassword, username); - SecurityContextHolder.getContext().setAuthentication(createNewAuthentication(currentUser, newPassword)); + Authentication authentication = createNewAuthentication(currentUser, newPassword); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authentication); + SecurityContextHolder.setContext(context); this.userCache.removeUserFromCache(username); } diff --git a/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java b/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java index 56563301ae..2324a3b201 100644 --- a/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java +++ b/remoting/src/main/java/org/springframework/security/remoting/rmi/ContextPropagatingRemoteInvocation.java @@ -27,6 +27,7 @@ import org.springframework.remoting.support.RemoteInvocation; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.SpringSecurityCoreVersion; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; /** @@ -97,7 +98,9 @@ public class ContextPropagatingRemoteInvocation extends RemoteInvocation { if (this.principal != null) { Authentication request = createAuthenticationRequest(this.principal, this.credentials); request.setAuthenticated(false); - SecurityContextHolder.getContext().setAuthentication(request); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(request); + SecurityContextHolder.setContext(context); logger.debug(LogMessage.format("Set SecurityContextHolder to contain: %s", request)); } try { diff --git a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java index e12bbb53c9..7b96e40d79 100644 --- a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2004-2020 the original author or authors. + * Copyright 2004-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ import org.springframework.security.authentication.InsufficientAuthenticationExc import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.SpringSecurityMessageSource; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; @@ -208,7 +209,8 @@ public class ExceptionTranslationFilter extends GenericFilterBean implements Mes AuthenticationException reason) throws ServletException, IOException { // SEC-112: Clear the SecurityContextHolder's Authentication, as the // existing Authentication is no longer considered valid - SecurityContextHolder.getContext().setAuthentication(null); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + SecurityContextHolder.setContext(context); this.requestCache.saveRequest(request, response); this.authenticationEntryPoint.commence(request, response, reason); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java index 6da3e7c13c..933463c98c 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java @@ -38,6 +38,7 @@ import org.springframework.security.authentication.event.InteractiveAuthenticati import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.SpringSecurityMessageSource; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.session.NullAuthenticatedSessionStrategy; import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; @@ -310,7 +311,9 @@ public abstract class AbstractAuthenticationProcessingFilter extends GenericFilt */ protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, Authentication authResult) throws IOException, ServletException { - SecurityContextHolder.getContext().setAuthentication(authResult); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authResult); + SecurityContextHolder.setContext(context); if (this.logger.isDebugEnabled()) { this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", authResult)); } 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 0a893b9748..341c8c16e1 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 @@ -32,6 +32,7 @@ import org.springframework.security.authentication.AuthenticationDetailsSource; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.util.Assert; import org.springframework.web.filter.GenericFilterBean; @@ -87,7 +88,10 @@ public class AnonymousAuthenticationFilter extends GenericFilterBean implements public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { if (SecurityContextHolder.getContext().getAuthentication() == null) { - SecurityContextHolder.getContext().setAuthentication(createAuthentication((HttpServletRequest) req)); + Authentication authentication = createAuthentication((HttpServletRequest) req); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authentication); + SecurityContextHolder.setContext(context); if (this.logger.isTraceEnabled()) { this.logger.trace(LogMessage.of(() -> "Set SecurityContextHolder to " + SecurityContextHolder.getContext().getAuthentication())); diff --git a/web/src/main/java/org/springframework/security/web/authentication/logout/SecurityContextLogoutHandler.java b/web/src/main/java/org/springframework/security/web/authentication/logout/SecurityContextLogoutHandler.java index 97b1156691..5dcd61ad97 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/logout/SecurityContextLogoutHandler.java +++ b/web/src/main/java/org/springframework/security/web/authentication/logout/SecurityContextLogoutHandler.java @@ -70,9 +70,12 @@ public class SecurityContextLogoutHandler implements LogoutHandler { } if (this.clearAuthentication) { SecurityContext context = SecurityContextHolder.getContext(); + SecurityContextHolder.clearContext(); context.setAuthentication(null); } - SecurityContextHolder.clearContext(); + else { + SecurityContextHolder.clearContext(); + } } public boolean isInvalidateHttpSession() { diff --git a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java index 6ee3d98e51..574e95de82 100755 --- a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.WebAttributes; import org.springframework.security.web.authentication.AuthenticationFailureHandler; @@ -206,7 +207,9 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, Authentication authResult) throws IOException, ServletException { this.logger.debug(LogMessage.format("Authentication success: %s", authResult)); - SecurityContextHolder.getContext().setAuthentication(authResult); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authResult); + SecurityContextHolder.setContext(context); if (this.eventPublisher != null) { this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass())); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java index f41fd586a4..d8573c0d70 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java @@ -32,6 +32,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.authentication.RememberMeServices; @@ -107,7 +108,9 @@ public class RememberMeAuthenticationFilter extends GenericFilterBean implements try { rememberMeAuth = this.authenticationManager.authenticate(rememberMeAuth); // Store to SecurityContextHolder - SecurityContextHolder.getContext().setAuthentication(rememberMeAuth); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(rememberMeAuth); + SecurityContextHolder.setContext(context); onSuccessfulAuthentication(request, response, rememberMeAuth); this.logger.debug(LogMessage.of(() -> "SecurityContextHolder populated with remember-me token: '" + SecurityContextHolder.getContext().getAuthentication() + "'")); diff --git a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java index c7e57ef4fa..3693b7c1c9 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserFilter.java @@ -47,6 +47,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.SpringSecurityMessageSource; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UserDetailsChecker; @@ -174,7 +175,9 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv try { Authentication targetUser = attemptSwitchUser(request); // update the current context to the new target user - SecurityContextHolder.getContext().setAuthentication(targetUser); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(targetUser); + SecurityContextHolder.setContext(context); // redirect to target url this.successHandler.onAuthenticationSuccess(request, response, targetUser); } @@ -188,7 +191,9 @@ public class SwitchUserFilter extends GenericFilterBean implements ApplicationEv // get the original authentication object (if exists) Authentication originalUser = attemptExitUser(request); // update the current context back to the original user - SecurityContextHolder.getContext().setAuthentication(originalUser); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(originalUser); + SecurityContextHolder.setContext(context); // redirect to target url this.successHandler.onAuthenticationSuccess(request, response, originalUser); return; diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java index e744825b4e..9d639adf15 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java @@ -31,6 +31,7 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.authentication.NullRememberMeServices; @@ -153,7 +154,9 @@ public class BasicAuthenticationFilter extends OncePerRequestFilter { this.logger.trace(LogMessage.format("Found username '%s' in Basic Authorization header", username)); if (authenticationIsRequired(username)) { Authentication authResult = this.authenticationManager.authenticate(authRequest); - SecurityContextHolder.getContext().setAuthentication(authResult); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authResult); + SecurityContextHolder.setContext(context); if (this.logger.isDebugEnabled()) { this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", authResult)); } diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/DigestAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/DigestAuthenticationFilter.java index 00a119cdc5..21efe4f5f8 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/DigestAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/DigestAuthenticationFilter.java @@ -210,7 +210,8 @@ public class DigestAuthenticationFilter extends GenericFilterBean implements Mes private void fail(HttpServletRequest request, HttpServletResponse response, AuthenticationException failed) throws IOException, ServletException { - SecurityContextHolder.getContext().setAuthentication(null); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + SecurityContextHolder.setContext(context); logger.debug(failed); this.authenticationEntryPoint.commence(request, response, failed); } diff --git a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java index 3a2dd9d36d..51113ac551 100644 --- a/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java +++ b/web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -225,7 +225,9 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory { return; } Authentication authentication = getAuthentication(authManager, username, password); - SecurityContextHolder.getContext().setAuthentication(authentication); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authentication); + SecurityContextHolder.setContext(context); } private Authentication getAuthentication(AuthenticationManager authManager, String username, String password)