Skip to content

Commit

Permalink
Merge pull request #956 from apache/feature/WW-5400-refactor
Browse files Browse the repository at this point in the history 
WW-5400 Simplifies how CspSettings is created
  • Loading branch information
@lukaszlenart
lukaszlenart authored Jun 10, 2024
2 parents cabc076 + 4a05653 commit cf34f0d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 39 deletions.
45 changes: 24 additions & 21 deletions core/src/main/java/org/apache/struts2/interceptor/csp/CspInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
package org.apache.struts2.interceptor.csp;

import com.opensymphony.xwork2.ActionInvocation;
import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
import com.opensymphony.xwork2.util.ClassLoaderUtil;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.action.CspSettingsAware;
Expand Down Expand Up @@ -48,7 +50,7 @@ public final class CspInterceptor extends AbstractInterceptor {
private String reportUri;
private String reportTo;

private String defaultCspSettingsClassName = DefaultCspSettings.class.getName();
private String cspSettingsClassName = DefaultCspSettings.class.getName();

@Override
public String intercept(ActionInvocation invocation) throws Exception {
Expand All @@ -57,26 +59,28 @@ public String intercept(ActionInvocation invocation) throws Exception {
LOG.trace("Using CspSettings provided by the action: {}", action);
applySettings(invocation, ((CspSettingsAware) action).getCspSettings());
} else {
LOG.trace("Using {} with action: {}", defaultCspSettingsClassName, action);
LOG.trace("Using {} with action: {}", cspSettingsClassName, action);
CspSettings cspSettings = createCspSettings(invocation);
applySettings(invocation, cspSettings);
}
return invocation.invoke();
}

// if the defaultCspSettingsClassName is not a real class, throw an exception
try {
Class.forName(defaultCspSettingsClassName, false, Thread.currentThread().getContextClassLoader());
}
catch (ClassNotFoundException e) {
throw new IllegalArgumentException("The defaultCspSettingsClassName must be a real class.");
}
private CspSettings createCspSettings(ActionInvocation invocation) throws ClassNotFoundException {
Class<?> cspSettingsClass;

// if defaultCspSettingsClassName does not implement CspSettings, throw an exception
if (!CspSettings.class.isAssignableFrom(Class.forName(defaultCspSettingsClassName))) {
throw new IllegalArgumentException("The defaultCspSettingsClassName must implement CspSettings.");
}
try {
cspSettingsClass = ClassLoaderUtil.loadClass(cspSettingsClassName, getClass());
} catch (ClassNotFoundException e) {
throw new ConfigurationException(String.format("The class %s doesn't exist!", cspSettingsClassName));
}

CspSettings cspSettings = (CspSettings) Class.forName(defaultCspSettingsClassName)
.getDeclaredConstructor().newInstance();
applySettings(invocation, cspSettings);
if (!CspSettings.class.isAssignableFrom(cspSettingsClass)) {
throw new ConfigurationException(String.format("The class %s doesn't implement %s!",
cspSettingsClassName, CspSettings.class.getName()));
}
return invocation.invoke();

return (CspSettings) invocation.getInvocationContext().getContainer().inject(cspSettingsClass);
}

private void applySettings(ActionInvocation invocation, CspSettings cspSettings) {
Expand Down Expand Up @@ -127,7 +131,6 @@ public void setReportUri(String reportUri) {
* only be used if the reportUri is set.
*
* @param reportTo the report group where csp violation reports will be sent
*
* @since Struts 6.5.0
*/
public void setReportTo(String reportTo) {
Expand Down Expand Up @@ -167,7 +170,7 @@ public void setPrependServletContext(boolean prependServletContext) {
*
* @since Struts 6.5.0
*/
public void setDefaultCspSettingsClassName(String defaultCspSettingsClassName) {
this.defaultCspSettingsClassName = defaultCspSettingsClassName;
public void setCspSettingsClassName(String cspSettingsClassName) {
this.cspSettingsClassName = cspSettingsClassName;
}
}
}
55 changes: 37 additions & 18 deletions core/src/test/java/org/apache/struts2/interceptor/CspInterceptorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.struts2.interceptor;

import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import org.apache.logging.log4j.util.Strings;
import org.apache.struts2.StrutsInternalTestCase;
Expand Down Expand Up @@ -75,7 +76,7 @@ public void test_whenNonceAlreadySetInSession_andRequestReceived_thenNewNonceIsS

public void testEnforcingCspHeadersSet() throws Exception {
String reportUri = "/csp-reports";
String reportTo = "csp-group";
String reportTo = "csp-group";
boolean enforcingMode = true;
interceptor.setReportUri(reportUri);
interceptor.setReportTo(reportTo);
Expand All @@ -92,7 +93,7 @@ public void testEnforcingCspHeadersSet() throws Exception {

public void testReportingCspHeadersSet() throws Exception {
String reportUri = "/csp-reports";
String reportTo = "csp-group";
String reportTo = "csp-group";
boolean enforcingMode = false;
interceptor.setReportUri(reportUri);
interceptor.setReportTo(reportTo);
Expand Down Expand Up @@ -179,7 +180,7 @@ public void testNoPrependContext() throws Exception {
checkHeader("/report-uri", enforcingMode);
}

public void testInvalidDefaultCspSettingsClassName() throws Exception {
public void testNonExistingCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");
Expand All @@ -189,23 +190,41 @@ public void testInvalidDefaultCspSettingsClassName() throws Exception {
interceptor.setPrependServletContext(false);

try {
interceptor.setDefaultCspSettingsClassName("foo");
interceptor.setCspSettingsClassName("foo");
interceptor.intercept(mai);
assert (false);
} catch (IllegalArgumentException e) {
assert (true);
fail("Expected exception");
} catch (ConfigurationException e) {
assertEquals("The class foo doesn't exist!", e.getMessage());
}
}

public void testCustomDefaultCspSettingsClassName() throws Exception {
public void testInvalidCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");

interceptor.setEnforcingMode(enforcingMode);
interceptor.setReportUri("/report-uri");
interceptor.setPrependServletContext(false);
interceptor.setDefaultCspSettingsClassName(CustomDefaultCspSettings.class.getName());

try {
interceptor.setCspSettingsClassName(Integer.class.getName());
interceptor.intercept(mai);
fail("Expected exception");
} catch (ConfigurationException e) {
assertEquals("The class java.lang.Integer doesn't implement org.apache.struts2.interceptor.csp.CspSettings!", e.getMessage());
}
}

public void testCustomCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");

interceptor.setEnforcingMode(enforcingMode);
interceptor.setReportUri("/report-uri");
interceptor.setPrependServletContext(false);
interceptor.setCspSettingsClassName(CustomDefaultCspSettings.class.getName());

interceptor.intercept(mai);

Expand All @@ -223,9 +242,9 @@ public void checkHeader(String reportUri, String reportTo, boolean enforcingMode
String expectedCspHeader;
if (Strings.isEmpty(reportUri)) {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; ",
CspSettings.OBJECT_SRC, CspSettings.NONE,
CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
CspSettings.BASE_URI, CspSettings.NONE
CspSettings.OBJECT_SRC, CspSettings.NONE,
CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
CspSettings.BASE_URI, CspSettings.NONE
);
} else {
if (Strings.isEmpty(reportTo)) {
Expand All @@ -235,8 +254,7 @@ public void checkHeader(String reportUri, String reportTo, boolean enforcingMode
CspSettings.BASE_URI, CspSettings.NONE,
CspSettings.REPORT_URI, reportUri
);
}
else {
} else {
expectedCspHeader = String.format("%s '%s'; %s 'nonce-%s' '%s' %s %s; %s '%s'; %s %s; %s %s; ",
CspSettings.OBJECT_SRC, CspSettings.NONE,
CspSettings.SCRIPT_SRC, session.getAttribute("nonce"), CspSettings.STRICT_DYNAMIC, CspSettings.HTTP, CspSettings.HTTPS,
Expand All @@ -263,10 +281,11 @@ protected void setUp() throws Exception {
super.setUp();
container.inject(interceptor);
ActionContext context = ActionContext.getContext()
.withServletRequest(request)
.withServletResponse(response)
.withSession(new SessionMap(request))
.bind();
.withContainer(container)
.withServletRequest(request)
.withServletResponse(response)
.withSession(new SessionMap(request))
.bind();
mai.setInvocationContext(context);
session = request.getSession();
}
Expand Down

0 comments on commit cf34f0d

Please sign in to comment.