You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2022/08/23 18:31:47 UTC
[struts] 01/01: WW-5215 Checks is session was already created before applying CSP settings
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5215-csp-session
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 74d4e2371616f4e51fe9986f916271554080ddd1
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Tue Aug 23 20:31:39 2022 +0200
WW-5215 Checks is session was already created before applying CSP settings
---
.../struts2/interceptor/csp/CspInterceptor.java | 5 +-
.../struts2/interceptor/csp/CspSettings.java | 9 +++
.../interceptor/csp/DefaultCspSettings.java | 83 +++++++++++---------
.../struts2/interceptor/CspInterceptorTest.java | 91 ++++++++++++----------
4 files changed, 112 insertions(+), 76 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
index 250179636..ca77436cc 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
@@ -23,6 +23,7 @@ import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
import com.opensymphony.xwork2.interceptor.PreResultListener;
import java.net.URI;
import java.util.Optional;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
/**
@@ -36,6 +37,7 @@ import javax.servlet.http.HttpServletResponse;
* @see DefaultCspSettings
**/
public final class CspInterceptor extends AbstractInterceptor implements PreResultListener {
+
private final CspSettings settings = new DefaultCspSettings();
@Override
@@ -45,8 +47,9 @@ public final class CspInterceptor extends AbstractInterceptor implements PreResu
}
public void beforeResult(ActionInvocation invocation, String resultCode) {
+ HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
- settings.addCspHeaders(response);
+ settings.addCspHeaders(request, response);
}
public void setReportUri(String reportUri) {
diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java b/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java
index 9699ab291..adf5b5072 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/csp/CspSettings.java
@@ -18,6 +18,7 @@
*/
package org.apache.struts2.interceptor.csp;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
/**
@@ -42,9 +43,17 @@ public interface CspSettings {
String HTTPS = "https:";
String CSP_REPORT_TYPE = "application/csp-report";
+ /**
+ * @deprecated use {@link #addCspHeaders(HttpServletRequest, HttpServletResponse)} instead
+ */
+ @Deprecated
void addCspHeaders(HttpServletResponse response);
+
+ void addCspHeaders(HttpServletRequest request, HttpServletResponse response);
+
// sets the uri where csp violation reports will be sent
void setReportUri(String uri);
+
// sets CSP headers in enforcing mode when true, and report-only when false
void setEnforcingMode(boolean value);
}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java b/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java
index 5a99c0a5b..7ab70d226 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java
@@ -18,13 +18,14 @@
*/
package org.apache.struts2.interceptor.csp;
-import com.opensymphony.xwork2.ActionContext;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.security.SecureRandom;
import java.util.Base64;
-import java.util.Map;
-import java.util.function.Supplier;
+import java.util.Objects;
import static java.lang.String.format;
@@ -37,50 +38,61 @@ import static java.lang.String.format;
*/
public class DefaultCspSettings implements CspSettings {
- private final SecureRandom sRand = new SecureRandom();
+ private final static Logger LOG = LogManager.getLogger(DefaultCspSettings.class);
- // this supplier computes a policy format
- private final Supplier<String> lazyPolicyBuilder = new Supplier<String>() {
- @Override
- public String get() {
- StringBuilder policyFormatBuilder = new StringBuilder()
- .append(OBJECT_SRC)
- .append(format(" '%s'; ", NONE))
- .append(SCRIPT_SRC)
- .append(" 'nonce-%s' ") // nonce placeholder
- .append(format("'%s' ", STRICT_DYNAMIC))
- .append(format("%s %s; ", HTTP, HTTPS))
- .append(BASE_URI)
- .append(format(" '%s'; ", NONE));
-
- if (reportUri != null) {
- policyFormatBuilder
- .append(REPORT_URI)
- .append(format(" %s", reportUri));
- }
-
- return format(policyFormatBuilder.toString(), getNonceString());
- }
- };
+ private final SecureRandom sRand = new SecureRandom();
private String reportUri;
// default to reporting mode
private String cspHeader = CSP_REPORT_HEADER;
+ @Override
public void addCspHeaders(HttpServletResponse response) {
- associateNonceWithSession();
- response.setHeader(cspHeader, lazyPolicyBuilder.get());
+ throw new UnsupportedOperationException("Unsupported implementation, use #addCspHeaders(HttpServletRequest request, HttpServletResponse response)");
}
- private String getNonceString() {
- Map<String, Object> session = ActionContext.getContext().getSession();
- return (String) session.get("nonce");
+ public void addCspHeaders(HttpServletRequest request, HttpServletResponse response) {
+ if (isSessionActive(request)) {
+ LOG.debug("Session is active, applying CSP settings");
+ associateNonceWithSession(request);
+ response.setHeader(cspHeader, cratePolicyFormat(request));
+ } else {
+ LOG.debug("Session is not active, ignoring CSP settings");
+ }
}
- private void associateNonceWithSession() {
- Map<String, Object> session = ActionContext.getContext().getSession();
+ private boolean isSessionActive(HttpServletRequest request) {
+ return request.getSession(false) != null;
+ }
+
+ private void associateNonceWithSession(HttpServletRequest request) {
String nonceValue = Base64.getUrlEncoder().encodeToString(getRandomBytes());
- session.put("nonce", nonceValue);
+ request.getSession().setAttribute("nonce", nonceValue);
+ }
+
+ private String cratePolicyFormat(HttpServletRequest request) {
+ StringBuilder policyFormatBuilder = new StringBuilder()
+ .append(OBJECT_SRC)
+ .append(format(" '%s'; ", NONE))
+ .append(SCRIPT_SRC)
+ .append(" 'nonce-%s' ") // nonce placeholder
+ .append(format("'%s' ", STRICT_DYNAMIC))
+ .append(format("%s %s; ", HTTP, HTTPS))
+ .append(BASE_URI)
+ .append(format(" '%s'; ", NONE));
+
+ if (reportUri != null) {
+ policyFormatBuilder
+ .append(REPORT_URI)
+ .append(format(" %s", reportUri));
+ }
+
+ return format(policyFormatBuilder.toString(), getNonceString(request));
+ }
+
+ private String getNonceString(HttpServletRequest request) {
+ Object nonce = request.getSession().getAttribute("nonce");
+ return Objects.toString(nonce);
}
private byte[] getRandomBytes() {
@@ -98,4 +110,5 @@ public class DefaultCspSettings implements CspSettings {
public void setReportUri(String reportUri) {
this.reportUri = reportUri;
}
+
}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
index 504be8bb4..a9ee1be11 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
@@ -21,16 +21,25 @@ package org.apache.struts2.interceptor;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import org.apache.logging.log4j.util.Strings;
-import org.apache.struts2.ServletActionContext;
import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.dispatcher.SessionMap;
import org.apache.struts2.interceptor.csp.CspInterceptor;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
-import java.util.HashMap;
-import java.util.Map;
+import javax.servlet.http.HttpSession;
-import static org.apache.struts2.interceptor.csp.CspSettings.*;
+import static org.apache.struts2.interceptor.csp.CspSettings.BASE_URI;
+import static org.apache.struts2.interceptor.csp.CspSettings.CSP_ENFORCE_HEADER;
+import static org.apache.struts2.interceptor.csp.CspSettings.CSP_REPORT_HEADER;
+import static org.apache.struts2.interceptor.csp.CspSettings.HTTP;
+import static org.apache.struts2.interceptor.csp.CspSettings.HTTPS;
+import static org.apache.struts2.interceptor.csp.CspSettings.NONE;
+import static org.apache.struts2.interceptor.csp.CspSettings.OBJECT_SRC;
+import static org.apache.struts2.interceptor.csp.CspSettings.REPORT_URI;
+import static org.apache.struts2.interceptor.csp.CspSettings.SCRIPT_SRC;
+import static org.apache.struts2.interceptor.csp.CspSettings.STRICT_DYNAMIC;
+import static org.junit.Assert.assertNotEquals;
public class CspInterceptorTest extends StrutsInternalTestCase {
@@ -38,7 +47,8 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
private final MockActionInvocation mai = new MockActionInvocation();
private final MockHttpServletRequest request = new MockHttpServletRequest();
private final MockHttpServletResponse response = new MockHttpServletResponse();
- private final Map<String, Object> session = new HashMap<>();
+
+ private HttpSession session;
public void test_whenRequestReceived_thenNonceIsSetInSession_andCspHeaderContainsIt() throws Exception {
String reportUri = "/barfoo";
@@ -48,8 +58,8 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
interceptor.intercept(mai);
- assertTrue("Nonce key does not exist", session.containsKey("nonce"));
- assertFalse("Nonce value is empty", Strings.isEmpty((String) session.get("nonce")));
+ assertNotNull("Nonce key does not exist", session.getAttribute("nonce"));
+ assertFalse("Nonce value is empty", Strings.isEmpty((String) session.getAttribute("nonce")));
checkHeader(reportUri, reporting);
}
@@ -58,13 +68,13 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
String enforcingMode = "true";
interceptor.setReportUri(reportUri);
interceptor.setEnforcingMode(enforcingMode);
- session.put("nonce", "foo");
+ session.setAttribute("nonce", "foo");
interceptor.intercept(mai);
- assertTrue("Nonce key does not exist", session.containsKey("nonce"));
- assertFalse("Nonce value is empty", Strings.isEmpty((String) session.get("nonce")));
- assertFalse("New nonce value couldn't be set", session.get("nonce").equals("foo"));
+ assertNotNull("Nonce key does not exist", session.getAttribute("nonce"));
+ assertFalse("Nonce value is empty", Strings.isEmpty((String) session.getAttribute("nonce")));
+ assertNotEquals("New nonce value couldn't be set", "foo", session.getAttribute("nonce"));
checkHeader(reportUri, enforcingMode);
}
@@ -73,13 +83,13 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
String enforcingMode = "true";
interceptor.setReportUri(reportUri);
interceptor.setEnforcingMode(enforcingMode);
- session.put("nonce", "foo");
+ session.setAttribute("nonce", "foo");
interceptor.intercept(mai);
- assertTrue("Nonce key does not exist", session.containsKey("nonce"));
- assertFalse("Nonce value is empty", Strings.isEmpty((String) session.get("nonce")));
- assertFalse("New nonce value couldn't be set", session.get("nonce").equals("foo"));
+ assertNotNull("Nonce key does not exist", session.getAttribute("nonce"));
+ assertFalse("Nonce value is empty", Strings.isEmpty((String) session.getAttribute("nonce")));
+ assertNotEquals("New nonce value couldn't be set", "foo", session.getAttribute("nonce"));
checkHeader(reportUri, enforcingMode);
}
@@ -88,13 +98,12 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
String enforcingMode = "false";
interceptor.setReportUri(reportUri);
interceptor.setEnforcingMode(enforcingMode);
- session.put("nonce", "foo");
+ session.setAttribute("nonce", "foo");
interceptor.intercept(mai);
- assertTrue("Nonce key does not exist", session.containsKey("nonce"));
- assertFalse("Nonce value is empty", Strings.isEmpty((String) session.get("nonce")));
- assertFalse("New nonce value couldn't be set", session.get("nonce").equals("foo"));
+ assertNotNull("Nonce value is empty", session.getAttribute("nonce"));
+ assertNotEquals("New nonce value couldn't be set", "foo", session.getAttribute("nonce"));
checkHeader(reportUri, enforcingMode);
}
@@ -116,11 +125,11 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
String enforcingMode = "false";
interceptor.setEnforcingMode(enforcingMode);
- try{
+ try {
interceptor.setReportUri("ww w. google.@com");
- assert(false);
- } catch (IllegalArgumentException e){
- assert(true);
+ assert (false);
+ } catch (IllegalArgumentException e) {
+ assert (true);
}
}
@@ -128,33 +137,33 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
String enforcingMode = "false";
interceptor.setEnforcingMode(enforcingMode);
- try{
+ try {
interceptor.setReportUri("some-uri");
- assert(false);
- } catch (IllegalArgumentException e){
- assert(true);
+ assert (false);
+ } catch (IllegalArgumentException e) {
+ assert (true);
}
}
- public void checkHeader(String reportUri, String enforcingMode){
+ public void checkHeader(String reportUri, String enforcingMode) {
String expectedCspHeader = "";
if (Strings.isEmpty(reportUri)) {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; ",
- OBJECT_SRC, NONE,
- SCRIPT_SRC, session.get("nonce"), STRICT_DYNAMIC, HTTP, HTTPS,
- BASE_URI, NONE
+ OBJECT_SRC, NONE,
+ SCRIPT_SRC, session.getAttribute("nonce"), STRICT_DYNAMIC, HTTP, HTTPS,
+ BASE_URI, NONE
);
} else {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; %s %s",
- OBJECT_SRC, NONE,
- SCRIPT_SRC, session.get("nonce"), STRICT_DYNAMIC, HTTP, HTTPS,
- BASE_URI, NONE,
- REPORT_URI, reportUri
+ OBJECT_SRC, NONE,
+ SCRIPT_SRC, session.getAttribute("nonce"), STRICT_DYNAMIC, HTTP, HTTPS,
+ BASE_URI, NONE,
+ REPORT_URI, reportUri
);
}
String header = "";
- if (enforcingMode.equals("true")){
+ if (enforcingMode.equals("true")) {
header = response.getHeader(CSP_ENFORCE_HEADER);
} else {
header = response.getHeader(CSP_REPORT_HEADER);
@@ -168,10 +177,12 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
protected void setUp() throws Exception {
super.setUp();
container.inject(interceptor);
- ServletActionContext.setRequest(request);
- ServletActionContext.setResponse(response);
- ActionContext context = ServletActionContext.getActionContext().bind();
- context.withSession(session);
+ ActionContext context = ActionContext.getContext()
+ .withServletRequest(request)
+ .withServletResponse(response)
+ .withSession(new SessionMap<>(request))
+ .bind();
mai.setInvocationContext(context);
+ session = request.getSession();
}
}