Author: fmeschbe
Date: Tue Jul 5 11:26:45 2011
New Revision: 1142992
URL: http://svn.apache.org/viewvc?rev=1142992&view=rev
Log:
SLING-2126 Add helper method to validate a target to redirect to after logging in. Also use
this
method in the redirects in the DefaultAuthenticationFeedbackHandler and AbstractAuthenticationHandler.
Finally, since we add a method, increase the micro version number for clients to be able properly
require the implementation.
Added:
sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
(with props)
Modified:
sling/trunk/bundles/auth/core/pom.xml
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandler.java
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java
Modified: sling/trunk/bundles/auth/core/pom.xml
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/pom.xml?rev=1142992&r1=1142991&r2=1142992&view=diff
==============================================================================
--- sling/trunk/bundles/auth/core/pom.xml (original)
+++ sling/trunk/bundles/auth/core/pom.xml Tue Jul 5 11:26:45 2011
@@ -68,7 +68,7 @@
</Bundle-DocURL>
<Export-Package>
org.apache.sling.auth.core;version=1.0.0,
- org.apache.sling.auth.core.spi;version=1.0.2,
+ org.apache.sling.auth.core.spi;version=1.0.4,
org.apache.sling.engine.auth;version=2.0.6
</Export-Package>
<Import-Package>
@@ -151,6 +151,10 @@
<artifactId>junit</artifactId>
</dependency>
<dependency>
+ <groupId>org.jmock</groupId>
+ <artifactId>jmock-junit4</artifactId>
+ </dependency>
+ <dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
</dependency>
Modified: sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandler.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandler.java?rev=1142992&r1=1142991&r2=1142992&view=diff
==============================================================================
--- sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandler.java
(original)
+++ sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandler.java
Tue Jul 5 11:26:45 2011
@@ -30,6 +30,10 @@ import javax.servlet.http.HttpServletReq
import javax.servlet.http.HttpServletResponse;
import org.apache.sling.api.auth.Authenticator;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceUtil;
+import org.apache.sling.auth.core.AuthenticationSupport;
+import org.slf4j.LoggerFactory;
/**
* The <code>AbstractAuthenticationHandler</code> implements the
@@ -184,7 +188,10 @@ public abstract class AbstractAuthentica
* @param response The response used to send the redirect to the client.
* @param target The target path to redirect the client to. This parameter
* must not be prefixed with the request's context path because
- * this will be added by this method.
+ * this will be added by this method. If this parameter is not
+ * a valid target request as per the
+ * {@link #isRedirectValid(HttpServletRequest, String)} method
+ * the target is modified to be the root of the request's context.
* @param params The map of parameters to be added to the target path. This
* may be <code>null</code>.
* @throws IOException If an error occurrs sending the redirect request
@@ -195,13 +202,20 @@ public abstract class AbstractAuthentica
* problem if the encoding required by the specification is
* missing.
* @since 1.0.2 (Bundle version 1.0.4)
+ * @since 1.0.4 (bundle version 1.0.8) the target is validated with the
+ * {@link #isRedirectValid(HttpServletRequest, String)} method.
*/
public static void sendRedirect(final HttpServletRequest request,
final HttpServletResponse response, final String target,
Map<String, String> params) throws IOException {
StringBuilder b = new StringBuilder();
b.append(request.getContextPath());
- b.append(target);
+
+ if (isRedirectValid(request, target)) {
+ b.append(target);
+ } else {
+ b.append("/");
+ }
if (params == null) {
params = new HashMap<String, String>();
@@ -239,6 +253,80 @@ public abstract class AbstractAuthentica
}
/**
+ * Returns <code>true</code> if the given redirect <code>target</code>
is
+ * valid according to the following list of requirements:
+ * <ul>
+ * <li>The <code>target</code> is neither <code>null</code>
nor an empty
+ * string</li>
+ * <li>The <code>target</code> is not an URL which is identified by
the
+ * character sequence <code>://</code> separating the scheme from the
+ * host</li>
+ * <li>If a <code>ResourceResolver</code> is available as a request
+ * attribute the <code>target</code> must resolve to an existing resource
+ * </li>
+ * <li>If a <code>ResourceResolver</code> is <i>not</i>
available as a
+ * request attribute the <code>target</code> must be an absolute path
+ * starting with a slash character</li>
+ * </ul>
+ * <p>
+ * If any of the conditions does not hold, the method returns
+ * <code>false</code> and logs a <i>warning</i> level message
with the
+ * <i>org.apache.sling.auth.core.spi.AbstractAuthenticationHandler</i>
+ * logger.
+ *
+ *
+ * @param request Providing the <code>ResourceResolver</code> attribute
+ * and the context to resolve the resource from the <code>target</code>.
+ * This may be <code>null</code> which cause the target to not be
+ * validated with a <code>ResoureResolver</code>
+ * @param target The redirect target to validate
+ * @return <code>true</code> if the redirect target can be considered
+ * valid
+ *
+ * @since 1.0.4 (bundle version 1.0.8)
+ */
+ public static boolean isRedirectValid(final HttpServletRequest request,
+ final String target) {
+ if (target == null || target.length() == 0) {
+ LoggerFactory.getLogger(AbstractAuthenticationHandler.class).warn(
+ "isRedirectValid: Redirect target must not be empty or null");
+ return false;
+ }
+
+ if (target.contains("://")) {
+ LoggerFactory.getLogger(AbstractAuthenticationHandler.class).warn(
+ "isRedirectValid: Redirect target '{}' must not be an URL",
+ target);
+ return false;
+ }
+
+ final int query = target.indexOf('?');
+ final String path = (query > 0) ? target.substring(0, query) : target;
+
+ if (request != null) {
+ ResourceResolver resolver = (ResourceResolver) request.getAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);
+ if (resolver != null) {
+ final boolean isValid = !ResourceUtil.isNonExistingResource(resolver.resolve(
+ request, path));
+ if (!isValid) {
+ LoggerFactory.getLogger(AbstractAuthenticationHandler.class).warn(
+ "isRedirectValid: Redirect target '{}' does not resolve to an existing
resource",
+ target);
+ }
+ return isValid;
+ }
+ }
+
+ final boolean isValid = target.startsWith("/");
+ if (!isValid) {
+ LoggerFactory.getLogger(AbstractAuthenticationHandler.class).warn(
+ "isRedirectValid: Redirect target '{}' must be an absolute path",
+ target);
+ }
+ return isValid;
+ }
+
+ /**
* Returns the name request attribute if it is a non-empty string value.
*
* @param request The request from which to retrieve the attribute
Modified: sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java?rev=1142992&r1=1142991&r2=1142992&view=diff
==============================================================================
--- sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java
(original)
+++ sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java
Tue Jul 5 11:26:45 2011
@@ -31,6 +31,29 @@ public class DefaultAuthenticationFeedba
* Handles an optional request for a redirect after successful
* authentication and <code>true</code> if the request has been redirected.
* <p>
+ * This method checks {@link AuthenticationSupport#REDIRECT_PARAMETER}
+ * request parameter for the redirect target. This parameter is handled
+ * as follows:
+ * <ul>
+ * <li>If the parameter does not exist, the method does not redirect and
+ * <code>false</code> is returned.</li>
+ * <li>If the parameter is the string <code>true</code> or is empty,
a
+ * redirect response to the request URI (<code>HttpServletRequest.getRequestURI()</code>)
+ * is sent and <code>true</code> is returned.</li>
+ * <li>If the parameter is a relative path, the path is made absolute
+ * by resolving it relative to the request URI
+ * (<code>HttpServletRequest.getRequestURI()</code>). The resulting
+ * target is validated with the
+ * {@link AbstractAuthenticationHandler#isRedirectValid(HttpServletRequest, String)}
+ * method. If valid a redirect to that target is sent back and <code>true</code>
+ * is returned. Otherwise a redirect to the servlet context root is
+ * sent back and <code>true</code> is returned.</li>
+ * <li>If the parameter is an absolute path it is validated with the
+ * {@link AbstractAuthenticationHandler#isRedirectValid(HttpServletRequest, String)}
+ * method. If valid a redirect to that path is sent back and <code>true</code>
+ * is returned. Otherwise a redirect to the servlet context root is
+ * sent back and <code>true</code> is returned.</li>
+ * <p>
* If sending the redirect response fails due to some IO problems, the
* request is still terminated but an error message is logged indicating the
* problem.
@@ -39,37 +62,24 @@ public class DefaultAuthenticationFeedba
* <code>false</code> is returned. Note, that <code>true</code>
is
* returned regardless of whether sending the redirect response
* succeeded or not.
+ *
+ * @since 1.0.4 (bundle version 1.0.8) the target is validated with the
+ * {@link AbstractAuthenticationHandler#isRedirectValid(HttpServletRequest, String)}
+ * method.
*/
public static boolean handleRedirect(final HttpServletRequest request,
final HttpServletResponse response) {
- final String redirect = request.getParameter(AuthenticationSupport.REDIRECT_PARAMETER);
+ final String redirect = getValidatedRedirectTarget(request);
if (redirect != null) {
-
- // find the redirect target
- final String target;
- if ("true".equalsIgnoreCase(redirect) || redirect.length() == 0) {
- // redirect to the same path
- target = request.getRequestURI();
-
- } else if (redirect.startsWith("/")) {
- // absolute target (in the servlet context)
- target = request.getContextPath() + redirect;
-
- } else {
- // redirect relative to the current request
- target = redirect;
-
- }
-
// and redirect ensuring the response is sent to the client
try {
- response.sendRedirect(target);
+ response.sendRedirect(redirect);
} catch (Exception e) {
// expected: IOException and IllegalStateException
LoggerFactory.getLogger(
DefaultAuthenticationFeedbackHandler.class).error(
- "handleRedirect: Failed to send redirect to " + target
+ "handleRedirect: Failed to send redirect to " + redirect
+ ", aborting request without redirect", e);
}
@@ -81,6 +91,38 @@ public class DefaultAuthenticationFeedba
return false;
}
+ private static String getValidatedRedirectTarget(
+ final HttpServletRequest request) {
+ String redirect = request.getParameter(AuthenticationSupport.REDIRECT_PARAMETER);
+ if (redirect == null) {
+ return null;
+ }
+
+ // redirect to the same path
+ if ("true".equalsIgnoreCase(redirect) || redirect.length() == 0) {
+ return request.getRequestURI();
+ }
+
+ // redirect relative to the current request (make absolute)
+ if (!redirect.startsWith("/") && !redirect.contains("://")) {
+ String path = request.getRequestURI();
+ path = path.substring(request.getContextPath().length());
+ int lastSlash = path.lastIndexOf('/');
+ path = (lastSlash > 0) ? path.substring(0, lastSlash + 1) : path;
+ redirect = path.concat(redirect);
+ }
+
+ // absolute target (in the servlet context)
+ if (!AbstractAuthenticationHandler.isRedirectValid(request, redirect)) {
+ LoggerFactory.getLogger(DefaultAuthenticationFeedbackHandler.class).error(
+ "handleRedirect: Redirect target '{}' is invalid, redirecting to '/'",
+ redirect);
+ redirect = "/";
+ }
+
+ return request.getContextPath().concat(redirect);
+ }
+
/**
* This default implementation does nothing.
* <p>
Added: sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java?rev=1142992&view=auto
==============================================================================
--- sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
(added)
+++ sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
Tue Jul 5 11:26:45 2011
@@ -0,0 +1,99 @@
+/*
+ * 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.sling.auth.core.spi;
+
+import javax.servlet.http.HttpServletRequest;
+import junit.framework.TestCase;
+
+import org.apache.sling.api.resource.NonExistingResource;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.SyntheticResource;
+import org.apache.sling.auth.core.AuthenticationSupport;
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.jmock.integration.junit4.JMock;
+import org.jmock.integration.junit4.JUnit4Mockery;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(JMock.class)
+public class AbstractAuthenticationHandlerTest {
+
+ final Mockery context = new JUnit4Mockery();
+ final ResourceResolver resolver = context.mock(ResourceResolver.class);
+ final HttpServletRequest request = context.mock(HttpServletRequest.class);
+
+ @Test
+ public void test_isRedirectValid_null_empty() {
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ null, null));
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ null, ""));
+ }
+
+ @Test
+ public void test_isRedirectValid_url() {
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ null, "http://www.google.com"));
+ }
+
+ @Test
+ public void test_isRedirectValid_no_request() {
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ null, "relative/path"));
+ TestCase.assertTrue(AbstractAuthenticationHandler.isRedirectValid(null,
+ "/absolute/path"));
+ }
+
+ @Test
+ public void test_isRedirectValid_no_resource_resolver() {
+ context.checking(new Expectations(){{
+ allowing(request).getAttribute(with(any(String.class)));
+ will(returnValue(null));
+ }});
+
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ request, "relative/path"));
+ TestCase.assertTrue(AbstractAuthenticationHandler.isRedirectValid(
+ request, "/absolute/path"));
+ }
+
+ @Test
+ public void test_isRedirectValid_resource_resolver() {
+ context.checking(new Expectations(){{
+ allowing(resolver).resolve(with(any(HttpServletRequest.class)), with(equal("/absolute/path")));
+ will(returnValue(new SyntheticResource(resolver, "/absolute/path", "test")));
+
+ allowing(resolver).resolve(with(any(HttpServletRequest.class)), with(equal("relative/path")));
+ will(returnValue(new NonExistingResource(resolver, "relative/path")));
+
+ allowing(request).getAttribute(with(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER));
+ will(returnValue(resolver));
+
+ allowing(request).getAttribute(with(any(String.class)));
+ will(returnValue(null));
+ }});
+
+ TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
+ request, "relative/path"));
+ TestCase.assertTrue(AbstractAuthenticationHandler.isRedirectValid(
+ request, "/absolute/path"));
+ }
+
+}
Propchange: sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision Rev Url
|