Author: cziegeler
Date: Mon Mar 10 09:53:48 2014
New Revision: 1575881
URL: http://svn.apache.org/r1575881
Log:
SLING-3443 : Parameter based redirection in FormAuthenticationHandler should
not handle absolute urls. Apply modified patch from Ravi Teja
Modified:
sling/trunk/bundles/auth/form/pom.xml
sling/trunk/bundles/auth/form/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
Modified: sling/trunk/bundles/auth/form/pom.xml
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/form/pom.xml?rev=1575881&r1=1575880&r2=1575881&view=diff
==============================================================================
--- sling/trunk/bundles/auth/form/pom.xml (original)
+++ sling/trunk/bundles/auth/form/pom.xml Mon Mar 10 09:53:48 2014
@@ -171,5 +171,23 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.easymock</groupId>
+ <artifactId>easymock</artifactId>
+ <version>3.2</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-module-junit4</artifactId>
+ <version>1.5.4</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-easymock</artifactId>
+ <version>1.5.4</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
Modified:
sling/trunk/bundles/auth/form/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/form/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java?rev=1575881&r1=1575880&r2=1575881&view=diff
==============================================================================
---
sling/trunk/bundles/auth/form/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
(original)
+++
sling/trunk/bundles/auth/form/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
Mon Mar 10 09:53:48 2014
@@ -37,6 +37,7 @@ import javax.servlet.http.HttpSession;
import org.apache.commons.codec.binary.Base64;
import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
import org.apache.felix.scr.annotations.Properties;
import org.apache.felix.scr.annotations.Property;
import org.apache.felix.scr.annotations.PropertyOption;
@@ -51,7 +52,6 @@ import org.apache.sling.api.resource.Res
import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.auth.core.AuthConstants;
import org.apache.sling.auth.core.AuthUtil;
-import org.apache.sling.auth.core.spi.AbstractAuthenticationHandler;
import org.apache.sling.auth.core.spi.AuthenticationHandler;
import org.apache.sling.auth.core.spi.AuthenticationInfo;
import org.apache.sling.auth.core.spi.DefaultAuthenticationFeedbackHandler;
@@ -76,7 +76,7 @@ import org.slf4j.LoggerFactory;
@Property(name = AuthenticationHandler.TYPE_PROPERTY, value =
HttpServletRequest.FORM_AUTH, propertyPrivate = true),
@Property(name = Constants.SERVICE_RANKING, intValue = 0, propertyPrivate
= false) })
@Service
-public class FormAuthenticationHandler extends AbstractAuthenticationHandler {
+public class FormAuthenticationHandler extends
DefaultAuthenticationFeedbackHandler implements AuthenticationHandler {
/**
* The name of the parameter providing the login form URL.
@@ -322,7 +322,7 @@ public class FormAuthenticationHandler e
// so that the invalid cookie isn't present on the authN
// operation.
authStorage.clear(request, response);
- if (this.loginAfterExpire || isValidateRequest(request)) {
+ if (this.loginAfterExpire ||
AuthUtil.isValidateRequest(request)) {
// signal the requestCredentials method a previous
login
// failure
request.setAttribute(FAILURE_REASON,
FormReason.TIMEOUT);
@@ -355,13 +355,13 @@ public class FormAuthenticationHandler e
return false;
}
- //check the referer to see if the request is for this handler
+ //check the referrer to see if the request is for this handler
if (!AuthUtil.checkReferer(request, loginForm)) {
//not for this handler, so return
return false;
}
- final String resource = setLoginResourceAttribute(request,
+ final String resource = AuthUtil.setLoginResourceAttribute(request,
request.getRequestURI());
if (includeLoginForm && (resourceResolverFactory != null)) {
@@ -401,7 +401,7 @@ public class FormAuthenticationHandler e
}
try {
- sendRedirect(request, response, loginForm, params);
+ AuthUtil.sendRedirect(request, response, loginForm, params);
} catch (IOException e) {
log.error("Failed to redirect to the login form " + loginForm, e);
}
@@ -477,12 +477,25 @@ public class FormAuthenticationHandler e
result = false;
} else {
// check whether redirect is requested by the resource parameter
- final String resource = getLoginResource(request, null);
- if (resource != null) {
+ final String targetResource =
AuthUtil.getLoginResource(request, null);
+ if (targetResource != null) {
try {
- response.sendRedirect(resource);
+ if (response.isCommitted()) {
+ throw new IllegalStateException("Response is
already committed");
+ }
+ response.resetBuffer();
+
+ StringBuilder b = new StringBuilder();
+ if (AuthUtil.isRedirectValid(request, targetResource)) {
+ b.append(targetResource);
+ } else if (request.getContextPath().length() == 0) {
+ b.append("/");
+ } else {
+ b.append(request.getContextPath());
+ }
+ response.sendRedirect(b.toString());
} catch (IOException ioe) {
- log.error("Failed to send redirect to: " +
resource, ioe);
+ log.error("Failed to send redirect to: " +
targetResource, ioe);
}
// terminate request, all done
@@ -593,8 +606,8 @@ public class FormAuthenticationHandler e
// as a POST request to the j_security_check page (unless
// the j_validate parameter is set); but only if this is not
// a validation request
- if (!isValidateRequest(request)) {
- setLoginResourceAttribute(request,
request.getContextPath());
+ if (!AuthUtil.isValidateRequest(request)) {
+ AuthUtil.setLoginResourceAttribute(request,
request.getContextPath());
}
}
}
@@ -731,8 +744,8 @@ public class FormAuthenticationHandler e
this.loginAfterExpire =
OsgiUtil.toBoolean(properties.get(PAR_LOGIN_AFTER_EXPIRE),
DEFAULT_LOGIN_AFTER_EXPIRE);
}
- protected void deactivate(
- @SuppressWarnings("unused") ComponentContext componentContext) {
+ @Deactivate
+ protected void deactivate() {
if (loginModule != null) {
loginModule.unregister();
loginModule = null;
Modified:
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/FormReasonTest.java?rev=1575881&r1=1575880&r2=1575881&view=diff
==============================================================================
---
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
(original)
+++
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
Mon Mar 10 09:53:48 2014
@@ -18,23 +18,24 @@
*/
package org.apache.sling.auth.form;
-import org.apache.sling.auth.form.FormReason;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
-import junit.framework.TestCase;
+import org.junit.Test;
-public class FormReasonTest extends TestCase {
+public class FormReasonTest {
- public void test_TIMEOUT() {
+ @Test public void test_TIMEOUT() {
assertEquals(FormReason.TIMEOUT,
FormReason.valueOf(FormReason.TIMEOUT.name()));
}
- public void test_INVALID_CREDENTIALS() {
+ @Test public void test_INVALID_CREDENTIALS() {
assertEquals(FormReason.INVALID_CREDENTIALS,
FormReason.valueOf(FormReason.INVALID_CREDENTIALS.name()));
}
- public void test_INVALID() {
+ @Test public void test_INVALID() {
try {
FormReason.valueOf("INVALID");
fail("unexpected result getting value of an invalid constant");
Modified:
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java?rev=1575881&r1=1575880&r2=1575881&view=diff
==============================================================================
---
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
(original)
+++
sling/trunk/bundles/auth/form/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
Mon Mar 10 09:53:48 2014
@@ -18,22 +18,42 @@
*/
package org.apache.sling.auth.form.impl;
+import static org.easymock.EasyMock.cmpEq;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertEquals;
+
import java.io.File;
+import java.lang.reflect.Method;
-import junit.framework.TestCase;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
-import org.apache.sling.auth.form.impl.FormAuthenticationHandler;
+import org.apache.sling.api.auth.Authenticator;
+import org.apache.sling.auth.core.spi.AuthenticationInfo;
+import org.apache.sling.auth.core.spi.DefaultAuthenticationFeedbackHandler;
import org.hamcrest.Description;
import org.hamcrest.text.StringStartsWith;
import org.jmock.Expectations;
import org.jmock.Mockery;
import org.jmock.api.Action;
import org.jmock.api.Invocation;
+import org.junit.Test;
+import org.junit.runner.RunWith;
import org.osgi.framework.BundleContext;
+import org.powermock.api.easymock.PowerMock;
+import org.powermock.api.support.membermodification.MemberMatcher;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(FormAuthenticationHandler.class)
+public class FormAuthenticationHandlerTest {
-public class FormAuthenticationHandlerTest extends TestCase {
-
- public void test_getTokenFile() {
+ @Test public void test_getTokenFile() {
final File root = new File("bundle999").getAbsoluteFile();
final SlingHomeAction slingHome = new SlingHomeAction();
slingHome.setSlingHome(new File("sling").getAbsolutePath());
@@ -85,7 +105,7 @@ public class FormAuthenticationHandlerTe
assertEquals(absFile, absFile0);
}
- public void test_getUserid() {
+ @Test public void test_getUserid() {
final FormAuthenticationHandler handler = new
FormAuthenticationHandler();
assertEquals(null, handler.getUserId(null));
assertEquals(null, handler.getUserId(""));
@@ -96,6 +116,61 @@ public class FormAuthenticationHandlerTe
}
/**
+ * Test for SLING-3443 Parameter based redirection should only handle
relative paths
+ * @throws Exception PowerMock.expectPrivate throws Exception and
UrlEncoder.encode
+ * throws UnsupportedEncodingException
+ * @since 1.0.6
+ */
+ @Test public void testRedirectionAfterLogin() throws Exception {
+ // Create mocks
+ final HttpServletRequest request =
createMock(HttpServletRequest.class);
+ final HttpServletResponse response =
createMock(HttpServletResponse.class);
+ final AuthenticationInfo authenticationInfo =
createMock(AuthenticationInfo.class);
+
+ // Use PowerMock to mock private method
+ final String methodName = "refreshAuthData";
+ final FormAuthenticationHandler authenticationHandler =
PowerMock.createPartialMock(FormAuthenticationHandler.class,
+ methodName);
+ final Method[] methods =
MemberMatcher.methods(FormAuthenticationHandler.class, methodName);
+ PowerMock.expectPrivate(authenticationHandler, methods[0], request,
response, authenticationInfo);
+
+ // Mock the static method since we are just unit testing the
authentication succeeded flow
+ PowerMock.mockStatic(DefaultAuthenticationFeedbackHandler.class);
+ expect(DefaultAuthenticationFeedbackHandler.handleRedirect(request,
response)).andReturn(false);
+
+ // Mocks the Authenticator.LOGIN_RESOURCE attribute
+ final String url = "http://www.blah.com";;
+
expect(request.getAttribute(Authenticator.LOGIN_RESOURCE)).andReturn(url);
+
+ // Mocks the HttpServletRequest and HttpServletResponse object
+ expect(request.getMethod()).andReturn("POST");
+
expect(request.getRequestURI()).andReturn("http://blah/blah/j_security_check";);
+ String contextPath = "/blah";
+ expect(request.getContextPath()).andReturn(contextPath).anyTimes();
+ expect(response.isCommitted()).andReturn(false);
+
+ // Mocking method with void return type
+ response.resetBuffer();
+ expectLastCall().once();
+
+ // The request should be redirected to the context root rather than the
+ // passing the parameter directly
+ response.sendRedirect(cmpEq(contextPath));
+
+ // Replay the mocks
+ replay(request);
+ replay(response);
+ replay(authenticationInfo);
+ replay(authenticationHandler);
+
+ // Test the method
+ authenticationHandler.authenticationSucceeded(request, response,
authenticationInfo);
+
+ // Verify mocks
+ verify(request, response, authenticationInfo, authenticationHandler);
+ }
+
+ /**
* The <code>RVA</code> action returns a file relative to some root file as
* requested by the first parameter of the invocation, expecting the first
* parameter to be a string.
