This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch wicket-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 00ded8b78a4895e25f1ecffe68d43b639abfecfd Author: Emond Papegaaij <[email protected]> AuthorDate: Thu Aug 6 15:47:59 2026 +0200 Distinguish page renders from listener invocations in resource isolation A resource isolation policy could not tell what a request was going to do with the targeted page, because it only received the page itself. That distinction is what the Fetch Metadata policy needs: a page may legitimately be rendered as the result of a top-level navigation from another site, but a listener on that page must not be invoked from another site. Wicket invokes listeners through ordinary GET navigations, so the headers alone do not separate the two. IResourceIsolationPolicy gains a RequestType and an isRequestAllowed overload that receives it. The existing two-argument method remains the single abstract method, so the interface stays functional and policies written against it keep working: the new overload has a default implementation that delegates to it. ResourceIsolationRequestCycleListener derives the type from the resolved handler through the new getRequestType(), which subclasses can override. FetchMetadataResourceIsolationPolicy allows another origin to render a page through a simple top-level navigation but not to invoke a listener. Its two-argument method applies the listener rules, which are the stricter of the two, so a caller that cannot say what the request does gets the safe answer. Sec-Fetch-Site: same-site means the request comes from the same registrable domain and scheme but a different origin, so by default it does not allow a listener invocation. Applications that trust every origin on their site can allow it with setSameSiteAllowed(true). Renders are unaffected: a page remains reachable by a top-level navigation from a sibling origin or from another site. OriginResourceIsolationPolicy is unchanged and stays in the default chain as the fallback for requests that carry no Fetch Metadata headers at all. The user guide chapter on resource isolation describes what the policy allows, what same-site means, and how to allow sibling origins with setSameSiteAllowed(true). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../ResourceIsolationRequestCycleListenerTest.java | 107 ++++++++++++++++++++- .../http/FetchMetadataResourceIsolationPolicy.java | 84 +++++++++++++++- .../protocol/http/IResourceIsolationPolicy.java | 50 +++++++++- .../ResourceIsolationRequestCycleListener.java | 26 ++++- .../src/main/asciidoc/security/security_5.adoc | 26 +++++ 5 files changed, 281 insertions(+), 12 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java index d4bc1c26b6..1d3f987321 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java @@ -22,6 +22,8 @@ import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPoli import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.DEST_DOCUMENT; import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.MODE_NAVIGATE; import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.MODE_NO_CORS; +import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.NONE; +import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.SAME_ORIGIN; import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.SAME_SITE; import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.SEC_FETCH_DEST_HEADER; import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.SEC_FETCH_MODE_HEADER; @@ -29,7 +31,9 @@ import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPoli import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.VARY_HEADER; import static org.junit.jupiter.api.Assertions.assertEquals; +import org.apache.wicket.protocol.http.IResourceIsolationPolicy.RequestType; import org.apache.wicket.protocol.http.IResourceIsolationPolicy.ResourceIsolationOutcome; +import org.apache.wicket.request.component.IRequestablePage; import org.apache.wicket.util.tester.WicketTestCase; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -112,17 +116,114 @@ public class ResourceIsolationRequestCycleListenerTest extends WicketTestCase } /** - * Tests whether a top level navigation request is allowed by FM checks + * A top-level navigation from another site may render a page, but not invoke a listener on it. */ @Test - void topLevelNavigationAllowedFM() + void topLevelNavigationListenerAborted() { tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE); tester.addRequestHeader(SEC_FETCH_MODE_HEADER, MODE_NAVIGATE); + assertRequestAborted(); + } + + /** + * The other half of {@link #topLevelNavigationListenerAborted()}: the same request may still + * render the page, so that pages remain linkable from elsewhere. + */ + @Test + void topLevelNavigationMayRenderButNotInvokeAListener() + { + // set on the request itself: addRequestHeader() only stages headers for the next request + tester.getRequest().setHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE); + tester.getRequest().setHeader(SEC_FETCH_MODE_HEADER, MODE_NAVIGATE); + // a simple top-level navigation is a GET; the mock request defaults to POST + tester.getRequest().setMethod("GET"); + + FetchMetadataResourceIsolationPolicy policy = new FetchMetadataResourceIsolationPolicy(); + IRequestablePage page = tester.getLastRenderedPage(); + + assertEquals(ResourceIsolationOutcome.ALLOWED, + policy.isRequestAllowed(tester.getRequest(), page, RequestType.RENDER)); + assertEquals(ResourceIsolationOutcome.DISALLOWED, + policy.isRequestAllowed(tester.getRequest(), page, RequestType.LISTENER)); + } + + /** + * A sibling origin on the same site is a different origin, so it may not invoke a listener. + */ + @Test + void sameSiteListenerAborted() + { + tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE); + + assertRequestAborted(); + } + + /** + * ... unless the application declares every origin on the site to be trusted. + */ + @Test + void sameSiteListenerAllowedWhenConfigured() + { + withCustomListener(new ResourceIsolationRequestCycleListener( + new FetchMetadataResourceIsolationPolicy().setSameSiteAllowed(true), + new OriginResourceIsolationPolicy())); + + tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE); + + assertRequestAccepted(); + } + + /** + * A same-site request may render a page regardless of the setting, as a top-level navigation. + */ + @Test + void sameSiteMayRenderRegardlessOfTheSetting() + { + tester.getRequest().setHeader(SEC_FETCH_SITE_HEADER, SAME_SITE); + tester.getRequest().setHeader(SEC_FETCH_MODE_HEADER, MODE_NAVIGATE); + tester.getRequest().setMethod("GET"); + + assertEquals(ResourceIsolationOutcome.ALLOWED, + new FetchMetadataResourceIsolationPolicy().isRequestAllowed(tester.getRequest(), + tester.getLastRenderedPage(), RequestType.RENDER)); + } + + /** + * Requests from the page itself are allowed, which is the normal case. + */ + @Test + void sameOriginListenerAccepted() + { + tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_ORIGIN); + assertRequestAccepted(); } + /** + * {@code none} means there was no initiating document - a typed URL or a bookmark - which + * another document cannot cause, so it is allowed. + */ + @Test + void browserInitiatedListenerAccepted() + { + tester.addRequestHeader(SEC_FETCH_SITE_HEADER, NONE); + + assertRequestAccepted(); + } + + /** + * Without Fetch Metadata headers the policy has nothing to judge on and the next policy decides. + */ + @Test + void missingFetchMetadataIsUnknown() + { + assertEquals(ResourceIsolationOutcome.UNKNOWN, + new FetchMetadataResourceIsolationPolicy().isRequestAllowed(tester.getRequest(), + tester.getLastRenderedPage(), RequestType.LISTENER)); + } + /** * Tests that a POST is not a simple top-level navigation request and is blocked */ @@ -165,7 +266,7 @@ public class ResourceIsolationRequestCycleListenerTest extends WicketTestCase @Test void varyHeaderSetWhenFetchMetadataAcceptsRequest() { - tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE); + tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_ORIGIN); tester.setFollowRedirects(false); assertRequestAccepted(); diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java index fc67e0e5ae..4d06a607e2 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java @@ -25,6 +25,24 @@ import org.apache.wicket.util.string.Strings; /** * Default resource isolation policy used in {@link ResourceIsolationRequestCycleListener}, * based on <a href="https://web.dev/fetch-metadata/";>https://web.dev/fetch-metadata/</a>. + * <p> + * The policy decides on the {@code Sec-Fetch-*} headers, which a browser sets itself and which + * cannot be set or removed by page content, and it distinguishes what the request does with the + * targeted page: + * <ul> + * <li>{@code same-origin} and {@code none} are allowed. {@code none} means there was no initiating + * document at all - a typed URL or a bookmark - which another document cannot cause.</li> + * <li>{@code same-site} is a <em>different</em> origin on the same site, so by default it may not + * invoke a listener. See {@link #setSameSiteAllowed(boolean)}.</li> + * <li>Anything else from another origin may still {@link RequestType#RENDER} a page through a simple + * top-level navigation, so that pages can be linked to from elsewhere, but may never invoke a + * {@link RequestType#LISTENER}.</li> + * <li>When the request carries no {@code Sec-Fetch-Site} header at all the outcome is + * {@link ResourceIsolationOutcome#UNKNOWN} and the next policy decides.</li> + * </ul> + * Wicket invokes listeners through ordinary GET navigations, so the headers alone do not say whether + * a request merely renders a page or performs an action on it. The {@link RequestType} the listener + * passes in is what separates the two. * * @see <a href="https://web.dev/fetch-metadata/";>https://web.dev/fetch-metadata/</a> * @@ -56,9 +74,22 @@ public class FetchMetadataResourceIsolationPolicy implements IResourceIsolationP private static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", " + SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER; + private boolean sameSiteAllowed = false; + + /** + * Called when the type of the request is not known. Applies the rules for a + * {@link RequestType#LISTENER}, which are the stricter of the two. + */ @Override public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage) + { + return isRequestAllowed(request, targetPage, RequestType.LISTENER); + } + + @Override + public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, + IRequestablePage targetPage, RequestType requestType) { // request made by a legacy browser with no support for Fetch Metadata String site = request.getHeader(SEC_FETCH_SITE_HEADER); @@ -66,19 +97,62 @@ public class FetchMetadataResourceIsolationPolicy implements IResourceIsolationP { return ResourceIsolationOutcome.UNKNOWN; } - - // Allow same-site and browser-initiated requests - if (SAME_ORIGIN.equals(site) || SAME_SITE.equals(site) || NONE.equals(site)) + + // Allow same-origin and browser-initiated requests. A browser cannot be made to report + // 'none' by another document, so it is not a forgeable value. + if (SAME_ORIGIN.equals(site) || NONE.equals(site)) + { + return ResourceIsolationOutcome.ALLOWED; + } + + // Allow requests from a sibling origin on the same site only when configured to do so + if (SAME_SITE.equals(site) && sameSiteAllowed) { return ResourceIsolationOutcome.ALLOWED; } - // Allow simple top-level navigations except <object> and <embed> - return isAllowedTopLevelNavigation(request) + // The request comes from another origin. Rendering a page is allowed for a simple top-level + // navigation, except <object> and <embed>, so that the page can still be linked to from + // elsewhere. Invoking a listener from another origin is not allowed. + return requestType == RequestType.RENDER && isAllowedTopLevelNavigation(request) ? ResourceIsolationOutcome.ALLOWED : ResourceIsolationOutcome.DISALLOWED; } + /** + * Sets whether requests from a different origin on the same site are allowed to invoke a + * listener. {@code Sec-Fetch-Site: same-site} means the request comes from the same registrable + * domain and scheme but a <em>different</em> origin, such as another subdomain or another port. + * <p> + * This is {@code false} by default, so a sibling origin cannot invoke a listener. Enable it when + * every origin on the site is trusted, for instance when the sibling subdomains are all part of + * the same application. Note that a hostile sibling - through a subdomain takeover, delegated + * user content, or an XSS elsewhere on the site - can then perform actions in the context of an + * authenticated user, because the browser sends the session cookie on a same-site request. + * <p> + * This setting does not affect renders: a page can always be reached by a simple top-level + * navigation, from a sibling origin or from another site entirely. + * + * @param sameSiteAllowed + * {@code true} to trust every origin on the same site + * @return {@code this} object for chaining + */ + public FetchMetadataResourceIsolationPolicy setSameSiteAllowed(boolean sameSiteAllowed) + { + this.sameSiteAllowed = sameSiteAllowed; + return this; + } + + /** + * @return whether requests from a different origin on the same site may invoke a listener, + * {@code false} by default + * @see #setSameSiteAllowed(boolean) + */ + public boolean isSameSiteAllowed() + { + return sameSiteAllowed; + } + private boolean isAllowedTopLevelNavigation(HttpServletRequest request) { String mode = request.getHeader(SEC_FETCH_MODE_HEADER); diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java index fb5de31218..11cb61bc2e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java @@ -51,8 +51,32 @@ public interface IResourceIsolationPolicy } /** - * Is the given request allowed. - * + * What the request is going to do with the targeted page. A policy usually has to treat these + * differently: a page may legitimately be rendered as the result of a top-level navigation from + * another site, but a listener on that page must never be invoked from another site. + * + * @author papegaaij + * + * @see IResourceIsolationPolicy#isRequestAllowed(jakarta.servlet.http.HttpServletRequest, + * org.apache.wicket.request.component.IRequestablePage, RequestType) + */ + public enum RequestType + { + /** The request renders the targeted page. */ + RENDER, + /** The request invokes a listener on the targeted page, such as a link or a form submit. */ + LISTENER + } + + /** + * Is the given request allowed. Implement {@link #isRequestAllowed(HttpServletRequest, + * IRequestablePage, RequestType)} instead when the outcome depends on what the request does with + * the page, which is normally the case. + * <p> + * Implementations of this method are called for both renders and listener invocations without + * being able to tell them apart, so they must apply the rules that are safe for a listener + * invocation. + * * @param request * request * @param targetPage @@ -62,6 +86,28 @@ public interface IResourceIsolationPolicy ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage); + /** + * Is the given request allowed, given what it is going to do with the targeted page. + * <p> + * This is the method {@link ResourceIsolationRequestCycleListener} calls. The default + * implementation ignores {@code requestType} and delegates to + * {@link #isRequestAllowed(HttpServletRequest, IRequestablePage)}, so that policies written + * against the two-argument method keep working unchanged. + * + * @param request + * request + * @param targetPage + * targeted page + * @param requestType + * what the request does with {@code targetPage} + * @return outcome, must not be <code>null</code> + */ + default ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, + IRequestablePage targetPage, RequestType requestType) + { + return isRequestAllowed(request, targetPage); + } + /** * Set possible response headers. * diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java index 5858b38fe2..4ad3792870 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java @@ -30,6 +30,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.wicket.RestartResponseException; import org.apache.wicket.core.request.handler.IPageRequestHandler; import org.apache.wicket.core.request.handler.RenderPageRequestHandler; +import org.apache.wicket.protocol.http.IResourceIsolationPolicy.RequestType; import org.apache.wicket.protocol.http.IResourceIsolationPolicy.ResourceIsolationOutcome; import org.apache.wicket.request.IRequestHandler; import org.apache.wicket.request.IRequestHandlerDelegate; @@ -284,6 +285,25 @@ public class ResourceIsolationRequestCycleListener implements IRequestCycleListe && !(handler instanceof RenderPageRequestHandler); } + /** + * Determines what the request does with the targeted page, so that the policies can distinguish + * a render - which may legitimately be the result of a top-level navigation from another site - + * from a listener invocation, which must never come from another site. + * <p> + * Note that {@link #isChecked(IRequestHandler)} excludes renders by default, so policies only + * see {@link RequestType#RENDER} when that method is overridden to extend resource isolation to + * page renders. + * + * @param handler + * the handler that is currently processing + * @return the type of request + */ + protected RequestType getRequestType(IRequestHandler handler) + { + return handler instanceof RenderPageRequestHandler ? RequestType.RENDER + : RequestType.LISTENER; + } + @Override public void onRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler) { @@ -322,10 +342,12 @@ public class ResourceIsolationRequestCycleListener implements IRequestCycleListe return; } + RequestType requestType = getRequestType(handler); + for (IResourceIsolationPolicy policy : resourceIsolationPolicies) { - ResourceIsolationOutcome outcome = policy - .isRequestAllowed(containerRequest, targetedPage); + ResourceIsolationOutcome outcome = policy.isRequestAllowed(containerRequest, + targetedPage, requestType); if (ResourceIsolationOutcome.DISALLOWED.equals(outcome)) { log.debug("Isolation policy {} has rejected a request to {}", diff --git a/wicket-user-guide/src/main/asciidoc/security/security_5.adoc b/wicket-user-guide/src/main/asciidoc/security/security_5.adoc index 8225ea6d3d..ecab4acb02 100644 --- a/wicket-user-guide/src/main/asciidoc/security/security_5.adoc +++ b/wicket-user-guide/src/main/asciidoc/security/security_5.adoc @@ -57,6 +57,32 @@ For example: } ---- +The _Sec-Fetch-Site_ header tells __FetchMetadataResourceIsolationPolicy__ where a request came from. A value of _same-origin_ means it was made from the very same scheme, host and port. _cross-site_ means it came from a different site altogether. _same-site_ sits in between: the same registrable domain and scheme, but a *different* origin - another subdomain, or another port. A _same-site_ request is therefore always cross-origin. + +The policy allows _same-origin_ requests, and browser-initiated ones (_none_, such as a typed url or a bookmark). For a request from another origin it looks at what that request does with the page: rendering is allowed for a simple top-level navigation, so our pages stay linkable from anywhere, while invoking a listener is not. + +Because a _same-site_ request is cross-origin, it is treated as one: by default a sibling origin such as _other.example.com_ cannot execute _Link.onClick()_ or submit a form on _app.example.com_, even though the browser does send our session cookie along with such a request. + +Note that this decision rests entirely with the Fetch Metadata policy: __OriginResourceIsolationPolicy__ is only consulted for requests that carry no Fetch Metadata headers at all, which no current browser does. + +If every origin on our site is trusted - for instance when all of our subdomains are part of the same application - we can allow sibling origins to invoke listeners as well with _setSameSiteAllowed(true)_: + +[source,java] +---- + @Override + protected void init() { + super.init(); + getRequestCycleListeners().add( + new ResourceIsolationRequestCycleListener( + new FetchMetadataResourceIsolationPolicy().setSameSiteAllowed(true), + new OriginResourceIsolationPolicy() + )); + // ... + } +---- + +WARNING: Enabling this trusts every origin on our site. A hostile sibling origin - obtained through a subdomain takeover, through delegated user content, or through an XSS somewhere else on the site - can then perform actions on behalf of an authenticated user. + _ResourceIsolationRequestCycleListener_ is not an alternative to _CryptoMapper_! Both of them could be used separately or in tandem to prevent CSRF attacks depending on the application requirements. NOTE: In the next chapter we will cover unit testing with Wicket. If your application is protected with _ResourceIsolationRequestCycleListener_ you have to properly set request header _"sec-fetch-site"_ to make you unit tests pass. In <<testing.adoc#_setting_request_headers,paragraph 23.1.10>> you will learn how to do it.
