From e7f94e797764cd28d9b093ccfede1d026f5883f7 Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 11:02:09 -0400 Subject: [PATCH 1/6] Allow the csrf cookie path to be set instead of inferred from the request context --- .../web/csrf/CookieCsrfTokenRepository.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index c275247c729..b50e3358c3b 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -53,6 +53,8 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private boolean cookieHttpOnly; + private String cookiePath; + public CookieCsrfTokenRepository() { this.setHttpOnlyMethod = ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", boolean.class); if (this.setHttpOnlyMethod != null) { @@ -72,7 +74,11 @@ public void saveToken(CsrfToken token, HttpServletRequest request, String tokenValue = token == null ? "" : token.getToken(); Cookie cookie = new Cookie(this.cookieName, tokenValue); cookie.setSecure(request.isSecure()); - cookie.setPath(getCookiePath(request)); + if ( this.cookiePath != null && !this.cookiePath.isEmpty()) { + cookie.setPath(this.cookiePath); + } else { + cookie.setPath(this.getRequestContext(request)); + } if (token == null) { cookie.setMaxAge(0); } @@ -148,7 +154,7 @@ public void setCookieHttpOnly(boolean cookieHttpOnly) { this.cookieHttpOnly = cookieHttpOnly; } - private String getCookiePath(HttpServletRequest request) { + private String getRequestContext(HttpServletRequest request) { String contextPath = request.getContextPath(); return contextPath.length() > 0 ? contextPath : "/"; } @@ -169,4 +175,12 @@ public static CookieCsrfTokenRepository withHttpOnlyFalse() { private String createNewToken() { return UUID.randomUUID().toString(); } -} \ No newline at end of file + + public void setCookiePath(String path) { + this.cookiePath = path; + } + + public void getCookiePath() { + return this.cookiePath; + } +} From 5b118b1651cc6165e25b46790a5950a0ed408a34 Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 11:07:52 -0400 Subject: [PATCH 2/6] Fixing spaces to tabs --- .../web/csrf/CookieCsrfTokenRepository.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index b50e3358c3b..a93122f2219 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -53,7 +53,7 @@ public final class CookieCsrfTokenRepository implements CsrfTokenRepository { private boolean cookieHttpOnly; - private String cookiePath; + private String cookiePath; public CookieCsrfTokenRepository() { this.setHttpOnlyMethod = ReflectionUtils.findMethod(Cookie.class, "setHttpOnly", boolean.class); @@ -74,11 +74,11 @@ public void saveToken(CsrfToken token, HttpServletRequest request, String tokenValue = token == null ? "" : token.getToken(); Cookie cookie = new Cookie(this.cookieName, tokenValue); cookie.setSecure(request.isSecure()); - if ( this.cookiePath != null && !this.cookiePath.isEmpty()) { - cookie.setPath(this.cookiePath); - } else { - cookie.setPath(this.getRequestContext(request)); - } + if ( this.cookiePath != null && !this.cookiePath.isEmpty()) { + cookie.setPath(this.cookiePath); + } else { + cookie.setPath(this.getRequestContext(request)); + } if (token == null) { cookie.setMaxAge(0); } @@ -176,11 +176,11 @@ private String createNewToken() { return UUID.randomUUID().toString(); } - public void setCookiePath(String path) { - this.cookiePath = path; - } - - public void getCookiePath() { - return this.cookiePath; - } + public void setCookiePath(String path) { + this.cookiePath = path; + } + + public void getCookiePath() { + return this.cookiePath; + } } From 9e1415afa3552bd1a9c2cdabd15b53e34166d398 Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 11:13:56 -0400 Subject: [PATCH 3/6] Fixing minor formatting errors Formatting --- .../web/csrf/CookieCsrfTokenRepository.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index a93122f2219..286256119db 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -74,7 +74,7 @@ public void saveToken(CsrfToken token, HttpServletRequest request, String tokenValue = token == null ? "" : token.getToken(); Cookie cookie = new Cookie(this.cookieName, tokenValue); cookie.setSecure(request.isSecure()); - if ( this.cookiePath != null && !this.cookiePath.isEmpty()) { + if (this.cookiePath != null && !this.cookiePath.isEmpty()) { cookie.setPath(this.cookiePath); } else { cookie.setPath(this.getRequestContext(request)); @@ -176,11 +176,11 @@ private String createNewToken() { return UUID.randomUUID().toString(); } - public void setCookiePath(String path) { - this.cookiePath = path; - } + public void setCookiePath(String path) { + this.cookiePath = path; + } - public void getCookiePath() { - return this.cookiePath; - } + public void getCookiePath() { + return this.cookiePath; + } } From 8786c4e14630cf4eb61a03e7baf9e880159f6371 Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 11:26:36 -0400 Subject: [PATCH 4/6] Return type should be string not void --- .../security/web/csrf/CookieCsrfTokenRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index 286256119db..ccdffb67f72 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -180,7 +180,7 @@ public void setCookiePath(String path) { this.cookiePath = path; } - public void getCookiePath() { + public String getCookiePath() { return this.cookiePath; } } From 05a843988814c611ce735c8daf215014387724cb Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 12:44:57 -0400 Subject: [PATCH 5/6] Adding JavaDoc to setCookiePath to note that it will override default functionality and adding tests --- .../web/csrf/CookieCsrfTokenRepository.java | 11 ++++++ .../csrf/CookieCsrfTokenRepositoryTests.java | 39 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java index ccdffb67f72..7b17277c5a7 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/CookieCsrfTokenRepository.java @@ -176,10 +176,21 @@ private String createNewToken() { return UUID.randomUUID().toString(); } + /** + * Set the path that the Cookie will be created with. This will will override the default functionality which uses the + * request context as the path. + * + * @param path the path to use + */ public void setCookiePath(String path) { this.cookiePath = path; } + /** + * Get the path that the CSRF cookie will be set to. + * + * @return the path to be used. + */ public String getCookiePath() { return this.cookiePath; } diff --git a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java index 6a19774fda0..759151d0cf8 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -149,6 +149,45 @@ public void saveTokenWithHttpOnlyFalse() { assertThat(tokenCookie.isHttpOnly()).isFalse(); } + + @Test + public void saveTokenCustomPath() { + String customPath = "/custompath"; + this.repository.setCookiePath(customPath); + CsrfToken token = this.repository.generateToken(this.request); + this.repository.saveToken(token, this.request, this.response); + + Cookie tokenCookie = this.response + .getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); + + assertThat(tokenCookie.getPath()).isEqualTo(this.repository.getCookiePath()); + } + + @Test + public void saveTokenEmptyCustomPath() { + String customPath = ""; + this.repository.setCookiePath(customPath); + CsrfToken token = this.repository.generateToken(this.request); + this.repository.saveToken(token, this.request, this.response); + + Cookie tokenCookie = this.response + .getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); + + assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); + } + + @Test + public void saveTokenNullCustomPath() { + String customPath = null; + this.repository.setCookiePath(customPath); + CsrfToken token = this.repository.generateToken(this.request); + this.repository.saveToken(token, this.request, this.response); + + Cookie tokenCookie = this.response + .getCookie(CookieCsrfTokenRepository.DEFAULT_CSRF_COOKIE_NAME); + + assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); + } @Test public void loadTokenNoCookiesNull() { From bec08d8f8836eca7be0fb647b4c477a9ad0cdbde Mon Sep 17 00:00:00 2001 From: Julio Valcarcel Date: Fri, 16 Sep 2016 13:09:29 -0400 Subject: [PATCH 6/6] Fixing checkstyle violations --- .../csrf/CookieCsrfTokenRepositoryTests.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java index 759151d0cf8..9a66dafb6ef 100644 --- a/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/csrf/CookieCsrfTokenRepositoryTests.java @@ -149,10 +149,10 @@ public void saveTokenWithHttpOnlyFalse() { assertThat(tokenCookie.isHttpOnly()).isFalse(); } - - @Test + + @Test public void saveTokenCustomPath() { - String customPath = "/custompath"; + String customPath = "/custompath"; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -162,10 +162,10 @@ public void saveTokenCustomPath() { assertThat(tokenCookie.getPath()).isEqualTo(this.repository.getCookiePath()); } - - @Test + + @Test public void saveTokenEmptyCustomPath() { - String customPath = ""; + String customPath = ""; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response); @@ -175,10 +175,10 @@ public void saveTokenEmptyCustomPath() { assertThat(tokenCookie.getPath()).isEqualTo(this.request.getContextPath()); } - - @Test + + @Test public void saveTokenNullCustomPath() { - String customPath = null; + String customPath = null; this.repository.setCookiePath(customPath); CsrfToken token = this.repository.generateToken(this.request); this.repository.saveToken(token, this.request, this.response);