You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by fm...@apache.org on 2011/11/18 22:47:57 UTC
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/
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));
+ }
}