incubator-sling-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fmesc...@apache.org
Subject svn commit: r1203865 - in /sling/trunk/bundles/auth/core/src: main/java/org/apache/sling/auth/core/spi/ test/java/org/apache/sling/auth/core/ test/java/org/apache/sling/auth/core/spi/
Date Fri, 18 Nov 2011 21:47:57 GMT
Author: fmeschbe
Date: Fri Nov 18 21:47:57 2011
New Revision: 1203865

URL: http://svn.apache.org/viewvc?rev=1203865&view=rev
Log:
SLING-2126 Move tests from AbstractAuthenticationHandlerTest to AuthUtilTest for methods moved
to the new AuthUtil class. Adapt AbstractAuthenticationHandler and DefaultAuthenticationFeedbackHandler
to use the new AuthUtil class.

Added:
    sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/AuthUtilTest.java
      - copied, changed from r1198984, sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
Removed:
    sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
Modified:
    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/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=1203865&r1=1203864&r2=1203865&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
Fri Nov 18 21:47:57 2011
@@ -32,9 +32,7 @@ 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.apache.sling.auth.core.AuthUtil;
 import org.slf4j.LoggerFactory;
 
 /**
@@ -205,7 +203,7 @@ public abstract class AbstractAuthentica
      *             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.
+     *      {@link AuthUtil#isRedirectValid(HttpServletRequest, String)} method.
      */
     public static void sendRedirect(final HttpServletRequest request,
             final HttpServletResponse response, final String target,
@@ -213,7 +211,7 @@ public abstract class AbstractAuthentica
         StringBuilder b = new StringBuilder();
         b.append(request.getContextPath());
 
-        if (isRedirectValid(request, target)) {
+        if (AuthUtil.isRedirectValid(request, target)) {
             b.append(target);
         } else {
             b.append("/");
@@ -287,45 +285,10 @@ public abstract class AbstractAuthentica
      *
      * @since 1.0.4 (bundle version 1.0.8)
      */
+    @Deprecated
     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;
+        return AuthUtil.isRedirectValid(request, target);
     }
 
     /**
@@ -422,10 +385,10 @@ public abstract class AbstractAuthentica
             // TODO: log.error("Failed to send 403/Forbidden response", ioe);
         }
     }
-    
+
 	/**
 	 * Check if the request is for this authentication handler.
-	 * 
+	 *
 	 * @param request the current request
 	 * @return true if the referer matches this handler, or false otherwise
 	 */
@@ -446,5 +409,5 @@ public abstract class AbstractAuthentica
         	}
         }
         return true;
-	}    
+	}
 }

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=1203865&r1=1203864&r2=1203865&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
Fri Nov 18 21:47:57 2011
@@ -21,7 +21,9 @@ package org.apache.sling.auth.core.spi;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.sling.api.resource.ResourceUtil;
 import org.apache.sling.auth.core.AuthenticationSupport;
+import org.apache.sling.auth.core.AuthUtil;
 import org.slf4j.LoggerFactory;
 
 public class DefaultAuthenticationFeedbackHandler implements
@@ -110,10 +112,11 @@ public class DefaultAuthenticationFeedba
             int lastSlash = path.lastIndexOf('/');
             path = (lastSlash > 0) ? path.substring(0, lastSlash + 1) : path;
             redirect = path.concat(redirect);
+            redirect = ResourceUtil.normalize(redirect);
         }
 
         // absolute target (in the servlet context)
-        if (!AbstractAuthenticationHandler.isRedirectValid(request, redirect)) {
+        if (!AuthUtil.isRedirectValid(request, redirect)) {
             LoggerFactory.getLogger(DefaultAuthenticationFeedbackHandler.class).error(
                 "handleRedirect: Redirect target '{}' is invalid, redirecting to '/'",
                 redirect);

Copied: sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/AuthUtilTest.java
(from r1198984, 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/AuthUtilTest.java?p2=sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/AuthUtilTest.java&p1=sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java&r1=1198984&r2=1203865&rev=1203865&view=diff
==============================================================================
--- sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/spi/AbstractAuthenticationHandlerTest.java
(original)
+++ sling/trunk/bundles/auth/core/src/test/java/org/apache/sling/auth/core/AuthUtilTest.java
Fri Nov 18 21:47:57 2011
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.sling.auth.core.spi;
+package org.apache.sling.auth.core;
 
 import javax.servlet.http.HttpServletRequest;
 import junit.framework.TestCase;
@@ -33,67 +33,117 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 @RunWith(JMock.class)
-public class AbstractAuthenticationHandlerTest {
+public class AuthUtilTest {
 
     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, ""));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(null, null));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(null, ""));
     }
 
     @Test
     public void test_isRedirectValid_url() {
-        TestCase.assertFalse(AbstractAuthenticationHandler.isRedirectValid(
-            null, "http://www.google.com"));
+        TestCase.assertFalse(AuthUtil.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"));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(null, "relative/path"));
+        TestCase.assertTrue(AuthUtil.isRedirectValid(null, "/absolute/path"));
+    }
+
+    @Test
+    public void test_isRedirectValid_normalized() {
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized//double/slash"));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized/double/slash//"));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized/./dot"));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized/../dot"));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized/dot/."));
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "/unnormalized/dot/.."));
     }
 
     @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"));
+        context.checking(new Expectations() {
+            {
+                allowing(request).getAttribute(with(any(String.class)));
+                will(returnValue(null));
+            }
+        });
+
+        TestCase.assertFalse(AuthUtil.isRedirectValid(request, "relative/path"));
+        TestCase.assertTrue(AuthUtil.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"));
+        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(AuthUtil.isRedirectValid(request, "relative/path"));
+        TestCase.assertTrue(AuthUtil.isRedirectValid(request, "/absolute/path"));
+    }
+
+    @Test
+    public void test_isBrowserRequest_null() {
+        context.checking(new Expectations() {
+            {
+                allowing(request).getHeader(with("User-Agent"));
+                will(returnValue(null));
+            }
+        });
+        TestCase.assertFalse(AuthUtil.isBrowserRequest(request));
     }
 
+    @Test
+    public void test_isBrowserRequest_Mozilla() {
+        context.checking(new Expectations() {
+            {
+                allowing(request).getHeader(with("User-Agent"));
+                will(returnValue("This is firefox (Mozilla)"));
+            }
+        });
+        TestCase.assertTrue(AuthUtil.isBrowserRequest(request));
+    }
+
+    @Test
+    public void test_isBrowserRequest_Opera() {
+        context.checking(new Expectations() {
+            {
+                allowing(request).getHeader(with("User-Agent"));
+                will(returnValue("This is opera (Opera)"));
+            }
+        });
+        TestCase.assertTrue(AuthUtil.isBrowserRequest(request));
+    }
+
+    @Test
+    public void test_isBrowserRequest_WebDAV() {
+        context.checking(new Expectations() {
+            {
+                allowing(request).getHeader(with("User-Agent"));
+                will(returnValue("WebDAV Client"));
+            }
+        });
+        TestCase.assertFalse(AuthUtil.isBrowserRequest(request));
+    }
 }



Mime
View raw message