Fix invalid redirect computation (fix #285)

jenkinsci · Apr 2, 2024 · 3661066 · 3661066
1 parent fca62d3
commit 3661066
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 21 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
@@ -65,7 +65,6 @@
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Field;
 import java.net.MalformedURLException;
-import java.net.URI;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.nio.charset.Charset;
@@ -776,26 +775,19 @@ protected AuthorizationCodeFlow buildAuthorizationCodeFlow() {
     }
 
     protected String getValidRedirectUrl(String url) {
+        final String rootUrl = getRootUrl();
         if (url != null && !url.isEmpty()) {
-            // Check if the URL is relative and starts with a slash
-            if (url.startsWith("/")) {
-                return URI.create(getRootUrl() + url).normalize().toString();
-            }
-            // If not relative, then check if it's a valid absolute URL
             try {
-                URL parsedUrl = new URL(url);
-                String host = parsedUrl.getHost();
-                String expectedHost = new URL(getRootUrl()).getHost();
-                // Check if the host matches the Jenkins domain
-                if (host.equals(expectedHost)) {
-                    return url; // The URL is absolute and valid
+                final String redirectUrl = new URL(new URL(rootUrl), url).toString();
+                // check redirect url stays within rootUrl
+                if (redirectUrl.startsWith(rootUrl)) {
+                    return redirectUrl;
                 }
             } catch (MalformedURLException e) {
-                // Invalid absolute URL, will return root URL
+                // Invalid URL, will return root URL
             }
         }
-        // If the URL is null, empty, or invalid, return the root URL
-        return getRootUrl();
+        return rootUrl;
     }
 
     /**

diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java
@@ -86,24 +86,31 @@ public void testShouldSetNullClientSecretWhenSecretIsNone() throws IOException {
 
     @Test
     public void testGetValidRedirectUrl() throws IOException {
-        String rootUrl = jenkinsRule.jenkins.getRootUrl();
+        // root url is http://localhost:????/jenkins/
+        final String rootUrl = jenkinsRule.jenkins.getRootUrl();
 
         TestRealm realm = new TestRealm.Builder(wireMockRule)
                 .WithMinimalDefaults().build();
-        assertEquals(rootUrl + "foo", realm.getValidRedirectUrl("/foo"));
-        assertEquals(rootUrl + "bar", realm.getValidRedirectUrl(rootUrl + "bar"));
+
+        assertEquals(rootUrl + "foo", realm.getValidRedirectUrl("foo"));
+        assertEquals(rootUrl + "foo", realm.getValidRedirectUrl("/jenkins/foo"));
+        assertEquals(rootUrl + "foo", realm.getValidRedirectUrl(rootUrl + "foo"));
         assertEquals(rootUrl, realm.getValidRedirectUrl(null));
         assertEquals(rootUrl, realm.getValidRedirectUrl(""));
-        assertEquals(rootUrl, realm.getValidRedirectUrl("foobar"));
     }
 
     @Test
     public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws IOException {
+        // root url is http://localhost:????/jenkins/
         String rootUrl = jenkinsRule.jenkins.getRootUrl();
 
         TestRealm realm = new TestRealm.Builder(wireMockRule)
                 .WithMinimalDefaults().build();
-        assertEquals(rootUrl, realm.getValidRedirectUrl("foobar"));
-        assertEquals(rootUrl, realm.getValidRedirectUrl("https://another-host/"));
+
+        assertEquals(rootUrl, realm.getValidRedirectUrl("/bar"));
+        assertEquals(rootUrl, realm.getValidRedirectUrl("../bar"));
+        assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/"));
+        assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/bar/"));
+        assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/jenkins/../bar/"));
     }
 }