You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2020/08/15 21:11:10 UTC
[wicket] branch master updated: WICKET-6786 renamed classes,
corrected javadoc
This is an automated email from the ASF dual-hosted git repository.
svenmeier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/master by this push:
new f865d85 WICKET-6786 renamed classes, corrected javadoc
f865d85 is described below
commit f865d85e7c5f86cf37d09e274c25864633fdfa25
Author: Sven Meier <sv...@apache.org>
AuthorDate: Wed Aug 12 23:10:53 2020 +0200
WICKET-6786 renamed classes, corrected javadoc
ResourceIsolationRequestCycleListener
IResourceIsolationPolicy
FetchMetadataResourceIsolationPolicy
OriginResourceIsolationPolicy
---
.../http/CsrfPreventionRequestCycleListener.java | 5 +-
...a => FetchMetadataResourceIsolationPolicy.java} | 47 +++++--
...onPolicy.java => IResourceIsolationPolicy.java} | 61 +++++----
...icy.java => OriginResourceIsolationPolicy.java} | 25 ++--
.../protocol/http/ResourceIsolationOutcome.java | 28 ----
... => ResourceIsolationRequestCycleListener.java} | 145 +++++++++++----------
.../CsrfPreventionRequestCycleListenerTest.java | 44 +------
...ResourceIsolationRequestCycleListenerTest.java} | 50 +++----
8 files changed, 200 insertions(+), 205 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
index f4d4040..a259fb1 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
@@ -104,11 +104,10 @@ import org.slf4j.LoggerFactory;
* conflict and the request should be suppressed</li>
* </ul>
*
- * @see FetchMetadataRequestCycleListener
- * @deprecated
+ * @deprecated Use {@link FetchMetadataResourceIsolationPolicy} instead
*/
@Deprecated(since = "9.1.0")
-public class CsrfPreventionRequestCycleListener extends OriginBasedResourceIsolationPolicy implements IRequestCycleListener
+public class CsrfPreventionRequestCycleListener extends OriginResourceIsolationPolicy implements IRequestCycleListener
{
private static final Logger log = LoggerFactory
.getLogger(CsrfPreventionRequestCycleListener.class);
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java
similarity index 60%
rename from wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
rename to wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java
index e3de5a1..18c5407 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataResourceIsolationPolicy.java
@@ -17,34 +17,54 @@
package org.apache.wicket.protocol.http;
import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
import org.apache.wicket.request.component.IRequestablePage;
import org.apache.wicket.util.string.Strings;
/**
- * Default resource isolation policy used in {@link FetchMetadataRequestCycleListener} that
- * implements the {@link ResourceIsolationPolicy} interface. This default policy is based on
- * <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>.
+ * Default resource isolation policy used in {@link ResourceIsolationRequestCycleListener},
+ * based on <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>.
*
* @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
*
* @author Santiago Diaz - saldiaz@google.com
* @author Ecenaz Jen Ozmen - ecenazo@google.com
*/
-public class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy
+public class FetchMetadataResourceIsolationPolicy implements IResourceIsolationPolicy
{
+ public static final String SEC_FETCH_SITE_HEADER = "sec-fetch-site";
+ public static final String SEC_FETCH_MODE_HEADER = "sec-fetch-mode";
+ public static final String SEC_FETCH_DEST_HEADER = "sec-fetch-dest";
+
+ public static final String SAME_ORIGIN = "same-origin";
+ public static final String SAME_SITE = "same-site";
+ public static final String NONE = "none";
+ public static final String MODE_NAVIGATE = "navigate";
+ public static final String DEST_OBJECT = "object";
+ public static final String DEST_EMBED = "embed";
+ public static final String CROSS_SITE = "cross-site";
+ public static final String CORS = "cors";
+ public static final String DEST_SCRIPT = "script";
+ public static final String DEST_IMAGE = "image";
+
+ public static final String VARY_HEADER = "Vary";
+
+ private static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", "
+ + SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER;
+
@Override
public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request,
IRequestablePage targetPage)
{
// request made by a legacy browser with no support for Fetch Metadata
- if (!hasFetchMetadataHeaders(request))
+ String site = request.getHeader(SEC_FETCH_SITE_HEADER);
+ if (Strings.isEmpty(site))
{
return ResourceIsolationOutcome.UNKNOWN;
}
-
- String site = request.getHeader(SEC_FETCH_SITE_HEADER);
-
+
// Allow same-site and browser-initiated requests
if (SAME_ORIGIN.equals(site) || SAME_SITE.equals(site) || NONE.equals(site))
{
@@ -70,11 +90,14 @@ public class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy
}
/**
- * Checks if Sec-Fetch-* headers are present
+ * Set vary headers to avoid caching responses processed by Fetch Metadata.
+ * <p>
+ * Caching these responses may return 403 responses to legitimate requests
+ * defeat the protection.
*/
- private static boolean hasFetchMetadataHeaders(HttpServletRequest containerRequest)
+ @Override
+ public void setHeaders(HttpServletResponse response)
{
- String secFetchSiteValue = containerRequest.getHeader(SEC_FETCH_SITE_HEADER);
- return !Strings.isEmpty(secFetchSiteValue);
+ response.addHeader(VARY_HEADER, VARY_HEADER_VALUE);
}
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java
similarity index 56%
rename from wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
rename to wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java
index 6995cb2..959c70f 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/IResourceIsolationPolicy.java
@@ -17,16 +17,17 @@
package org.apache.wicket.protocol.http;
import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
import org.apache.wicket.request.component.IRequestablePage;
/**
- * Interface for the resource isolation policies to be used for fetch metadata checks.
- *
- * Resource isolation policies are designed to protect against cross origin attacks and use the
- * {@code sec-fetch-*} request headers to decide whether to accept or reject a request. Read more
- * about <a href="https://web.dev/fetch-metadata/">Fetch Metadata.</a>
- *
- * See {@link DefaultResourceIsolationPolicy} for the default implementation used.
+ * Interface for the resource isolation policies.
+ * <p>
+ * Resource isolation policies are designed to protect against cross origin attacks.
+ * <p>
+ * See {@link FetchMetadataResourceIsolationPolicy} for the default implementation used
+ * by {@link ResourceIsolationRequestCycleListener}.
*
* @see <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
*
@@ -34,23 +35,39 @@ import org.apache.wicket.request.component.IRequestablePage;
* @author Ecenaz Jen Ozmen - ecenazo@google.com
*/
@FunctionalInterface
-public interface ResourceIsolationPolicy
+public interface IResourceIsolationPolicy
{
- String SEC_FETCH_SITE_HEADER = "sec-fetch-site";
- String SEC_FETCH_MODE_HEADER = "sec-fetch-mode";
- String SEC_FETCH_DEST_HEADER = "sec-fetch-dest";
- String VARY_HEADER = "Vary";
- String SAME_ORIGIN = "same-origin";
- String SAME_SITE = "same-site";
- String NONE = "none";
- String MODE_NAVIGATE = "navigate";
- String DEST_OBJECT = "object";
- String DEST_EMBED = "embed";
- String CROSS_SITE = "cross-site";
- String CORS = "cors";
- String DEST_SCRIPT = "script";
- String DEST_IMAGE = "image";
+ /**
+ * Indicates the outcome for a resource isolation policy for a request. When the outcome is
+ * {@link #UNKNOWN}, the next policy will be consulted.
+ *
+ * @author papegaaij
+ *
+ * @see IResourceIsolationPolicy#isRequestAllowed(javax.servlet.http.HttpServletRequest, org.apache.wicket.request.component.IRequestablePage)
+ */
+ public enum ResourceIsolationOutcome
+ {
+ ALLOWED, DISALLOWED, UNKNOWN
+ }
+ /**
+ * Is the given request allowed.
+ *
+ * @param request
+ * request
+ * @param targetPage
+ * targeted page
+ * @return outcome, must not be <code>null</code>
+ */
ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request,
IRequestablePage targetPage);
+
+ /**
+ * Set possible response headers.
+ *
+ * @param response
+ */
+ default void setHeaders(HttpServletResponse response)
+ {
+ }
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginResourceIsolationPolicy.java
similarity index 92%
rename from wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
rename to wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginResourceIsolationPolicy.java
index 3df6bbf..ea742b0 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginResourceIsolationPolicy.java
@@ -21,7 +21,9 @@ import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Locale;
+
import javax.servlet.http.HttpServletRequest;
+
import org.apache.wicket.request.component.IRequestablePage;
import org.apache.wicket.request.http.WebRequest;
import org.apache.wicket.util.lang.Checks;
@@ -29,10 +31,18 @@ import org.apache.wicket.util.string.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPolicy
+/**
+ * {@link IResourceIsolationPolicy} based on {@link WebRequest#HEADER_ORIGIN} and
+ * {@link WebRequest#HEADER_REFERER} headers.
+ * <p>
+ * This origin-based listener can be used in combination with the
+ * {@link ResourceIsolationRequestCycleListener} to add support for legacy browsers that don't send
+ * Sec-Fetch-* headers yet.
+ *
+ */
+public class OriginResourceIsolationPolicy implements IResourceIsolationPolicy
{
- private static final Logger log = LoggerFactory
- .getLogger(OriginBasedResourceIsolationPolicy.class);
+ private static final Logger log = LoggerFactory.getLogger(OriginResourceIsolationPolicy.class);
/**
* A white list of accepted origins (host names/domain names) presented as
@@ -53,7 +63,7 @@ public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPoli
* the acceptable origin
* @return this
*/
- public OriginBasedResourceIsolationPolicy addAcceptedOrigin(String acceptedOrigin)
+ public OriginResourceIsolationPolicy addAcceptedOrigin(String acceptedOrigin)
{
Checks.notNull("acceptedOrigin", acceptedOrigin);
@@ -69,14 +79,11 @@ public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPoli
}
/**
- * This origin-based listener can be used in combination with the
- * {@link FetchMetadataRequestCycleListener} to add support for legacy browsers that don't send
- * Sec-Fetch-* headers yet.
- *
* @return whether the request is allowed based on its origin
*/
@Override
- public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
+ public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request,
+ IRequestablePage targetPage)
{
String sourceUri = getSourceUri(request);
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
deleted file mode 100644
index 1250bfc..0000000
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.protocol.http;
-
-/**
- * Indicates the outcome for a resource isolation policy for a request. When the outcome is
- * {@link #UNKNOWN}, the next policy will be consulted.
- *
- * @author papegaaij
- */
-public enum ResourceIsolationOutcome
-{
- ALLOWED, DISALLOWED, UNKNOWN
-}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java
similarity index 70%
rename from wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
rename to wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java
index d1500b9..3c658a7 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListener.java
@@ -17,10 +17,6 @@
package org.apache.wicket.protocol.http;
import static java.util.Arrays.asList;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_DEST_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_MODE_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_SITE_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.VARY_HEADER;
import java.util.ArrayList;
import java.util.Arrays;
@@ -29,10 +25,12 @@ import java.util.List;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
+import javax.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.ResourceIsolationOutcome;
import org.apache.wicket.request.IRequestHandler;
import org.apache.wicket.request.IRequestHandlerDelegate;
import org.apache.wicket.request.component.IRequestablePage;
@@ -46,33 +44,31 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * The Fetch Metadata Request Cycle Listener is Wicket's implementation of Fetch Metadata. This adds
- * a layer of protection for modern browsers that prevents Cross-Site Request Forgery attacks.
- *
- * This request listener uses the {@link DefaultResourceIsolationPolicy} by default and can be
- * customized with additional Resource Isolation Policies.
- *
- * This listener can be configured to add exempted URL paths that are intended to be used
- * cross-site.
- *
+ * This {@link RequestCycle} listener ensures resource isolation, adding a layer of protection for
+ * modern browsers that prevent <em>Cross-Site Request Forgery</em> attacks.
+ * <p>
+ * It uses the {@link FetchMetadataResourceIsolationPolicy} and
+ * {@link OriginResourceIsolationPolicy} by default and can be customized with additional
+ * {@link IResourceIsolationPolicy}s.
+ * <p>
+ * URL paths that are intended to be used cross-site can be excempted from these policies.
+ * <p>
* Learn more about Fetch Metadata and resource isolation at
- * <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
+ * <a href="https://web.dev/fetch-metadata">https://web.dev/fetch-metadata/</a>
*
* @author Santiago Diaz - saldiaz@google.com
* @author Ecenaz Jen Ozmen - ecenazo@google.com
*/
-public class FetchMetadataRequestCycleListener implements IRequestCycleListener
+public class ResourceIsolationRequestCycleListener implements IRequestCycleListener
{
private static final Logger log = LoggerFactory
- .getLogger(FetchMetadataRequestCycleListener.class);
+ .getLogger(ResourceIsolationRequestCycleListener.class);
public static final String ERROR_MESSAGE = "The request was blocked by a resource isolation policy";
- public static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", "
- + SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER;
/**
* The action to perform when the outcome of the resource isolation policy is DISALLOWED or
- * UNKNOWN.
+ * UNKNOWN.
*/
public enum CsrfAction
{
@@ -83,6 +79,13 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
{
return "aborted";
}
+
+ @Override
+ void apply(ResourceIsolationRequestCycleListener listener, HttpServletRequest request,
+ IRequestablePage page)
+ {
+ listener.abortHandler(request, page);
+ }
},
/**
@@ -94,6 +97,13 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
{
return "suppressed";
}
+
+ @Override
+ void apply(ResourceIsolationRequestCycleListener listener, HttpServletRequest request,
+ IRequestablePage page)
+ {
+ listener.suppressHandler(request, page);
+ }
},
/** Detects a CSRF request, logs it and allows the request to continue. */
@@ -103,17 +113,28 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
{
return "allowed";
}
- },
+
+ @Override
+ void apply(ResourceIsolationRequestCycleListener listener, HttpServletRequest request,
+ IRequestablePage page)
+ {
+ listener.allowHandler(request, page);
+ }
+ };
+
+ abstract void apply(ResourceIsolationRequestCycleListener listener,
+ HttpServletRequest request, IRequestablePage page);
}
/**
- * Action to perform when none resource isolation policies can determine the validity of the
+ * Action to perform when no resource isolation policy can determine the validity of the
* request.
*/
private CsrfAction unknownOutcomeAction = CsrfAction.ABORT;
/**
- * Action to perform when DISALLOWED is reported by a resource isolation policy.
+ * Action to perform when {@link ResourceIsolationOutcome#DISALLOWED} is reported by a
+ * resource isolation policy.
*/
private CsrfAction disallowedOutcomeAction = CsrfAction.ABORT;
@@ -129,26 +150,26 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
*/
private String errorMessage = ERROR_MESSAGE;
-
private final Set<String> exemptedPaths = new HashSet<>();
- private final List<ResourceIsolationPolicy> resourceIsolationPolicies = new ArrayList<>();
+
+ private final List<IResourceIsolationPolicy> resourceIsolationPolicies = new ArrayList<>();
/**
- * Create a new FetchMetadataRequestCycleListener with the given policies. If no policies are
- * given, {@link DefaultResourceIsolationPolicy} and {@link OriginBasedResourceIsolationPolicy}
- * will be used. The policies are checked in order. The first outcome that's not
+ * Create a new listener with the given policies. If no policies are given,
+ * {@link FetchMetadataResourceIsolationPolicy} and {@link OriginResourceIsolationPolicy} will
+ * be used. The policies are checked in order. The first outcome that's not
* {@link ResourceIsolationOutcome#UNKNOWN} will be used.
*
* @param policies
* the policies to check requests against.
*/
- public FetchMetadataRequestCycleListener(ResourceIsolationPolicy... policies)
+ public ResourceIsolationRequestCycleListener(IResourceIsolationPolicy... policies)
{
this.resourceIsolationPolicies.addAll(asList(policies));
if (policies.length == 0)
{
- this.resourceIsolationPolicies.addAll(asList(new DefaultResourceIsolationPolicy(),
- new OriginBasedResourceIsolationPolicy()));
+ this.resourceIsolationPolicies.addAll(asList(new FetchMetadataResourceIsolationPolicy(),
+ new OriginResourceIsolationPolicy()));
}
}
@@ -161,7 +182,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
*
* @return this (for chaining)
*/
- public FetchMetadataRequestCycleListener setUnknownOutcomeAction(CsrfAction action)
+ public ResourceIsolationRequestCycleListener setUnknownOutcomeAction(CsrfAction action)
{
this.unknownOutcomeAction = action;
return this;
@@ -176,7 +197,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
*
* @return this
*/
- public FetchMetadataRequestCycleListener setDisallowedOutcomeAction(CsrfAction action)
+ public ResourceIsolationRequestCycleListener setDisallowedOutcomeAction(CsrfAction action)
{
this.disallowedOutcomeAction = action;
return this;
@@ -190,7 +211,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
*
* @return this
*/
- public FetchMetadataRequestCycleListener setErrorCode(int errorCode)
+ public ResourceIsolationRequestCycleListener setErrorCode(int errorCode)
{
this.errorCode = errorCode;
return this;
@@ -204,7 +225,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
*
* @return this
*/
- public FetchMetadataRequestCycleListener setErrorMessage(String errorMessage)
+ public ResourceIsolationRequestCycleListener setErrorMessage(String errorMessage)
{
this.errorMessage = errorMessage;
return this;
@@ -251,11 +272,11 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
/**
* Override to change the request handler types that are checked. Currently only action handlers
- * (form submits, link clicks, AJAX events) are checked for a matching Origin HTTP header.
+ * (form submits, link clicks, AJAX events) are checked.
*
* @param handler
* the handler that is currently processing
- * @return true when the Origin HTTP header should be checked for this {@code handler}
+ * @return true when resource isolation should be checked for this {@code handler}
*/
protected boolean isChecked(IRequestHandler handler)
{
@@ -284,7 +305,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
{
if (log.isDebugEnabled())
{
- log.debug("Targeted page {} was opted out of the CSRF origin checks, allowed",
+ log.debug("Targeted page {} was opted out of resource isolation, allowed",
targetedPage.getClass().getName());
}
return;
@@ -301,22 +322,23 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
return;
}
- for (ResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies)
+ for (IResourceIsolationPolicy policy : resourceIsolationPolicies)
{
- ResourceIsolationOutcome outcome = resourceIsolationPolicy
+ ResourceIsolationOutcome outcome = policy
.isRequestAllowed(containerRequest, targetedPage);
if (ResourceIsolationOutcome.DISALLOWED.equals(outcome))
{
log.debug("Isolation policy {} has rejected a request to {}",
- Classes.simpleName(resourceIsolationPolicy.getClass()), pathInfo);
- triggerAction(disallowedOutcomeAction, containerRequest, targetedPage);
+ Classes.simpleName(policy.getClass()), pathInfo);
+ disallowedOutcomeAction.apply(this, containerRequest, targetedPage);
+ return;
}
else if (ResourceIsolationOutcome.ALLOWED.equals(outcome))
{
return;
}
}
- triggerAction(unknownOutcomeAction, containerRequest, targetedPage);
+ unknownOutcomeAction.apply(this, containerRequest, targetedPage);
}
else
{
@@ -326,42 +348,31 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
}
}
- private void triggerAction(CsrfAction action, HttpServletRequest request, IRequestablePage page)
- {
- switch (action)
- {
- case ALLOW :
- allowHandler(request, page);
- break;
- case SUPPRESS :
- suppressHandler(request, page);
- break;
- case ABORT :
- abortHandler(request, page);
- break;
- }
- }
-
+ /**
+ * Allow isolation policy to add headers.
+ *
+ * @see IResourceIsolationPolicy#setHeaders(HttpServletResponse)
+ */
@Override
public void onEndRequest(RequestCycle cycle)
{
- // set vary headers to avoid caching responses processed by Fetch Metadata
- // caching these responses may return 403 responses to legitimate requests
- // or defeat the protection
if (cycle.getResponse() instanceof WebResponse)
{
WebResponse webResponse = (WebResponse)cycle.getResponse();
if (webResponse.isHeaderSupported())
{
- webResponse.addHeader(VARY_HEADER, VARY_HEADER_VALUE);
+ for (IResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies)
+ {
+ resourceIsolationPolicy
+ .setHeaders((HttpServletResponse)webResponse.getContainerResponse());
+ }
}
}
}
/**
- * Handles the case where the resource isolation policies resulted in
- * {@link ResourceIsolationOutcome#UNKNOWN} or {@link ResourceIsolationOutcome#DISALLOWED} and
- * the action was set to {#link {@link CsrfAction#ALLOW}.
+ * Allow the execution of the listener in the request because the outcome results in
+ * {@link CsrfAction#ALLOW}.
*
* @param request
* the request
@@ -374,7 +385,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
}
/**
- * Supresses the execution of the listener in the request because the outcome results in
+ * Suppress the execution of the listener in the request because the outcome results in
* {@link CsrfAction#SUPPRESS}.
*
* @param request
@@ -390,7 +401,7 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
}
/**
- * Aborts the request because the outcome results in {@link CsrfAction#ABORT}.
+ * Abort the request because the outcome results in {@link CsrfAction#ABORT}.
*
* @param request
* the request
diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListenerTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListenerTest.java
index 9b5134d..1f6524d 100644
--- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListenerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListenerTest.java
@@ -16,14 +16,12 @@
*/
package org.apache.wicket.protocol.http;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.CROSS_SITE;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_DEST_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_SITE_HEADER;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import javax.servlet.http.HttpServletRequest;
+
import org.apache.wicket.RestartResponseException;
import org.apache.wicket.protocol.http.CsrfPreventionRequestCycleListener.CsrfAction;
import org.apache.wicket.request.IRequestHandler;
@@ -34,7 +32,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/**
- * Test cases for the CsrfPreventionRequestCycleListener. FirstPage has a link that when clicked
+ * Test cases for the {@link CsrfPreventionRequestCycleListener}. FirstPage has a link that when clicked
* should render SecondPage.
*/
class CsrfPreventionRequestCycleListenerTest extends WicketTestCase
@@ -415,7 +413,6 @@ class CsrfPreventionRequestCycleListenerTest extends WicketTestCase
{
csrfListener.addAcceptedOrigin("example.com");
tester.addRequestHeader(WebRequest.HEADER_ORIGIN, "http://example.com/");
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
tester.clickLink("link");
@@ -423,43 +420,6 @@ class CsrfPreventionRequestCycleListenerTest extends WicketTestCase
tester.assertRenderedPage(SecondPage.class);
}
- /** Tests whitelisting with conflicting subdomain origin when sec-fetch-site is cross-site. */
- @Test
- void crossSiteButWhitelistedSubdomainOriginAllowed()
- {
- csrfListener.addAcceptedOrigin("example.com");
-
- tester.addRequestHeader(WebRequest.HEADER_ORIGIN, "http://foo.example.com/");
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
- tester.addRequestHeader(SEC_FETCH_DEST_HEADER, CROSS_SITE);
-
- tester.clickLink("link");
-
- tester.assertRenderedPage(SecondPage.class);
- assertOriginsWhitelisted();
- }
-
- /**
- * Tests when the listener is disabled for a specific page (by overriding
- * {@link CsrfPreventionRequestCycleListener#isChecked(IRequestablePage)}) and when fetch
- * metadata headers indicate cross-site request
- */
- @Test
- void crossSitePageNotCheckedAllowed()
- {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER,
- CROSS_SITE);
- csrfListener.setConflictingOriginAction(CsrfAction.ABORT);
-
- // disable the check for this page
- checkPage = false;
-
- tester.clickLink("link");
-
- assertConflictingOriginsRequestAllowed();
- tester.assertRenderedPage(SecondPage.class);
- }
-
/*
* Infrastructure code for these test cases starts here.
*/
diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java
similarity index 72%
rename from wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
rename to wicket-core/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java
index d1a5960..2dce6ee 100644
--- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/ResourceIsolationRequestCycleListenerTest.java
@@ -16,42 +16,46 @@
*/
package org.apache.wicket.protocol.http;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.CROSS_SITE;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.DEST_EMBED;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.DEST_OBJECT;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.MODE_NAVIGATE;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SAME_ORIGIN;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SAME_SITE;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_DEST_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_MODE_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.SEC_FETCH_SITE_HEADER;
-import static org.apache.wicket.protocol.http.ResourceIsolationPolicy.VARY_HEADER;
+import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.CROSS_SITE;
+import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.DEST_EMBED;
+import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.DEST_OBJECT;
+import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.MODE_NAVIGATE;
+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;
+import static org.apache.wicket.protocol.http.FetchMetadataResourceIsolationPolicy.SEC_FETCH_SITE_HEADER;
+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.ResourceIsolationOutcome;
import org.apache.wicket.util.tester.WicketTestCase;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
+/**
+ * Test for {@link ResourceIsolationRequestCycleListener}.
+ */
+public class ResourceIsolationRequestCycleListenerTest extends WicketTestCase
{
- private FetchMetadataRequestCycleListener fetchMetadataListener;
+ private ResourceIsolationRequestCycleListener listener;
@BeforeEach
void before()
{
- withCustomListener(new FetchMetadataRequestCycleListener());
+ withCustomListener(new ResourceIsolationRequestCycleListener());
}
- void withCustomListener(FetchMetadataRequestCycleListener fetchMetadataListener)
+ void withCustomListener(ResourceIsolationRequestCycleListener fetchMetadataListener)
{
WebApplication application = tester.getApplication();
- if (this.fetchMetadataListener != null)
+ if (this.listener != null)
{
- application.getRequestCycleListeners().remove(this.fetchMetadataListener);
+ application.getRequestCycleListeners().remove(this.listener);
}
- this.fetchMetadataListener = fetchMetadataListener;
+ this.listener = fetchMetadataListener;
application.getRequestCycleListeners().add(fetchMetadataListener);
tester.startPage(FirstPage.class);
@@ -112,6 +116,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
void varyHeaderSetWhenFetchMetadataRejectsRequest()
{
tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+ tester.setFollowRedirects(false);
assertRequestAborted();
String vary = tester.getLastResponse().getHeader("Vary");
@@ -135,6 +140,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
void varyHeaderSetWhenFetchMetadataAcceptsRequest()
{
tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE);
+ tester.setFollowRedirects(false);
assertRequestAccepted();
String vary = tester.getLastResponse().getHeader(VARY_HEADER);
@@ -153,7 +159,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
@Test
void whenAtFirstNotUnkownRejectsRequest_thenRequestRejected()
{
- withCustomListener(new FetchMetadataRequestCycleListener(
+ withCustomListener(new ResourceIsolationRequestCycleListener(
(request, page) -> ResourceIsolationOutcome.UNKNOWN,
(request, page) -> ResourceIsolationOutcome.UNKNOWN,
(request, page) -> ResourceIsolationOutcome.DISALLOWED,
@@ -164,7 +170,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
@Test
void whenFirstNotUnknownPolicieAcceptRequest_thenRequestAccepted()
{
- withCustomListener(new FetchMetadataRequestCycleListener(
+ withCustomListener(new ResourceIsolationRequestCycleListener(
(request, page) -> ResourceIsolationOutcome.UNKNOWN,
(request, page) -> ResourceIsolationOutcome.ALLOWED,
(request, page) -> ResourceIsolationOutcome.ALLOWED,
@@ -175,9 +181,9 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
@Test
void whenCrossOriginRequestToExempted_thenRequestAccepted()
{
- fetchMetadataListener
+ listener
.addExemptedPaths("/wicket/bookmarkable/org.apache.wicket.protocol.http.FirstPage");
- withCustomListener(fetchMetadataListener);
+ withCustomListener(listener);
tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
assertRequestAccepted();
@@ -189,7 +195,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
assertEquals(tester.getLastResponse().getStatus(),
javax.servlet.http.HttpServletResponse.SC_FORBIDDEN);
assertEquals(tester.getLastResponse().getErrorMessage(),
- FetchMetadataRequestCycleListener.ERROR_MESSAGE);
+ ResourceIsolationRequestCycleListener.ERROR_MESSAGE);
}
private void assertRequestAccepted()