From 0bd4266d2f263e235d091613e283590a092bbe5f Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 23 Sep 2024 19:58:06 +0200 Subject: [PATCH] WW-5297 Fixes checking nonce of invalidated session --- .../org/apache/struts2/components/UIBean.java | 8 +++- .../apache/struts2/components/UIBeanTest.java | 43 ++++++++++++++++--- .../views/java/simple/AbstractTest.java | 11 +++++ .../struts2/views/java/simple/HeadTest.java | 2 +- .../struts2/views/java/simple/LinkTest.java | 18 ++------ .../struts2/views/java/simple/ScriptTest.java | 12 ------ 6 files changed, 59 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/UIBean.java b/core/src/main/java/org/apache/struts2/components/UIBean.java index 59d3713edb..c787fd1002 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -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; @@ -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 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(); diff --git a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java index 6903289421..1bff068893 100644 --- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java +++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java @@ -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; @@ -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); } } @@ -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); @@ -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); @@ -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!"); } @@ -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"; } @@ -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() { @@ -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); @@ -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); @@ -347,9 +357,12 @@ public void testNonce() { MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); ActionContext actionContext = stack.getActionContext(); - Map 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(); @@ -357,6 +370,26 @@ public void testNonce() { 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(); diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AbstractTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AbstractTest.java index 3c6c37f97f..582b296c2e 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AbstractTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AbstractTest.java @@ -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; @@ -51,6 +52,8 @@ public abstract class AbstractTest extends TestCase { private final Map commonAttrs = new HashMap<>(); private final Map dynamicAttrs = new HashMap<>(); + protected static final String NONCE_VAL = "r4andom"; + protected SimpleTheme theme; protected StringWriter writer; @@ -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; @@ -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(); @@ -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); diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/HeadTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/HeadTest.java index 073da1f2b6..e116b3df40 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/HeadTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/HeadTest.java @@ -31,7 +31,7 @@ public void testRenderTextField() { map.putAll(tag.getParameters()); theme.renderTag(getTagName(), context); String output = writer.getBuffer().toString(); - String expected = s(""); + String expected = s(""); assertEquals(expected, output); } diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/LinkTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/LinkTest.java index 873ede2696..c8c9c414a2 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/LinkTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/LinkTest.java @@ -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"); @@ -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() { @@ -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 @@ -108,12 +102,6 @@ protected String getTagName() { @Override protected void setUp() throws Exception { super.setUp(); - - ActionContext actionContext = stack.getActionContext(); - Map session = new HashMap<>(); - session.put("nonce", NONCE_VAL); - actionContext.withSession(session); - this.tag = new Link(stack, request, response); } } diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java index 832653692e..b37c76ec8b 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java @@ -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"); @@ -77,11 +70,6 @@ protected String getTagName() { protected void setUp() throws Exception { super.setUp(); - ActionContext actionContext = stack.getActionContext(); - Map session = new HashMap<>(); - session.put("nonce", NONCE_VAL); - actionContext.withSession(session); - this.tag = new Script(stack, request, response); tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker()); }