You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2021/06/03 21:31:23 UTC
[sling-org-apache-sling-jcr-jackrabbit-usermanager] branch master
updated: SLING-10456 adjust HTTP status code for invalid :redirect value
(#6)
This is an automated email from the ASF dual-hosted git repository.
enorman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-jackrabbit-usermanager.git
The following commit(s) were added to refs/heads/master by this push:
new cfdde23 SLING-10456 adjust HTTP status code for invalid :redirect value (#6)
cfdde23 is described below
commit cfdde23fbd2bc4b486084ba7e27d42b3187dd565
Author: Eric Norman <er...@gmail.com>
AuthorDate: Thu Jun 3 14:31:16 2021 -0700
SLING-10456 adjust HTTP status code for invalid :redirect value (#6)
for usermanager post requests
---
.../usermanager/impl/post/AbstractPostServlet.java | 33 +++++++------
.../usermanager/it/post/CreateGroupIT.java | 25 ++++++++++
.../usermanager/it/post/CreateUserIT.java | 26 ++++++++++
.../usermanager/it/post/RemoveAuthorizablesIT.java | 27 ++++++++++
.../usermanager/it/post/UpdateGroupIT.java | 28 +++++++++++
.../usermanager/it/post/UpdateUserIT.java | 57 ++++++++++++++++++++++
.../it/post/UserManagerClientTestSupport.java | 2 +
7 files changed, 183 insertions(+), 15 deletions(-)
diff --git a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/AbstractPostServlet.java b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/AbstractPostServlet.java
index 33ad48c..542baa4 100644
--- a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/AbstractPostServlet.java
+++ b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/AbstractPostServlet.java
@@ -157,9 +157,20 @@ public abstract class AbstractPostServlet extends
// check for redirect URL if processing succeeded
if (response.isSuccessful()) {
- String redirect = getRedirectUrl(request, response);
+ String redirect = null;
+ try {
+ redirect = getRedirectUrl(request, response);
+ } catch (IOException e) {
+ if (log.isDebugEnabled()) {
+ log.debug(String.format("Exception while handling redirect for POST %s with %s",
+ request.getResource().getPath(), getClass().getName()), e);
+ }
+ // http status code for 422 Unprocessable Entity
+ response.setStatus(422, "invalid redirect");
+ response.setError(e);
+ }
if (redirect != null) {
- httpResponse.sendRedirect(redirect);
+ httpResponse.sendRedirect(redirect); // NOSONAR
return;
}
}
@@ -264,7 +275,7 @@ public abstract class AbstractPostServlet extends
* @deprecated use {@link #getRedirectUrl(HttpServletRequest, PostResponse)} instead
*/
@Deprecated
- protected String getRedirectUrl(HttpServletRequest request, AbstractPostResponse ctx) {
+ protected String getRedirectUrl(HttpServletRequest request, AbstractPostResponse ctx) throws IOException {
return getRedirectUrl(request, (PostResponse)ctx);
}
@@ -273,8 +284,9 @@ public abstract class AbstractPostServlet extends
* @param request the request
* @param ctx the post processor
* @return the redirect location or <code>null</code>
+ * @throws IOException if there is something invalid with the :redirect value
*/
- protected String getRedirectUrl(HttpServletRequest request, PostResponse ctx) {
+ protected String getRedirectUrl(HttpServletRequest request, PostResponse ctx) throws IOException {
// redirect param has priority (but see below, magic star)
String result = request.getParameter(SlingPostConstants.RP_REDIRECT_TO);
if (result != null) {
@@ -282,16 +294,11 @@ public abstract class AbstractPostServlet extends
URI redirectUri = new URI(result);
if (redirectUri.getAuthority() != null) {
// if it has a host information
- log.warn("redirect target includes host information ({}). This is not allowed for security reasons!", redirectUri.getAuthority());
- result = null;
+ throw new IOException("The redirect target included host information. This is not allowed for security reasons!");
}
} catch (URISyntaxException e) {
- log.warn("given redirect target is not a valid uri: {}", e.getLocalizedMessage());
- result = null;
+ throw new IOException("The redirect target was not a valid uri");
}
- }
- if (result != null) {
- log.debug("redirect requested as [{}] for path [{}]", result, ctx.getPath());
if (ctx.getPath() != null) {
// redirect to created/modified Resource
@@ -320,10 +327,6 @@ public abstract class AbstractPostServlet extends
// name
result = result.concat(ResourceUtil.getName(ctx.getPath()));
}
-
- if (log.isDebugEnabled()) {
- log.debug("Will redirect to {}", result);
- }
}
}
return result;
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateGroupIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateGroupIT.java
index 84e40fd..3c06703 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateGroupIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateGroupIT.java
@@ -182,4 +182,29 @@ public class CreateGroupIT extends UserManagerClientTestSupport {
assertNotNull(jsonObj);
}
+ private void testCreateGroupRedirect(String redirectTo, int expectedStatus) throws IOException {
+ String postUrl = String.format("%s/system/userManager/group.create.html", baseServerUri);
+
+ testGroupId = "testGroup" + getNextInt();
+ List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair(":name", testGroupId));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+ assertAuthenticatedAdminPostStatus(postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testCreateGroupValidRedirect() throws IOException, JsonException {
+ testCreateGroupRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testCreateGroupInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testCreateGroupRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testCreateGroupInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testCreateGroupRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
}
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateUserIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateUserIT.java
index cfe2e30..acf8e6e 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateUserIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/CreateUserIT.java
@@ -277,4 +277,30 @@ public class CreateUserIT extends UserManagerClientTestSupport {
assertEquals("Thanks!", content); //verify that the content matches the custom response
}
+ private void testCreateUserRedirect(String redirectTo, int expectedStatus) throws IOException {
+ testUserId = "testUser" + getNextInt();
+ String postUrl = String.format("%s/system/userManager/user.create.html", baseServerUri);
+ final List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair(":name", testUserId));
+ postParams.add(new BasicNameValuePair("pwd", "testPwd"));
+ postParams.add(new BasicNameValuePair("pwdConfirm", "testPwd"));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+ assertAuthenticatedAdminPostStatus(postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testCreateUserValidRedirect() throws IOException, JsonException {
+ testCreateUserRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testCreateUserInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testCreateUserRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testCreateUserInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testCreateUserRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
}
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/RemoveAuthorizablesIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/RemoveAuthorizablesIT.java
index 6b615c3..bae6671 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/RemoveAuthorizablesIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/RemoveAuthorizablesIT.java
@@ -260,4 +260,31 @@ public class RemoveAuthorizablesIT extends UserManagerClientTestSupport {
assertNotNull(jsonObj);
}
+ private void testRemoveAuthorizablesRedirect(String redirectTo, int expectedStatus) throws IOException {
+ String userId = createTestUser();
+ String groupId = createTestGroup();
+
+ String postUrl = String.format("%s/system/userManager.delete.html", baseServerUri);
+ List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair(":applyTo", "group/" + groupId));
+ postParams.add(new BasicNameValuePair(":applyTo", "user/" + userId));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+ assertAuthenticatedAdminPostStatus(postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testRemoveAuthorizableValidRedirect() throws IOException, JsonException {
+ testRemoveAuthorizablesRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testRemoveAuthorizableInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testRemoveAuthorizablesRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testRemoveAuthorizableInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testRemoveAuthorizablesRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
}
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateGroupIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateGroupIT.java
index 0dcdab9..09f50cc 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateGroupIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateGroupIT.java
@@ -258,5 +258,33 @@ public class UpdateGroupIT extends UserManagerClientTestSupport {
assertNotNull(jsonObj);
}
+ private void testUpdateGroupRedirect(String redirectTo, int expectedStatus) throws IOException {
+ testGroupId = createTestGroup();
+
+ String postUrl = String.format("%s/system/userManager/group/%s.update.html", baseServerUri, testGroupId);
+
+ List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair("displayName", "My Updated Test Group"));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+
+ Credentials creds = new UsernamePasswordCredentials("admin", "admin");
+ assertAuthenticatedPostStatus(creds, postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testUpdateGroupValidRedirect() throws IOException, JsonException {
+ testUpdateGroupRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testUpdateGroupInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testUpdateGroupRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testUpdateGroupInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testUpdateGroupRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
}
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateUserIT.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateUserIT.java
index f44271a..b23dc7a 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateUserIT.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UpdateUserIT.java
@@ -376,4 +376,61 @@ public class UpdateUserIT extends UserManagerClientTestSupport {
httpContext.getCookieStore().clear();
}
+ private void testChangeUserPasswordRedirect(String redirectTo, int expectedStatus) throws IOException {
+ testUserId = createTestUser();
+
+ String postUrl = String.format("%s/system/userManager/user/%s.changePassword.html", baseServerUri, testUserId);
+
+ List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair("oldPwd", "testPwd"));
+ postParams.add(new BasicNameValuePair("newPwd", "testNewPwd"));
+ postParams.add(new BasicNameValuePair("newPwdConfirm", "testNewPwd"));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+
+ Credentials creds = new UsernamePasswordCredentials(testUserId, "testPwd");
+ assertAuthenticatedPostStatus(creds, postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testChangeUserPasswordValidRedirect() throws IOException, JsonException {
+ testChangeUserPasswordRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testChangeUserPasswordInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testChangeUserPasswordRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testChangeUserPasswordInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testChangeUserPasswordRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ private void testUpdateUserRedirect(String redirectTo, int expectedStatus) throws IOException {
+ testUserId = createTestUser();
+
+ String postUrl = String.format("%s/system/userManager/user/%s.update.html", baseServerUri, testUserId);
+
+ List<NameValuePair> postParams = new ArrayList<>();
+ postParams.add(new BasicNameValuePair("displayName", "My Updated Test User"));
+ postParams.add(new BasicNameValuePair(":redirect", redirectTo));
+ Credentials creds = new UsernamePasswordCredentials(testUserId, "testPwd");
+ assertAuthenticatedPostStatus(creds, postUrl, expectedStatus, postParams, null);
+ }
+
+ @Test
+ public void testUpdateUserValidRedirect() throws IOException, JsonException {
+ testUpdateUserRedirect("/*.html", HttpServletResponse.SC_MOVED_TEMPORARILY);
+ }
+
+ @Test
+ public void testUpdateUserInvalidRedirectWithAuthority() throws IOException, JsonException {
+ testUpdateUserRedirect("https://sling.apache.org", SC_UNPROCESSABLE_ENTITY);
+ }
+
+ @Test
+ public void testUpdateUserInvalidRedirectWithInvalidURI() throws IOException, JsonException {
+ testUpdateUserRedirect("https://", SC_UNPROCESSABLE_ENTITY);
+ }
+
}
diff --git a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UserManagerClientTestSupport.java b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UserManagerClientTestSupport.java
index 219b64f..64c96cc 100644
--- a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UserManagerClientTestSupport.java
+++ b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/UserManagerClientTestSupport.java
@@ -86,6 +86,8 @@ import org.osgi.service.cm.ConfigurationAdmin;
* servlets
*/
public abstract class UserManagerClientTestSupport extends UserManagerTestSupport {
+ protected static final int SC_UNPROCESSABLE_ENTITY = 422; // http status code for 422 Unprocessable Entity
+
protected static final String TEST_FOLDER_JSON = "{'jcr:primaryType': 'nt:unstructured'}";
protected static final String CONTENT_TYPE_JSON = "application/json";