Skip to content

Commit

Permalink
Align POM with minimal requirements, java 11 and fix URL validation (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-doubez authored Mar 14, 2024
1 parent 33d278c commit f0d703f
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 15 deletions.
10 changes: 7 additions & 3 deletions Jenkinsfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
buildPlugin(useContainerAgent: true,
// Opt-in to the Artifact Caching Proxy, to be removed when it will be in opt-out.
// See https://github.com/jenkins-infra/helpdesk/issues/2752 for more details and updates.
artifactCachingProxyEnabled: true)
// Opt-in to the Artifact Caching Proxy, to be removed when it will be in opt-out.
// See https://github.com/jenkins-infra/helpdesk/issues/2752 for more details and updates.
artifactCachingProxyEnabled: true,
configurations: [
[platform: 'linux', jdk: 17],
[platform: 'windows', jdk: 11],
])
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>4.51</version>
<version>4.78</version>
<relativePath />
</parent>

<properties>
<revision>2.7</revision>
<changelist>-SNAPSHOT</changelist>
<gitHubRepo>jenkinsci/oic-auth-plugin</gitHubRepo>
<jenkins.version>2.346.1</jenkins.version>
<jenkins.version>2.361</jenkins.version>
<spotbugs.failOnError>true</spotbugs.failOnError>
<spotbugs.effort>Max</spotbugs.effort>
<configuration-as-code.version>1569.vb_72405b_80249</configuration-as-code.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
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;
Expand Down Expand Up @@ -734,7 +735,7 @@ protected String getValidRedirectUrl(String url) {
if (url != null && !url.isEmpty()) {
// Check if the URL is relative and starts with a slash
if (url.startsWith("/")) {
return getRootUrl() + url; // Convert to absolute URL
return URI.create(getRootUrl() + url).normalize().toString();
}
// If not relative, then check if it's a valid absolute URL
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ public class EscapeHatchCrumbExclusionTest {

private FilterChain chain = null;

private Request newRequestWithPath(String requestPath) {
return new Request(null, null) {
@Override
public String getPathInfo() {
return requestPath;
}
};
}

@Test
public void process_WithNullPath() throws IOException, ServletException {
Request request = new Request(null, null);
Expand All @@ -27,8 +36,7 @@ public void process_WithNullPath() throws IOException, ServletException {

@Test
public void process_WithWrongPath() throws IOException, ServletException {
Request request = new Request(null, null);
request.setPathInfo("fictionalPath");
Request request = newRequestWithPath("fictionalPath");
assertFalse(crumb.process(request, response, chain));
}

Expand All @@ -41,8 +49,7 @@ public void doFilter(ServletRequest arg0, ServletResponse arg1) throws IOExcepti
}
};

Request request = new Request(null, null);
request.setPathInfo("/securityRealm/escapeHatch");
Request request = newRequestWithPath("/securityRealm/escapeHatch");
assertTrue(crumb.process(request, response, chain));
}
}
17 changes: 12 additions & 5 deletions src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import hudson.util.Secret;
import java.io.IOException;
import java.net.MalformedURLException;

import org.acegisecurity.AuthenticationManager;
import org.acegisecurity.BadCredentialsException;
import org.acegisecurity.GrantedAuthority;
Expand All @@ -18,7 +16,6 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;

public class OicSecurityRealmTest {

Expand Down Expand Up @@ -89,14 +86,24 @@ public void testShouldSetNullClientSecretWhenSecretIsNone() throws IOException {

@Test
public void testGetValidRedirectUrl() throws IOException {
String rootUrl = "http://localhost:" + wireMockRule.port() + "/jenkins/";
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, realm.getValidRedirectUrl(null));
assertEquals(rootUrl, realm.getValidRedirectUrl(""));
assertThrows(MalformedURLException.class, () -> realm.getValidRedirectUrl("foobar"));
assertEquals(rootUrl, realm.getValidRedirectUrl("foobar"));
}

@Test
public void testShouldReturnRootUrlWhenRedirectUrlIsInvalid() throws IOException {
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/"));
}
}

0 comments on commit f0d703f

Please sign in to comment.