From 3679227b1143df6fe2e7ba596f252286d7c7aca0 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Sun, 17 Jul 2011 22:21:32 -0500 Subject: [PATCH] SEC-1735: Do not remove SecurityContext from HttpSession when anonymous Authentication is saved if original SecurityContext was anonymous --- .../HttpSessionSecurityContextRepository.java | 16 +++++++++------- ...ttpSessionSecurityContextRepositoryTests.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java index ad6e8180ca..f2310c82d8 100644 --- a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java +++ b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java @@ -93,7 +93,7 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo } requestResponseHolder.setResponse(new SaveToSessionResponseWrapper(response, request, - httpSession != null, context.hashCode())); + httpSession != null, context)); return context; } @@ -295,6 +295,7 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo private HttpServletRequest request; private boolean httpSessionExistedAtStartOfRequest; private int contextHashBeforeChainExecution; + private final SecurityContext contextBeforeExecution; /** * Takes the parameters required to call saveContext() successfully in @@ -304,17 +305,17 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo * @param httpSessionExistedAtStartOfRequest indicates whether there was a session in place before the * filter chain executed. If this is true, and the session is found to be null, this indicates that it was * invalidated during the request and a new session will now be created. - * @param contextHashBeforeChainExecution the hashcode of the context before the filter chain executed. - * The context will only be stored if it has a different hashcode, indicating that the context changed - * during the request. + * @param context the context before the filter chain executed. + * The context will only be stored if it or its contents changed during the request. */ SaveToSessionResponseWrapper(HttpServletResponse response, HttpServletRequest request, boolean httpSessionExistedAtStartOfRequest, - int contextHashBeforeChainExecution) { + SecurityContext contextBeforeExcecution) { super(response, disableUrlRewriting); this.request = request; this.httpSessionExistedAtStartOfRequest = httpSessionExistedAtStartOfRequest; - this.contextHashBeforeChainExecution = contextHashBeforeChainExecution; + this.contextHashBeforeChainExecution = contextBeforeExcecution.hashCode(); + this.contextBeforeExecution = contextBeforeExcecution; } /** @@ -338,8 +339,9 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo logger.debug("SecurityContext is empty or anonymous - context will not be stored in HttpSession. "); } - if (httpSession != null) { + if (httpSession != null && !contextObject.equals(contextBeforeExecution)) { // SEC-1587 A non-anonymous context may still be in the session + // SEC-1735 remove if the contextBeforeExecution was not anonymous httpSession.removeAttribute(SPRING_SECURITY_CONTEXT_KEY); } return; diff --git a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java index 21580bb59c..0fe64a5fb6 100644 --- a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java @@ -1,6 +1,7 @@ package org.springframework.security.web.context; import static org.junit.Assert.*; +import static org.springframework.security.web.context.HttpSessionSecurityContextRepository.*; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; @@ -186,6 +187,21 @@ public class HttpSessionSecurityContextRepositoryTests { assertTrue(repo.generateNewContext() instanceof MockContext); } + // SEC-1735 + @Test + public void contextIsNotRemovedFromSessionIfContextBeforeExecutionDefault() throws Exception { + HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository(); + MockHttpServletRequest request = new MockHttpServletRequest(); + HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, new MockHttpServletResponse()); + repo.loadContext(holder); + SecurityContext ctxInSession = SecurityContextHolder.createEmptyContext(); + ctxInSession.setAuthentication(testToken); + request.getSession().setAttribute(SPRING_SECURITY_CONTEXT_KEY, ctxInSession); + SecurityContextHolder.getContext().setAuthentication(new AnonymousAuthenticationToken("x","x", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"))); + repo.saveContext(SecurityContextHolder.getContext(), holder.getRequest(), holder.getResponse()); + assertSame(ctxInSession,request.getSession().getAttribute(SPRING_SECURITY_CONTEXT_KEY)); + } + @Test @SuppressWarnings("deprecation") public void sessionDisableUrlRewritingPreventsSessionIdBeingWrittenToUrl() throws Exception {