incubator-sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fmesc...@apache.org
Subject svn commit: r1142992 - in /sling/trunk/bundles/auth/core: ./ src/main/java/org/apache/sling/auth/core/spi/ src/test/java/org/apache/sling/auth/core/spi/
Date Tue, 05 Jul 2011 11:26:46 GMT
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



Mime
View raw message