Skip to content

Commit

Permalink
Merge pull request apache#1060 from apache/fix/WW-5297-invalid-session
Browse files Browse the repository at this point in the history 
WW-5297 Fixes checking nonce of invalidated session
  • Loading branch information
@lukaszlenart
lukaszlenart authored Oct 1, 2024
2 parents 48c4e3b + 0bd4266 commit 75cc664
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 35 deletions.
8 changes: 6 additions & 2 deletions core/src/main/java/org/apache/struts2/components/UIBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import java.io.Writer;
import java.util.HashMap;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -863,10 +864,13 @@ public void evaluateParams() {
}

// to be used with the CSP interceptor - adds the nonce value as a parameter to be accessed from ftl files
Map<String, Object> session = stack.getActionContext().getSession();
Object nonceValue = session != null ? session.get("nonce") : null;
HttpSession session = stack.getActionContext().getServletRequest().getSession(false);
Object nonceValue = session != null ? session.getAttribute("nonce") : null;

if (nonceValue != null) {
addParameter("nonce", nonceValue.toString());
} else {
LOG.debug("Session is not active, cannot obtain nonce value");
}

evaluateExtraParams();
Expand Down
43 changes: 38 additions & 5 deletions core/src/test/java/org/apache/struts2/components/UIBeanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
import org.apache.struts2.components.template.Template;
import org.apache.struts2.components.template.TemplateEngine;
import org.apache.struts2.components.template.TemplateEngineManager;
import org.apache.struts2.dispatcher.SessionMap;
import org.apache.struts2.dispatcher.StaticContentLoader;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.mock.web.MockHttpSession;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsCheckerTest.NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER;
Expand Down Expand Up @@ -160,7 +161,7 @@ public TemplateEngine getTemplateEngine(Template template, String templateTypeOv
try {
txtFld.mergeTemplate(null, new Template(null, null, null));
fail("Exception not thrown");
} catch(final Exception e){
} catch (final Exception e) {
assertTrue(e instanceof ConfigurationException);
}
}
Expand Down Expand Up @@ -225,6 +226,7 @@ public void testSetAccesskey() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

TextField txtFld = new TextField(stack, req, res);
txtFld.setAccesskey(accesskeyValue);
Expand All @@ -238,6 +240,7 @@ public void testValueParameterEvaluation() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

TextField txtFld = new TextField(stack, req, res);
txtFld.addParameter("value", value);
Expand All @@ -250,11 +253,13 @@ public void testValueParameterRecursion() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

stack.push(new Object() {
public String getMyValue() {
return "%{myBad}";
}

public String getMyBad() {
throw new IllegalStateException("Recursion detected!");
}
Expand All @@ -273,11 +278,13 @@ public void testValueNameParameterNotAccepted() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

stack.push(new Object() {
public String getMyValueName() {
return "getMyValue()";
}

public String getMyValue() {
return "value";
}
Expand All @@ -300,6 +307,7 @@ public void testValueNameParameterGetterAccepted() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

stack.push(new Object() {
public String getMyValue() {
Expand All @@ -320,6 +328,7 @@ public void testSetClass() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

TextField txtFld = new TextField(stack, req, res);
txtFld.setCssClass(cssClass);
Expand All @@ -333,6 +342,7 @@ public void testSetStyle() {
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext.getContext().withServletRequest(req);

TextField txtFld = new TextField(stack, req, res);
txtFld.setStyle(cssStyle);
Expand All @@ -347,16 +357,39 @@ public void testNonce() {
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext actionContext = stack.getActionContext();
Map<String, Object> session = new HashMap<>();
session.put("nonce", nonceVal);
actionContext.withSession(session);
actionContext.withServletRequest(req);
MockHttpSession session = new MockHttpSession();
session.putValue("nonce", nonceVal);
req.setSession(session);

actionContext.withSession(new SessionMap(req));

DoubleSelect dblSelect = new DoubleSelect(stack, req, res);
dblSelect.evaluateParams();

assertEquals(nonceVal, dblSelect.getParameters().get("nonce"));
}

public void testNonceOfInvalidSession() {
String nonceVal = "r4nd0m";
ValueStack stack = ActionContext.getContext().getValueStack();
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
ActionContext actionContext = stack.getActionContext();
actionContext.withServletRequest(req);
MockHttpSession session = new MockHttpSession();
session.putValue("nonce", nonceVal);
req.setSession(session);
actionContext.withSession(new SessionMap(req));

session.invalidate();

DoubleSelect dblSelect = new DoubleSelect(stack, req, res);
dblSelect.evaluateParams();

assertNull(dblSelect.getParameters().get("nonce"));
}

public void testSetNullUiStaticContentPath() {
// given
ValueStack stack = ActionContext.getContext().getValueStack();
Expand Down
11 changes: 11 additions & 0 deletions plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AbstractTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import java.io.StringWriter;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -51,6 +52,8 @@ public abstract class AbstractTest extends TestCase {
private final Map<String, String> commonAttrs = new HashMap<>();
private final Map<String, String> dynamicAttrs = new HashMap<>();

protected static final String NONCE_VAL = "r4andom";

protected SimpleTheme theme;

protected StringWriter writer;
Expand All @@ -62,6 +65,7 @@ public abstract class AbstractTest extends TestCase {
protected TemplateRenderingContext context;
protected HttpServletRequest request;
protected HttpServletResponse response;
private HttpSession session;

protected abstract UIBean getUIBean() throws Exception;

Expand Down Expand Up @@ -107,6 +111,12 @@ protected void setUp() throws Exception {
expect(request.getContextPath()).andReturn("/some/path").anyTimes();
response = createNiceMock(HttpServletResponse.class);

session = createNiceMock(HttpSession.class);
expect(session.getAttribute("nonce")).andReturn(NONCE_VAL).anyTimes();
expect(request.getSession(false)).andReturn(session).anyTimes();

actionContext.withServletRequest(request);

expect(stack.getActionContext()).andReturn(actionContext).anyTimes();
expect(stack.getContext()).andReturn(stackContext).anyTimes();

Expand All @@ -116,6 +126,7 @@ protected void setUp() throws Exception {
TextParser parser = new OgnlTextParser();
expect(container.getInstance(TextParser.class)).andReturn(parser).anyTimes();

replay(session);
replay(request);
replay(stack);
replay(container);
Expand Down
2 changes: 1 addition & 1 deletion plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/HeadTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void testRenderTextField() {
map.putAll(tag.getParameters());
theme.renderTag(getTagName(), context);
String output = writer.getBuffer().toString();
String expected = s("<script type='text/javascript' base='/some/path' src='/some/path/static/utils.js'></script>");
String expected = s("<script type='text/javascript' base='/some/path' src='/some/path/static/utils.js' nonce='r4andom'></script>");
assertEquals(expected, output);
}

Expand Down
18 changes: 3 additions & 15 deletions plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/LinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,13 @@
*/
package org.apache.struts2.views.java.simple;

import com.opensymphony.xwork2.ActionContext;
import org.apache.struts2.components.Link;
import org.apache.struts2.components.UIBean;

import java.util.HashMap;
import java.util.Map;

public class LinkTest extends AbstractTest{
public class LinkTest extends AbstractTest {

private Link tag;

private static final String NONCE_VAL = "r4andom";

public void testRenderLinkTag() {
tag.setHref("testhref");
tag.setHreflang("test");
Expand Down Expand Up @@ -60,7 +54,7 @@ public void testRenderLinkTag() {
assertTrue("Incorrect as attribute for link tag", output.contains(s("as='test'")));
assertFalse("Non-existent disabled attribute for link tag", output.contains(s("disabled='disabled'")));
assertTrue("Incorrect title attribute for link tag", output.contains(s("title='test'")));
assertTrue("Incorrect nonce attribute for link tag", output.contains(s("nonce='" + NONCE_VAL+"'")));
assertTrue("Incorrect nonce attribute for link tag", output.contains(s("nonce='" + NONCE_VAL + "'")));
}

public void testRenderLinkTagAsStylesheet() {
Expand Down Expand Up @@ -92,7 +86,7 @@ public void testRenderLinkTagAsStylesheet() {
assertTrue("Incorrect as attribute for link tag", output.contains(s("as='test'")));
assertTrue("Incorrect disabled attribute for link tag", output.contains(s("disabled='disabled'")));
assertTrue("Incorrect title attribute for link tag", output.contains(s("title='test'")));
assertTrue("Incorrect nonce attribute for link tag", output.contains(s("nonce='" + NONCE_VAL+"'")));
assertTrue("Incorrect nonce attribute for link tag", output.contains(s("nonce='" + NONCE_VAL + "'")));
}

@Override
Expand All @@ -108,12 +102,6 @@ protected String getTagName() {
@Override
protected void setUp() throws Exception {
super.setUp();

ActionContext actionContext = stack.getActionContext();
Map<String, Object> session = new HashMap<>();
session.put("nonce", NONCE_VAL);
actionContext.withSession(session);

this.tag = new Link(stack, request, response);
}
}
12 changes: 0 additions & 12 deletions plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,15 @@
*/
package org.apache.struts2.views.java.simple;

import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker;
import org.apache.struts2.components.Script;
import org.apache.struts2.components.UIBean;


import java.util.HashMap;
import java.util.Map;


public class ScriptTest extends AbstractTest {

private Script tag;

private static final String NONCE_VAL = "r4andom";

public void testRenderScriptTag() {
tag.setName("name_");
tag.setType("text/javascript");
Expand Down Expand Up @@ -77,11 +70,6 @@ protected String getTagName() {
protected void setUp() throws Exception {
super.setUp();

ActionContext actionContext = stack.getActionContext();
Map<String, Object> session = new HashMap<>();
session.put("nonce", NONCE_VAL);
actionContext.withSession(session);

this.tag = new Script(stack, request, response);
tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker());
}
Expand Down

0 comments on commit 75cc664

Please sign in to comment.