Merge pull request #956 from apache/feature/WW-5400-refactor

WW-5400 Simplifies how CspSettings is created
This commit is contained in:
Lukasz Lenart
2024-06-10 07:02:49 +02:00
committed by GitHub
2 changed files with 62 additions and 40 deletions
@@ -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;
@@ -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 {
@@ -57,28 +59,30 @@ public final class CspInterceptor extends AbstractInterceptor {
LOG.trace("Using CspSettings provided by the action: {}", action);
applySettings(invocation, ((CspSettingsAware) action).getCspSettings());
} else {
LOG.trace("Using {} with action: {}", defaultCspSettingsClassName, action);
// 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.");
}
// if defaultCspSettingsClassName does not implement CspSettings, throw an exception
if (!CspSettings.class.isAssignableFrom(Class.forName(defaultCspSettingsClassName))) {
throw new IllegalArgumentException("The defaultCspSettingsClassName must implement CspSettings.");
}
CspSettings cspSettings = (CspSettings) Class.forName(defaultCspSettingsClassName)
.getDeclaredConstructor().newInstance();
LOG.trace("Using {} with action: {}", cspSettingsClassName, action);
CspSettings cspSettings = createCspSettings(invocation);
applySettings(invocation, cspSettings);
}
return invocation.invoke();
}
private CspSettings createCspSettings(ActionInvocation invocation) throws ClassNotFoundException {
Class<?> cspSettingsClass;
try {
cspSettingsClass = ClassLoaderUtil.loadClass(cspSettingsClassName, getClass());
} catch (ClassNotFoundException e) {
throw new ConfigurationException(String.format("The class %s doesn't exist!", cspSettingsClassName));
}
if (!CspSettings.class.isAssignableFrom(cspSettingsClass)) {
throw new ConfigurationException(String.format("The class %s doesn't implement %s!",
cspSettingsClassName, CspSettings.class.getName()));
}
return (CspSettings) invocation.getInvocationContext().getContainer().inject(cspSettingsClass);
}
private void applySettings(ActionInvocation invocation, CspSettings cspSettings) {
HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
HttpServletResponse response = invocation.getInvocationContext().getServletResponse();
@@ -127,7 +131,6 @@ public final class CspInterceptor extends AbstractInterceptor {
* 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) {
@@ -167,7 +170,7 @@ public final class CspInterceptor extends AbstractInterceptor {
*
* @since Struts 6.5.0
*/
public void setDefaultCspSettingsClassName(String defaultCspSettingsClassName) {
this.defaultCspSettingsClassName = defaultCspSettingsClassName;
public void setCspSettingsClassName(String cspSettingsClassName) {
this.cspSettingsClassName = cspSettingsClassName;
}
}
}
@@ -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;
@@ -75,7 +76,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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);
@@ -92,7 +93,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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);
@@ -179,7 +180,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
checkHeader("/report-uri", enforcingMode);
}
public void testInvalidDefaultCspSettingsClassName() throws Exception {
public void testNonExistingCspSettingsClassName() throws Exception {
boolean enforcingMode = true;
mai.setAction(new TestAction());
request.setContextPath("/app");
@@ -189,15 +190,15 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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");
@@ -205,7 +206,25 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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);
@@ -223,9 +242,9 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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)) {
@@ -235,8 +254,7 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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,
@@ -263,10 +281,11 @@ public class CspInterceptorTest extends StrutsInternalTestCase {
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();
}