(sling-org-apache-sling-auth-oauth-client) branch master updated: SLING-12982 Restrict possible values of redirect parameter (#34)

[rombert]

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch master
in repository 
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-oauth-client.git


The following commit(s) were added to refs/heads/master by this push:
     new 15d63fd  SLING-12982 Restrict possible values of redirect parameter 
(#34)
15d63fd is described below

commit 15d63fdccba93c046c871bb1a5afdd6a5208ff19
Author: Nicola Scendoni <nscend...@adobe.com>
AuthorDate: Wed Oct 29 10:49:13 2025 +0100

    SLING-12982 Restrict possible values of redirect parameter (#34)
---
 .../oauth_client/impl/OAuthEntryPointServlet.java     | 19 ++++++++++++++++---
 .../oauth_client/impl/OAuthEntryPointServletTest.java | 13 +++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git 
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
 
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
index c5270f9..4f7c7a9 100644
--- 
a/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
+++ 
b/src/main/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServlet.java
@@ -22,7 +22,6 @@ import javax.servlet.Servlet;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
-import java.io.IOException;
 import java.net.URI;
 import java.util.List;
 import java.util.Map;
@@ -72,7 +71,7 @@ public class OAuthEntryPointServlet extends 
SlingAllMethodsServlet {
 
     @Override
     protected void doGet(@NotNull SlingHttpServletRequest request, @NotNull 
SlingHttpServletResponse response)
-            throws ServletException, IOException {
+            throws ServletException {
 
         try {
             String desiredConnectionName = request.getParameter("c");
@@ -105,11 +104,13 @@ public class OAuthEntryPointServlet extends 
SlingAllMethodsServlet {
     }
 
     private @NotNull RedirectTarget getAuthenticationRequestUri(
-            @NotNull ClientConnection connection, @NotNull 
SlingHttpServletRequest request, @NotNull URI callbackUri) {
+            @NotNull ClientConnection connection, @NotNull 
SlingHttpServletRequest request, @NotNull URI callbackUri)
+            throws OAuthEntryPointException {
         ResolvedConnection conn = ResolvedOAuthConnection.resolve(connection);
 
         // TODO: Should we redirect to the target url when redirect is null?
         String redirect = 
request.getParameter(RedirectHelper.PARAMETER_NAME_REDIRECT);
+        validateRedirect(redirect);
 
         String perRequestKey = new Identifier().getValue();
         OAuthCookieValue oAuthCookieValue = new 
OAuthCookieValue(perRequestKey, connection.name(), redirect);
@@ -117,4 +118,16 @@ public class OAuthEntryPointServlet extends 
SlingAllMethodsServlet {
         return RedirectHelper.buildRedirectTarget(
                 new String[] {PATH}, callbackUri, conn, oAuthCookieValue, 
cryptoService);
     }
+
+    private void validateRedirect(String redirect) throws 
OAuthEntryPointException {
+        // Validate that it is not a cross-site redirect
+        if (redirect == null || redirect.isEmpty()) {
+            return;
+        }
+        if (!redirect.startsWith("/")) {
+            String message = "Invalid redirect URL: " + redirect;
+            // Relative redirect within the same domain is allowed
+            throw new OAuthEntryPointException(message, new 
IllegalArgumentException(message));
+        }
+    }
 }
diff --git 
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
 
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
index f86d008..bc7cda0 100644
--- 
a/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
+++ 
b/src/test/java/org/apache/sling/auth/oauth_client/impl/OAuthEntryPointServletTest.java
@@ -36,6 +36,7 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertThrows;
 
 @ExtendWith(SlingContextExtension.class)
 class OAuthEntryPointServletTest {
@@ -94,6 +95,18 @@ class OAuthEntryPointServletTest {
         assertThat(location).as("authentication request 
uri").hasParameter("access_type", "offline");
     }
 
+    @Test
+    void redirectWithValidConnectionAndInvalidRedirect() throws 
ServletException, IOException {
+
+        context.request().setQueryString("c=" + MOCK_OIDC_PARAM + 
"&redirect=http://invalid-url";);
+        MockSlingHttpServletResponse response = context.response();
+
+        OAuthEntryPointException exception =
+                assertThrows(OAuthEntryPointException.class, () -> 
servlet.service(context.request(), response));
+
+        assertThat(exception.getMessage()).as("Expected exception 
message").contains("Internal error");
+    }
+
     @Test
     void missingConnectionParameter() throws ServletException, IOException {