From 39e84013f79486bb5f22eabcb7e4068d6bd37b26 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 3 Sep 2019 15:57:10 -0600 Subject: [PATCH] ClearSiteDataHeaderWriter Directives Fixes gh-7347 --- .../writers/ClearSiteDataHeaderWriter.java | 44 +++++++++++++------ .../ClearSiteDataServerHttpHeadersWriter.java | 6 +-- .../ClearSiteDataHeaderWriterTests.java | 12 +++-- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriter.java b/web/src/main/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriter.java index 7669e67aab..3b125edd1e 100644 --- a/web/src/main/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriter.java +++ b/web/src/main/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriter.java @@ -21,6 +21,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.security.web.header.HeaderWriter; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -60,13 +61,13 @@ public final class ClearSiteDataHeaderWriter implements HeaderWriter { * the request is secure as per the Incomplete Clearing section. *

* - * @param sources (i.e. "cache", "cookies", "storage", "executionContexts" or "*") + * @param directives (i.e. "cache", "cookies", "storage", "executionContexts" or "*") * @throws {@link IllegalArgumentException} if sources is null or empty. */ - public ClearSiteDataHeaderWriter(String ...sources) { - Assert.notEmpty(sources, "sources cannot be empty or null"); + public ClearSiteDataHeaderWriter(Directive... directives) { + Assert.notEmpty(directives, "directives cannot be empty or null"); this.requestMatcher = new SecureRequestMatcher(); - this.headerValue = joinQuotes(sources); + this.headerValue = transformToHeaderValue(directives); } @Override @@ -81,12 +82,33 @@ public final class ClearSiteDataHeaderWriter implements HeaderWriter { } } - private String joinQuotes(String ...sources) { - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < sources.length-1; i++) { - sb.append(quote(sources[i])).append(", "); + /** + *

Represents the directive values expected by the {@link ClearSiteDataHeaderWriter}

. + */ + public enum Directive { + CACHE("cache"), + COOKIES("cookies"), + STORAGE("storage"), + EXECUTION_CONTEXTS("executionContexts"), + ALL("*"); + + private final String headerValue; + + Directive(String headerValue) { + this.headerValue = "\"" + headerValue + "\""; } - sb.append(quote(sources[sources.length-1])); + + public String getHeaderValue() { + return this.headerValue; + } + } + + private String transformToHeaderValue(Directive... directives) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < directives.length - 1; i++) { + sb.append(directives[i].headerValue).append(", "); + } + sb.append(directives[directives.length - 1].headerValue); return sb.toString(); } @@ -96,10 +118,6 @@ public final class ClearSiteDataHeaderWriter implements HeaderWriter { } } - private String quote(String source) { - return "\"" + source + "\""; - } - @Override public String toString() { return getClass().getName() + " [headerValue=" + this.headerValue + "]"; diff --git a/web/src/main/java/org/springframework/security/web/server/header/ClearSiteDataServerHttpHeadersWriter.java b/web/src/main/java/org/springframework/security/web/server/header/ClearSiteDataServerHttpHeadersWriter.java index f0195ac4c6..69049c1d45 100644 --- a/web/src/main/java/org/springframework/security/web/server/header/ClearSiteDataServerHttpHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/server/header/ClearSiteDataServerHttpHeadersWriter.java @@ -15,11 +15,11 @@ */ package org.springframework.security.web.server.header; +import reactor.core.publisher.Mono; + import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; -import reactor.core.publisher.Mono; - /** *

Writes the {@code Clear-Site-Data} response header when the request is secure.

* @@ -40,7 +40,7 @@ public final class ClearSiteDataServerHttpHeadersWriter implements ServerHttpHea * @throws IllegalArgumentException if the argument is null or empty */ public ClearSiteDataServerHttpHeadersWriter(Directive... directives) { - Assert.notEmpty(directives, "directives cannot be empty or null."); + Assert.notEmpty(directives, "directives cannot be empty or null"); this.headerWriterDelegate = StaticServerHttpHeadersWriter.builder() .header(CLEAR_SITE_DATA_HEADER, transformToHeaderValue(directives)) .build(); diff --git a/web/src/test/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriterTests.java b/web/src/test/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriterTests.java index 1c6348a169..e1d6f2b003 100644 --- a/web/src/test/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/header/writers/ClearSiteDataHeaderWriterTests.java @@ -25,6 +25,10 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter.Directive.CACHE; +import static org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter.Directive.COOKIES; +import static org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter.Directive.EXECUTION_CONTEXTS; +import static org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter.Directive.STORAGE; /** * @@ -52,7 +56,7 @@ public class ClearSiteDataHeaderWriterTests { @Test public void createInstanceWhenMissingSourceThenThrowsException() { this.thrown.expect(Exception.class); - this.thrown.expectMessage("sources cannot be empty or null"); + this.thrown.expectMessage("directives cannot be empty or null"); new ClearSiteDataHeaderWriter(); } @@ -60,7 +64,7 @@ public class ClearSiteDataHeaderWriterTests { @Test public void writeHeaderWhenRequestNotSecureThenHeaderIsNotPresent() { this.request.setSecure(false); - ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter("cache"); + ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter(CACHE); headerWriter.writeHeaders(this.request, this.response); assertThat(this.response.getHeader(HEADER_NAME)).isNull(); @@ -68,7 +72,7 @@ public class ClearSiteDataHeaderWriterTests { @Test public void writeHeaderWhenRequestIsSecureThenHeaderValueMatchesPassedSource() { - ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter("storage"); + ClearSiteDataHeaderWriter headerWriter = new ClearSiteDataHeaderWriter(STORAGE); headerWriter.writeHeaders(this.request, this.response); assertThat(this.response.getHeader(HEADER_NAME)).isEqualTo("\"storage\""); @@ -77,7 +81,7 @@ public class ClearSiteDataHeaderWriterTests { @Test public void writeHeaderWhenRequestIsSecureThenHeaderValueMatchesPassedSources() { ClearSiteDataHeaderWriter headerWriter = - new ClearSiteDataHeaderWriter("cache", "cookies", "storage", "executionContexts"); + new ClearSiteDataHeaderWriter(CACHE, COOKIES, STORAGE, EXECUTION_CONTEXTS); headerWriter.writeHeaders(this.request, this.response); assertThat(this.response.getHeader(HEADER_NAME))