From db5f1e69f1d10ad3af645aef3576a78f36932861 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 17 Dec 2008 00:14:48 +0000 Subject: [PATCH] SEC-949: Added the option of specifying -1 as the token-validity-seconds value in order to set the cookie maxAge to expire when the browser closes. --- .../RememberMeBeanDefinitionParser.java | 7 +- .../AbstractRememberMeServices.java | 3 +- ...ersistentTokenBasedRememberMeServices.java | 23 ++++--- .../TokenBasedRememberMeServices.java | 8 ++- .../security/config/spring-security-2.5.rnc | 10 ++- .../security/config/spring-security-2.5.xsd | 13 ++-- ...HttpSecurityBeanDefinitionParserTests.java | 31 +++++++-- .../TokenBasedRememberMeServicesTests.java | 64 ++++++++++++------- 8 files changed, 112 insertions(+), 47 deletions(-) diff --git a/core/src/main/java/org/springframework/security/config/RememberMeBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/RememberMeBeanDefinitionParser.java index c5ec0d9763..0f366056eb 100644 --- a/core/src/main/java/org/springframework/security/config/RememberMeBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/RememberMeBeanDefinitionParser.java @@ -98,7 +98,12 @@ public class RememberMeBeanDefinitionParser implements BeanDefinitionParser { } if (tokenValiditySet) { - services.getPropertyValues().addPropertyValue("tokenValiditySeconds", new Integer(tokenValiditySeconds)); + Integer tokenValidity = new Integer(tokenValiditySeconds); + if (tokenValidity.intValue() < 0 && isPersistent) { + parserContext.getReaderContext().error(ATT_TOKEN_VALIDITY + " cannot be negative if using" + + " a persistent remember-me token repository", source); + } + services.getPropertyValues().addPropertyValue("tokenValiditySeconds", tokenValidity); } services.setSource(source); services.getPropertyValues().addPropertyValue(ATT_KEY, key); diff --git a/core/src/main/java/org/springframework/security/ui/rememberme/AbstractRememberMeServices.java b/core/src/main/java/org/springframework/security/ui/rememberme/AbstractRememberMeServices.java index a8d331766b..6bdfd1e3ce 100644 --- a/core/src/main/java/org/springframework/security/ui/rememberme/AbstractRememberMeServices.java +++ b/core/src/main/java/org/springframework/security/ui/rememberme/AbstractRememberMeServices.java @@ -36,6 +36,7 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, public static final String SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY = "SPRING_SECURITY_REMEMBER_ME_COOKIE"; public static final String DEFAULT_PARAMETER = "_spring_security_remember_me"; + public static final int TWO_WEEKS_S = 1209600; private static final String DELIMITER = ":"; @@ -52,7 +53,7 @@ public abstract class AbstractRememberMeServices implements RememberMeServices, private String parameter = DEFAULT_PARAMETER; private boolean alwaysRemember; private String key; - private int tokenValiditySeconds = 1209600; // 14 days + private int tokenValiditySeconds = TWO_WEEKS_S; public void afterPropertiesSet() throws Exception { Assert.hasLength(key); diff --git a/core/src/main/java/org/springframework/security/ui/rememberme/PersistentTokenBasedRememberMeServices.java b/core/src/main/java/org/springframework/security/ui/rememberme/PersistentTokenBasedRememberMeServices.java index 986d74e300..77adf794c3 100644 --- a/core/src/main/java/org/springframework/security/ui/rememberme/PersistentTokenBasedRememberMeServices.java +++ b/core/src/main/java/org/springframework/security/ui/rememberme/PersistentTokenBasedRememberMeServices.java @@ -1,17 +1,18 @@ package org.springframework.security.ui.rememberme; -import org.springframework.security.Authentication; -import org.springframework.security.userdetails.UserDetails; -import org.springframework.dao.DataAccessException; - -import org.apache.commons.codec.binary.Base64; - -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.security.SecureRandom; import java.util.Arrays; import java.util.Date; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.commons.codec.binary.Base64; +import org.springframework.dao.DataAccessException; +import org.springframework.security.Authentication; +import org.springframework.security.userdetails.UserDetails; +import org.springframework.util.Assert; + /** * {@link RememberMeServices} implementation based on Barry Jaspan's * Improved Persistent Login Cookie @@ -164,4 +165,10 @@ public class PersistentTokenBasedRememberMeServices extends AbstractRememberMeSe public void setTokenLength(int tokenLength) { this.tokenLength = tokenLength; } + + @Override + public void setTokenValiditySeconds(int tokenValiditySeconds) { + Assert.isTrue(tokenValiditySeconds > 0, "tokenValiditySeconds must be positive for this implementation"); + super.setTokenValiditySeconds(tokenValiditySeconds); + } } diff --git a/core/src/main/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServices.java b/core/src/main/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServices.java index 9f00e22de8..42d295bf05 100644 --- a/core/src/main/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServices.java +++ b/core/src/main/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServices.java @@ -68,7 +68,9 @@ import java.util.Date; * require a more secure remember-me approach (see {@link PersistentTokenBasedRememberMeServices}). *

* By default the tokens will be valid for 14 days from the last successful authentication attempt. This can be changed - * using {@link #setTokenValiditySeconds(int)}. + * using {@link #setTokenValiditySeconds(int)}. If this value is less than zero, the expiryTime will remain at + * 14 days, but the negative value will be used for the maxAge property of the cookie, meaning that it will + * not be stored when the browser is closed. * * * @author Ben Alex @@ -147,7 +149,9 @@ public class TokenBasedRememberMeServices extends AbstractRememberMeServices { } int tokenLifetime = calculateLoginLifetime(request, successfulAuthentication); - long expiryTime = System.currentTimeMillis() + 1000L*tokenLifetime; + long expiryTime = System.currentTimeMillis(); + // SEC-949 + expiryTime += 1000L* (tokenLifetime < 0 ? TWO_WEEKS_S : tokenLifetime); String signatureValue = makeTokenSignature(expiryTime, username, password); diff --git a/core/src/main/resources/org/springframework/security/config/spring-security-2.5.rnc b/core/src/main/resources/org/springframework/security/config/spring-security-2.5.rnc index 800bc3f892..153fefab76 100644 --- a/core/src/main/resources/org/springframework/security/config/spring-security-2.5.rnc +++ b/core/src/main/resources/org/springframework/security/config/spring-security-2.5.rnc @@ -16,7 +16,7 @@ path-type = attribute path-type {"ant" | "regex"} port = ## Specifies an IP port number. Used to configure an embedded LDAP server, for example. - attribute port { xsd:integer } + attribute port { xsd:positiveInteger } url = ## Specifies a URL. attribute url { xsd:string } @@ -258,6 +258,10 @@ http.attlist &= http.attlist &= ## Allows the access denied page to be set (the user will be redirected here if an AccessDeniedException is raised). attribute access-denied-page {xsd:string}? +http.attlist &= + ## + attribute disable-url-rewriting {boolean}? + intercept-url = ## Specifies the access attributes and/or filter list for a particular set of URLs. @@ -389,8 +393,8 @@ remember-me.attlist &= user-service-ref? remember-me.attlist &= - ## The period (in seconds) for which the remember-me cookie should be valid. - attribute token-validity-seconds {xsd:positiveInteger}? + ## The period (in seconds) for which the remember-me cookie should be valid. If set to a negative value + attribute token-validity-seconds {xsd:integer}? token-repository-ref = ## Reference to a PersistentTokenRepository bean for use with the persistent token remember-me implementation. diff --git a/core/src/main/resources/org/springframework/security/config/spring-security-2.5.xsd b/core/src/main/resources/org/springframework/security/config/spring-security-2.5.xsd index 5c6b0b13ef..5f4841e5ae 100644 --- a/core/src/main/resources/org/springframework/security/config/spring-security-2.5.xsd +++ b/core/src/main/resources/org/springframework/security/config/spring-security-2.5.xsd @@ -51,7 +51,7 @@ - + Specifies an IP port number. Used to configure an embedded LDAP server, for example. @@ -206,7 +206,7 @@ Specifies a URL. - + Specifies an IP port number. Used to configure an embedded LDAP server, for example. @@ -890,6 +890,11 @@ here if an AccessDeniedException is raised). + + + + + @@ -1187,10 +1192,10 @@ Id - + The period (in seconds) for which the remember-me cookie should be - valid. + valid. If set to a negative value diff --git a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java index 6f8dcc4081..c7d7acd6ac 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -370,11 +370,11 @@ public class HttpSecurityBeanDefinitionParserTests { @Test public void rememberMeServiceWorksWithTokenRepoRef() { setContext( - "" + - " " + - "" + - " " + AUTH_PROVIDER_XML); + "" + + " " + + "" + + " " + AUTH_PROVIDER_XML); Object rememberMeServices = appContext.getBean(BeanIds.REMEMBER_ME_SERVICES); assertTrue(rememberMeServices instanceof PersistentTokenBasedRememberMeServices); @@ -394,7 +394,6 @@ public class HttpSecurityBeanDefinitionParserTests { assertTrue(rememberMeServices instanceof PersistentTokenBasedRememberMeServices); } - @Test public void rememberMeServiceWorksWithExternalServicesImpl() throws Exception { setContext( @@ -426,6 +425,26 @@ public class HttpSecurityBeanDefinitionParserTests { "tokenValiditySeconds")); } + @Test + public void rememberMeTokenValidityAllowsNegativeValueForNonPersistentImplementation() throws Exception { + setContext( + "" + + " " + + "" + AUTH_PROVIDER_XML); + assertEquals(-1, FieldUtils.getFieldValue(appContext.getBean(BeanIds.REMEMBER_ME_SERVICES), + "tokenValiditySeconds")); + } + + @Test(expected=BeanDefinitionParsingException.class) + public void rememberMeTokenValidityRejectsNegativeValueForPersistentImplementation() throws Exception { + setContext( + "" + + " " + + "" + + " " + + AUTH_PROVIDER_XML); + } + @Test public void rememberMeServiceConfigurationParsesWithCustomUserService() { setContext( diff --git a/core/src/test/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServicesTests.java b/core/src/test/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServicesTests.java index 357e0194b7..ea9c4aff70 100644 --- a/core/src/test/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServicesTests.java +++ b/core/src/test/java/org/springframework/security/ui/rememberme/TokenBasedRememberMeServicesTests.java @@ -16,6 +16,7 @@ package org.springframework.security.ui.rememberme; import static org.junit.Assert.*; +import static org.springframework.security.ui.rememberme.TokenBasedRememberMeServices.*; import java.util.Date; @@ -102,7 +103,7 @@ public class TokenBasedRememberMeServicesTests { Authentication result = services.autoLogin(new MockHttpServletRequest(), response); assertNull(result); // No cookie set - assertNull(response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY)); + assertNull(response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY)); } @Test @@ -115,12 +116,12 @@ public class TokenBasedRememberMeServicesTests { Authentication result = services.autoLogin(request, response); assertNull(result); - assertNull(response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY)); + assertNull(response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY)); } @Test public void autoLoginReturnsNullForExpiredCookieAndClearsCookie() throws Exception { - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, generateCorrectCookieContentForToken(System.currentTimeMillis() - 1000000, "someone", "password", "key")); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -128,14 +129,14 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @Test public void autoLoginReturnsNullAndClearsCookieIfMissingThreeTokensInCookieValue() throws Exception { - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, new String(Base64.encodeBase64("x".getBytes()))); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -143,14 +144,14 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @Test public void autoLoginClearsNonBase64EncodedCookie() throws Exception { - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, "NOT_BASE_64_ENCODED"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -158,7 +159,7 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @@ -166,7 +167,7 @@ public class TokenBasedRememberMeServicesTests { @Test public void autoLoginClearsCookieIfSignatureBlocksDoesNotMatchExpectedValue() throws Exception { jmock.checking(udsWillReturnUser); - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, generateCorrectCookieContentForToken(System.currentTimeMillis() + 1000000, "someone", "password", "WRONG_KEY")); MockHttpServletRequest request = new MockHttpServletRequest(); @@ -176,14 +177,14 @@ public class TokenBasedRememberMeServicesTests { assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @Test public void autoLoginClearsCookieIfTokenDoesNotContainANumberInCookieValue() throws Exception { - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, new String(Base64.encodeBase64("username:NOT_A_NUMBER:signature".getBytes()))); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -191,7 +192,7 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @@ -199,7 +200,7 @@ public class TokenBasedRememberMeServicesTests { @Test public void autoLoginClearsCookieIfUserNotFound() throws Exception { jmock.checking(udsWillThrowNotFound); - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, generateCorrectCookieContentForToken(System.currentTimeMillis() + 1000000, "someone", "password", "key")); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -208,7 +209,7 @@ public class TokenBasedRememberMeServicesTests { assertNull(services.autoLogin(request, response)); - Cookie returnedCookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie returnedCookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(returnedCookie); assertEquals(0, returnedCookie.getMaxAge()); } @@ -216,7 +217,7 @@ public class TokenBasedRememberMeServicesTests { @Test public void autoLoginWithValidTokenAndUserSucceeds() throws Exception { jmock.checking(udsWillReturnUser); - Cookie cookie = new Cookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, + Cookie cookie = new Cookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY, generateCorrectCookieContentForToken(System.currentTimeMillis() + 1000000, "someone", "password", "key")); MockHttpServletRequest request = new MockHttpServletRequest(); request.setCookies(new Cookie[] {cookie}); @@ -236,7 +237,7 @@ public class TokenBasedRememberMeServicesTests { services.setKey("d"); assertEquals("d", services.getKey()); - assertEquals(TokenBasedRememberMeServices.DEFAULT_PARAMETER, services.getParameter()); + assertEquals(DEFAULT_PARAMETER, services.getParameter()); services.setParameter("some_param"); assertEquals("some_param", services.getParameter()); @@ -250,7 +251,7 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); services.loginFail(request, response); - Cookie cookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie cookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(cookie); assertEquals(0, cookie.getMaxAge()); } @@ -259,12 +260,12 @@ public class TokenBasedRememberMeServicesTests { public void loginSuccessIgnoredIfParameterNotSetOrFalse() { TokenBasedRememberMeServices services = new TokenBasedRememberMeServices(); MockHttpServletRequest request = new MockHttpServletRequest(); - request.addParameter(TokenBasedRememberMeServices.DEFAULT_PARAMETER, "false"); + request.addParameter(DEFAULT_PARAMETER, "false"); MockHttpServletResponse response = new MockHttpServletResponse(); services.loginSuccess(request, response, new TestingAuthenticationToken("someone", "password","ROLE_ABC")); - Cookie cookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie cookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNull(cookie); } @@ -278,7 +279,7 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); services.loginSuccess(request, response, new TestingAuthenticationToken("someone", "password","ROLE_ABC")); - Cookie cookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie cookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); String expiryTime = services.decodeCookie(cookie.getValue())[1]; long expectedExpiryTime = 1000L * 500000000; expectedExpiryTime += System.currentTimeMillis(); @@ -297,17 +298,36 @@ public class TokenBasedRememberMeServicesTests { MockHttpServletResponse response = new MockHttpServletResponse(); services.loginSuccess(request, response, new TestingAuthenticationToken("someone", "password","ROLE_ABC")); - Cookie cookie = response.getCookie(TokenBasedRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + Cookie cookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); assertNotNull(cookie); assertEquals(services.getTokenValiditySeconds(), cookie.getMaxAge()); assertTrue(Base64.isArrayByteBase64(cookie.getValue().getBytes())); assertTrue(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))); } - + // SEC-933 @Test public void obtainPasswordReturnsNullForTokenWithNullCredentials() throws Exception { TestingAuthenticationToken token = new TestingAuthenticationToken("username", null); assertNull(services.retrievePassword(token)); } + + // SEC-949 + @Test + public void negativeValidityPeriodIsSetOnCookieButExpiryTimeRemainsAtTwoWeeks() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter(DEFAULT_PARAMETER, "true"); + + MockHttpServletResponse response = new MockHttpServletResponse(); + services.setTokenValiditySeconds(-1); + services.loginSuccess(request, response, new TestingAuthenticationToken("someone", "password","ROLE_ABC")); + + Cookie cookie = response.getCookie(SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); + assertNotNull(cookie); + // Check the expiry time is within 50ms of two weeks from current time + assertTrue(determineExpiryTimeFromBased64EncodedToken(cookie.getValue()) - System.currentTimeMillis() > + TWO_WEEKS_S - 50); + assertEquals(-1, cookie.getMaxAge()); + assertTrue(Base64.isArrayByteBase64(cookie.getValue().getBytes())); + } }