You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2017/11/07 09:19:16 UTC
[sling-org-apache-sling-auth-form] 05/10: SLING-3443 : Parameter
based redirection in FormAuthenticationHandler should not handle absolute
urls. Apply modified patch from Ravi Teja
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to annotated tag org.apache.sling.auth.form-1.0.6
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-form.git
commit e80937a2def68841a7d9b18b0a6b3a9e504b071b
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Mar 10 09:53:48 2014 +0000
SLING-3443 : Parameter based redirection in FormAuthenticationHandler should not handle absolute urls. Apply modified patch from Ravi Teja
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/auth/form@1575881 13f79535-47bb-0310-9956-ffa450edef68
---
pom.xml | 18 +++++
.../auth/form/impl/FormAuthenticationHandler.java | 41 +++++++----
.../org/apache/sling/auth/form/FormReasonTest.java | 13 ++--
.../form/impl/FormAuthenticationHandlerTest.java | 85 ++++++++++++++++++++--
4 files changed, 132 insertions(+), 25 deletions(-)
diff --git a/pom.xml b/pom.xml
index 684238f..bbfbab5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -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>
diff --git a/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java b/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
index 03938af..361a11d 100644
--- a/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
+++ b/src/main/java/org/apache/sling/auth/form/impl/FormAuthenticationHandler.java
@@ -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.ResourceResolver;
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 extends AbstractAuthenticationHandler {
// 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 extends AbstractAuthenticationHandler {
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 extends AbstractAuthenticationHandler {
}
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 extends AbstractAuthenticationHandler {
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 extends AbstractAuthenticationHandler {
// 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 extends AbstractAuthenticationHandler {
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;
diff --git a/src/test/java/org/apache/sling/auth/form/FormReasonTest.java b/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
index 4409621..d756179 100644
--- a/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
+++ b/src/test/java/org/apache/sling/auth/form/FormReasonTest.java
@@ -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");
diff --git a/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java b/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
index 589bf30..2dfe487 100644
--- a/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
+++ b/src/test/java/org/apache/sling/auth/form/impl/FormAuthenticationHandlerTest.java
@@ -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;
-public class FormAuthenticationHandlerTest extends TestCase {
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(FormAuthenticationHandler.class)
+public class FormAuthenticationHandlerTest {
- 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 FormAuthenticationHandlerTest extends TestCase {
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 FormAuthenticationHandlerTest extends TestCase {
}
/**
+ * 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.
--
To stop receiving notification emails like this one, please contact
"commits@sling.apache.org" <co...@sling.apache.org>.