diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 69ac76ad..74150fa8 100644 --- 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 index 35542238..e2f9be26 100644 --- 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/")); } }