From b27d7afd24212081a41bb9774fce2b9cbfae81d3 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Sun, 6 Dec 2009 15:28:03 +0000
Subject: [PATCH] SEC-1315: Modify HttpSessionSecurityContextRepository to
 check for anonymous token before creating a session. Moved the anonymity
 check to be before the session creation.

---
 .../HttpSessionSecurityContextRepository.java | 24 +++++++++----------
 ...SessionSecurityContextRepositoryTests.java | 16 +++++++++++++
 2 files changed, 28 insertions(+), 12 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 ff9fcd8121..43b6450ee0 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
@@ -325,6 +325,14 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo
          */
         @Override
         void saveContext(SecurityContext context) {
+            // See SEC-776
+            if (authenticationTrustResolver.isAnonymous(context.getAuthentication())) {
+                if (logger.isDebugEnabled()) {
+                    logger.debug("SecurityContext contents are anonymous - context will not be stored in HttpSession. ");
+                }
+                return;
+            }
+
             HttpSession httpSession = request.getSession(false);
 
             if (httpSession == null) {
@@ -334,18 +342,10 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo
             // If HttpSession exists, store current SecurityContextHolder contents but only if
             // the SecurityContext has actually changed (see JIRA SEC-37)
             if (httpSession != null && context.hashCode() != contextHashBeforeChainExecution) {
-                // See SEC-776
-                // TODO: Move this so that a session isn't created if user is anonymous
-                if (authenticationTrustResolver.isAnonymous(context.getAuthentication())) {
-                    if (logger.isDebugEnabled()) {
-                        logger.debug("SecurityContext contents are anonymous - context will not be stored in HttpSession. ");
-                    }
-                } else {
-                    httpSession.setAttribute(SPRING_SECURITY_CONTEXT_KEY, context);
+                httpSession.setAttribute(SPRING_SECURITY_CONTEXT_KEY, context);
 
-                    if (logger.isDebugEnabled()) {
-                        logger.debug("SecurityContext stored to HttpSession: '" + context + "'");
-                    }
+                if (logger.isDebugEnabled()) {
+                    logger.debug("SecurityContext stored to HttpSession: '" + context + "'");
                 }
             }
         }
@@ -374,7 +374,7 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo
 
             if (contextObject.equals(context)) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("HttpSession is null, but SecurityContext has not changed from default: ' "
+                    logger.debug("HttpSession is null, but SecurityContext has not changed from default empty context: ' "
                             + context
                             + "'; not creating HttpSession or storing SecurityContext");
                 }
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 2220e90b9a..21580bb59c 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
@@ -5,8 +5,10 @@ import static org.junit.Assert.*;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.authentication.AnonymousAuthenticationToken;
 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.web.context.HttpRequestResponseHolder;
@@ -146,6 +148,20 @@ public class HttpSessionSecurityContextRepositoryTests {
         assertNull(request.getSession(false));
     }
 
+    // SEC-1315
+    @Test
+    public void noSessionIsCreatedIfAnonymousTokenIsUsed() throws Exception {
+        HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        HttpRequestResponseHolder holder = new HttpRequestResponseHolder(request, response);
+        SecurityContextHolder.setContext(repo.loadContext(holder));
+        SecurityContextHolder.getContext().setAuthentication(
+                new AnonymousAuthenticationToken("key", "anon", AuthorityUtils.createAuthorityList("ANON")));
+        repo.saveContext(SecurityContextHolder.getContext(), holder.getRequest(), holder.getResponse());
+        assertNull(request.getSession(false));
+    }
+
     @Test
     @Deprecated
     public void settingCloneFromContextLoadsClonedContextObject() throws Exception {