diff --git a/docs/configuration/README.md b/docs/configuration/README.md index 53c2a839..b0df21e5 100644 --- a/docs/configuration/README.md +++ b/docs/configuration/README.md @@ -103,6 +103,16 @@ They are called claims in OpenID Connect terminology. | emailFieldName | jmes path | claim to use for populating user email | | groupsFieldName | jmes path | groups the user belongs to | +## Custom Query Parameters For Login and Logout Endpoints + +Optional list of key / value query parameter pairs which will be appended +when calling the login resp. the logout endpoint. + +| field | format | description | +|-----------------|--------|--------------------------------------------------------------------| +| queryParamName | string | Name of the query parameter. | +| queryParamValue | string | Value of the query parameter. If empty, only the key will be sent. | + ## JCasC configuration reference @@ -142,6 +152,12 @@ jenkins: rootURLFromRequest: sendScopesInTokenRequest: postLogoutRedirectUrl: + loginQueryParamNameValuePairs: + - queryParamName: + queryParamValue: + logoutQueryParamNameValuePairs: + - queryParamName: + queryParamValue: # Security allowTokenAccessWithoutOicSession: allowedTokenExpirationClockSkewSeconds: diff --git a/docs/images/global-config.png b/docs/images/global-config.png index 34fdb631..31817540 100644 Binary files a/docs/images/global-config.png and b/docs/images/global-config.png differ diff --git a/src/main/java/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable.java b/src/main/java/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable.java new file mode 100644 index 00000000..718670ae --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable.java @@ -0,0 +1,88 @@ +package org.jenkinsci.plugins.oic; + +import hudson.model.AbstractDescribableImpl; +import hudson.model.Descriptor; +import hudson.util.FormValidation; +import hudson.util.FormValidation.Kind; +import org.apache.commons.lang3.StringUtils; +import org.kohsuke.stapler.QueryParameter; + +public abstract class AbstractKeyValueDescribable> + extends AbstractDescribableImpl { + + private final String key; + private final String value; + + /** + * Create a new instance with the provided key/value combination. + * @param key non-blank String to use as the key, will be {@code trim}ed before persisting + * @param value non-blank string for the value, will be {@code trim}ed before persisting + * @throws Descriptor.FormException if either key/value are {@code null} or are not valid values + */ + public AbstractKeyValueDescribable(String key, String value) throws Descriptor.FormException { + this(key, value, false); + } + + /** + * Create a new instance with the provided key/value combination. + * @param key non-blank String to use as the key, will be {@code trim}ed before persisting + * @param value possibly blank string for the value, will be {@code trim}ed before persisting + * @param allowBlankValue {@code true} it {@code value} may be blank (but not null) + * @throws Descriptor.FormException if either key/value are {@code null} or are not valid values + */ + public AbstractKeyValueDescribable(String key, String value, boolean allowBlankValue) + throws Descriptor.FormException { + // formValidation should not error for blank entries so we need to explicitly check them + if (StringUtils.isBlank(key)) { + throw new Descriptor.FormException("key must not be blank", "key"); + } + if (!allowBlankValue && StringUtils.isBlank(value)) { + throw new Descriptor.FormException("value must not be blank", "value"); + } + + FormValidation keyValidation = getDescriptor().doCheckKey(key); + if (keyValidation.kind == Kind.ERROR) { + throw new Descriptor.FormException(keyValidation.getMessage(), "key"); + } + FormValidation valueValidation = getDescriptor().doCheckValue(value); + if (valueValidation.kind == Kind.ERROR) { + throw new Descriptor.FormException(valueValidation.getMessage(), "value"); + } + this.key = key.trim(); + this.value = value == null ? "" : value.trim(); + } + + public String getKey() { + return key; + } + + public String getValue() { + return value; + } + + @Override + public DescriptorImpl getDescriptor() { + return (DescriptorImpl) super.getDescriptor(); + } + + public static class DescriptorImpl> extends Descriptor { + + /** + * Check the key for validity. + * In addition to being used by the UI, any FormValidation of {@code Kind.ERROR} will cause a fail to create the describable. + * By default, this method returns {@link FormValidation#ok()}, subclasses should override this in order to provide any required checking. + */ + public FormValidation doCheckKey(@SuppressWarnings("unused") @QueryParameter String key) { + return FormValidation.ok(); + } + + /** + * Check the key for validity. + * In addition to being used by the UI, any FormValidation of {@code Kind.ERROR} will cause a fail to create the describable. + * By default this method returns {@link FormValidation#ok()}, subclasses should override this in order to provide any required checking. + */ + public FormValidation doCheckValue(@SuppressWarnings("unused") @QueryParameter String value) { + return FormValidation.ok(); + } + } +} diff --git a/src/main/java/org/jenkinsci/plugins/oic/AbstractQueryParameter.java b/src/main/java/org/jenkinsci/plugins/oic/AbstractQueryParameter.java new file mode 100644 index 00000000..dfa070a7 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/AbstractQueryParameter.java @@ -0,0 +1,31 @@ +package org.jenkinsci.plugins.oic; + +import hudson.model.Descriptor; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + +public abstract class AbstractQueryParameter> + extends AbstractKeyValueDescribable { + + public AbstractQueryParameter(String key, String value) throws Descriptor.FormException { + super(key, value); + } + + public AbstractQueryParameter(String key, String value, boolean allowEmptyValue) throws Descriptor.FormException { + super(key, value, allowEmptyValue); + } + + /** + * Return {@link #getKey()} encoded with {@code application/x-www-form-urlencoded} in {@code UTF-8} + */ + public String getURLEncodedKey() { + return URLEncoder.encode(getKey(), StandardCharsets.UTF_8); + } + + /** + * Return {@link #getValue()} encoded with {@code application/x-www-form-urlencoded} in {@code UTF-8} + */ + public String getURLEncodedValue() { + return URLEncoder.encode(getValue(), StandardCharsets.UTF_8); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/oic/LoginQueryParameter.java b/src/main/java/org/jenkinsci/plugins/oic/LoginQueryParameter.java new file mode 100644 index 00000000..2418d1db --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/LoginQueryParameter.java @@ -0,0 +1,38 @@ +package org.jenkinsci.plugins.oic; + +import hudson.Extension; +import hudson.model.Descriptor.FormException; +import hudson.util.FormValidation; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.QueryParameter; +import org.pac4j.oidc.config.OidcConfiguration; + +public class LoginQueryParameter extends AbstractQueryParameter { + + @DataBoundConstructor + public LoginQueryParameter(String key, String value) throws FormException { + super(key, value); + } + + @Extension + public static class DescriptorImpl extends AbstractKeyValueDescribable.DescriptorImpl { + + @Override + public FormValidation doCheckKey(@QueryParameter String key) { + return switch (key.trim()) { + case OidcConfiguration.SCOPE, + OidcConfiguration.RESPONSE_TYPE, + OidcConfiguration.RESPONSE_MODE, + OidcConfiguration.REDIRECT_URI, + OidcConfiguration.CLIENT_ID, + OidcConfiguration.STATE, + OidcConfiguration.MAX_AGE, + OidcConfiguration.PROMPT, + OidcConfiguration.NONCE, + OidcConfiguration.CODE_CHALLENGE, + OidcConfiguration.CODE_CHALLENGE_METHOD -> FormValidation.error(key + " is a reserved word"); + default -> FormValidation.ok(); + }; + } + } +} diff --git a/src/main/java/org/jenkinsci/plugins/oic/LogoutQueryParameter.java b/src/main/java/org/jenkinsci/plugins/oic/LogoutQueryParameter.java new file mode 100644 index 00000000..c1fe89d5 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/LogoutQueryParameter.java @@ -0,0 +1,28 @@ +package org.jenkinsci.plugins.oic; + +import hudson.Extension; +import hudson.model.Descriptor.FormException; +import hudson.util.FormValidation; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.QueryParameter; + +public class LogoutQueryParameter extends AbstractQueryParameter { + + @DataBoundConstructor + public LogoutQueryParameter(String key, String value) throws FormException { + super(key, value, true); + } + + @Extension + public static class DescriptorImpl extends AbstractKeyValueDescribable.DescriptorImpl { + + @Override + public FormValidation doCheckKey(@QueryParameter String key) { + return switch (key.trim()) { + case "id_token_hint", "state", "post_logout_redirect_uri" -> FormValidation.error( + key + " is a reserved word"); + default -> FormValidation.ok(); + }; + } + } +} diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 847275ef..20fbb7d0 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -81,13 +81,17 @@ import java.util.ArrayList; import java.util.Base64; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Random; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; +import java.util.stream.Collectors; import javax.annotation.PostConstruct; import jenkins.model.IdStrategy; import jenkins.model.IdStrategyDescriptor; @@ -317,6 +321,12 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { */ private transient ProxyAwareResourceRetriever proxyAwareResourceRetriever; + @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "we are not using standard serialization") + private List loginQueryParameters; + + @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "we are not using standard serialization") + private List logoutQueryParameters; + @DataBoundConstructor public OicSecurityRealm( String clientId, @@ -417,6 +427,24 @@ protected Object readResolve() throws ObjectStreamException { return this; } + @DataBoundSetter + public void setLoginQueryParameters(List values) { + this.loginQueryParameters = values; + } + + public List getLoginQueryParameters() { + return loginQueryParameters; + } + + @DataBoundSetter + public void setLogoutQueryParameters(List values) { + this.logoutQueryParameters = values; + } + + public List getLogoutQueryParameters() { + return logoutQueryParameters; + } + public String getClientId() { return clientId; } @@ -588,6 +616,11 @@ protected TokenValidator createTokenValidator() { conf.setDisablePkce(true); } opMetadataResolver.init(); + if (loginQueryParameters != null && !loginQueryParameters.isEmpty()) { + for (LoginQueryParameter lqp : loginQueryParameters) { + conf.addCustomParam(lqp.getURLEncodedKey(), lqp.getURLEncodedValue()); + } + } return conf; } @@ -1273,22 +1306,44 @@ Object getStateAttribute(HttpSession session) { } @CheckForNull - private String maybeOpenIdLogoutEndpoint(String idToken, String state, String postLogoutRedirectUrl) { + String maybeOpenIdLogoutEndpoint(String idToken, String state, String postLogoutRedirectUrl) { final URI url = serverConfiguration.toProviderMetadata().getEndSessionEndpointURI(); if (this.logoutFromOpenidProvider && url != null) { - StringBuilder openidLogoutEndpoint = new StringBuilder(url.toString()); - + Map segmentsMap = new HashMap<>(); + Set segmentsSet = new HashSet<>(); if (!Strings.isNullOrEmpty(idToken)) { - openidLogoutEndpoint.append("?id_token_hint=").append(idToken).append("&"); - } else { - openidLogoutEndpoint.append("?"); + segmentsMap.put("id_token_hint", idToken); + } + if (!Strings.isNullOrEmpty(state) && !"null".equals(state)) { + segmentsMap.put("state", state); } - openidLogoutEndpoint.append("state=").append(state); - if (postLogoutRedirectUrl != null) { - openidLogoutEndpoint - .append("&post_logout_redirect_uri=") - .append(URLEncoder.encode(postLogoutRedirectUrl, StandardCharsets.UTF_8)); + segmentsMap.put( + "post_logout_redirect_uri", URLEncoder.encode(postLogoutRedirectUrl, StandardCharsets.UTF_8)); + } + if (logoutQueryParameters != null && !logoutQueryParameters.isEmpty()) { + logoutQueryParameters.forEach(lqp -> { + String key = lqp.getURLEncodedKey(); + String value = lqp.getURLEncodedValue(); + if (value.isEmpty()) { + segmentsSet.add(key); + } else { + segmentsMap.put(key, value); + } + }); + } + + StringBuilder openidLogoutEndpoint = new StringBuilder(url.toString()); + String concatChar = openidLogoutEndpoint.toString().contains("?") ? "&" : "?"; + if (!segmentsMap.isEmpty()) { + String joinedString = segmentsMap.entrySet().stream() + .map(entry -> entry.getKey() + "=" + entry.getValue()) + .collect(Collectors.joining("&")); + openidLogoutEndpoint.append(concatChar).append(joinedString); + concatChar = "&"; + } + if (!segmentsSet.isEmpty()) { + openidLogoutEndpoint.append(concatChar).append(String.join("&", segmentsSet)); } return openidLogoutEndpoint.toString(); } diff --git a/src/main/resources/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable/config.jelly b/src/main/resources/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable/config.jelly new file mode 100644 index 00000000..80ca0409 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable/config.jelly @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable/config.properties b/src/main/resources/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable/config.properties new file mode 100644 index 00000000..e69de29b diff --git a/src/main/resources/org/jenkinsci/plugins/oic/Messages.properties b/src/main/resources/org/jenkinsci/plugins/oic/Messages.properties index ca952cd7..bd747ce0 100644 --- a/src/main/resources/org/jenkinsci/plugins/oic/Messages.properties +++ b/src/main/resources/org/jenkinsci/plugins/oic/Messages.properties @@ -1,5 +1,8 @@ OicLogoutAction.OicLogout = Oic Logout +OicQueryParameterConfiguration.QueryParameterNameRequired = Query parameter name is required. +OicQueryParameterConfiguration.QueryParameterValueRequired = Query parameter value is required. + OicSecurityRealm.DisplayName = Login with Openid Connect OicSecurityRealm.CouldNotRefreshToken = Unable to refresh access token OicSecurityRealm.ClientIdRequired = Client id is required. diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly index 313525b4..d0e64dcb 100644 --- a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.jelly @@ -32,6 +32,28 @@ + + + + + +
+ +
+
+
+
+ + + +
+ +
+
+
+
+
+ diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties index a52f4249..df9d1014 100644 --- a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties +++ b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/config.properties @@ -14,6 +14,14 @@ EnablePKCE=Enable Proof Key for Code Exchange (PKCE) FullnameFieldName=Full name field name Group=Group GroupsFieldName=Groups field name +LoginLogoutQueryParametersTitle=Query Parameters for Login and Logout Endpoints +LoginQueryParameter.Title=Login query parameter +LoginLogoutQueryParamNameValuePairs.header=Query Parameter +LoginQueryParametersTitle=Query Parameters for Login Endpoint +LoginQueryParamNameValuePairs.add=Add Login Query Parameter +LogoutQueryParametersTitle=Query Parameters for Logout Endpoint +LogoutQueryParameter.Title=Logout query parameter +LogoutQueryParamNameValuePairs.add=Add Logout Query Parameter LogoutFromOpenIDProvider=Logout from OpenID Provider PostLogoutRedirectUrl=Post logout redirect URL Secret=Secret diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-loginQueryParameters.html b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-loginQueryParameters.html new file mode 100644 index 00000000..4623d064 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-loginQueryParameters.html @@ -0,0 +1,3 @@ +
+ You can add arbitrary HTTP query parameters to the login URL by adding them here. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-logoutQueryParameters.html b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-logoutQueryParameters.html new file mode 100644 index 00000000..fd2c9ecb --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/oic/OicSecurityRealm/help-logoutQueryParameters.html @@ -0,0 +1,3 @@ +
+ You can add arbitrary HTTP query parameters to the logout URL by adding them here. +
\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java b/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java index 3ac1617c..76dbfbbf 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java @@ -12,6 +12,7 @@ import io.jenkins.plugins.casc.model.CNode; import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; import jenkins.model.Jenkins; import org.jenkinsci.plugins.oic.OicSecurityRealm.TokenAuthMethod; import org.junit.jupiter.api.Test; @@ -28,6 +29,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertNotNull; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -82,6 +84,18 @@ void testConfig(JenkinsConfiguredWithCodeRule j) { assertTrue(oicSecurityRealm.isRootURLFromRequest()); assertEquals("http://localhost/jwks", serverConf.getJwksServerUrl()); assertFalse(oicSecurityRealm.isDisableTokenVerification()); + assertNotNull(oicSecurityRealm.getLoginQueryParameters()); + assertNotNull(oicSecurityRealm.getLogoutQueryParameters()); + assertEquals( + "loginkey1x\"xx@me=loginvalue1xxxx@you&?loginneu&/test==login?sunny%&/xx\"x", + oicSecurityRealm.getLoginQueryParameters().stream() + .map(config -> config.getKey() + "=" + config.getValue()) + .collect(Collectors.joining("&"))); + assertEquals( + "logoutkey1x\"xx@me=logoutvalue1xxxx@you&?logoutneu&/test==logout?sunny%&/xx\"x", + oicSecurityRealm.getLogoutQueryParameters().stream() + .map(config -> config.getKey() + "=" + config.getValue()) + .collect(Collectors.joining("&"))); } @Test @@ -136,6 +150,8 @@ void testMinimal(JenkinsConfiguredWithCodeRule j) { assertFalse(oicSecurityRealm.isRootURLFromRequest()); assertNull(serverConf.getJwksServerUrl()); assertFalse(oicSecurityRealm.isDisableTokenVerification()); + assertNull(oicSecurityRealm.getLoginQueryParameters()); + assertNull(oicSecurityRealm.getLogoutQueryParameters()); } @Test @@ -165,6 +181,9 @@ void testMinimalWellKnown(JenkinsConfiguredWithCodeRule j) { assertFalse(oicSecurityRealm.isDisableTokenVerification()); assertEquals(urlBase + "/well.known", serverConf.getWellKnownOpenIDConfigurationUrl()); + + assertNull(oicSecurityRealm.getLoginQueryParameters()); + assertNull(oicSecurityRealm.getLogoutQueryParameters()); } /** Class to setup WellKnownMockExtension for well known with stub and setting port in env variable diff --git a/src/test/java/org/jenkinsci/plugins/oic/LoginQueryParameterTest.java b/src/test/java/org/jenkinsci/plugins/oic/LoginQueryParameterTest.java new file mode 100644 index 00000000..16ffeec0 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/oic/LoginQueryParameterTest.java @@ -0,0 +1,63 @@ +package org.jenkinsci.plugins.oic; + +import hudson.model.Descriptor; +import hudson.model.Descriptor.FormException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + +@WithJenkins +public class LoginQueryParameterTest { + + @BeforeEach + public void setUp(JenkinsRule jenkinsRule) { + jenkinsRule.jenkins.getDescriptorList(LoginQueryParameter.class).add(new LoginQueryParameter.DescriptorImpl()); + } + + @Test + public void testInvalidKey() { + FormException e = + assertThrows(Descriptor.FormException.class, () -> new LoginQueryParameter("scope", "anything")); + assertThat(e.getMessage(), is("scope is a reserved word")); + assertThat(e.getFormField(), is("key")); + } + + @Test + public void testNullKeyOrValue() { + FormException e = assertThrows(Descriptor.FormException.class, () -> new LoginQueryParameter(null, null)); + assertThat(e.getMessage(), is("key must not be blank")); + assertThat(e.getFormField(), is("key")); + + e = assertThrows(Descriptor.FormException.class, () -> new LoginQueryParameter("test", null)); + assertThat(e.getMessage(), is("value must not be blank")); + assertThat(e.getFormField(), is("value")); + } + + @Test + public void testNoRestrictionsInValue() throws Exception { + new LoginQueryParameter("anything", "scope"); + } + + @Test + public void testValidKey() throws Exception { + LoginQueryParameter lqp = new LoginQueryParameter("myKey", "myValue"); + assertThat(lqp.getKey(), is("myKey")); + assertThat(lqp.getURLEncodedKey(), is("myKey")); + assertThat(lqp.getValue(), is("myValue")); + assertThat(lqp.getURLEncodedValue(), is("myValue")); + } + + @Test + public void testValidKeyWithEscaping() throws Exception { + LoginQueryParameter lqp = new LoginQueryParameter("my Key", "my/Value"); + assertThat(lqp.getKey(), is("my Key")); + assertThat(lqp.getURLEncodedKey(), is("my+Key")); + assertThat(lqp.getValue(), is("my/Value")); + assertThat(lqp.getURLEncodedValue(), is("my%2FValue")); + } +} diff --git a/src/test/java/org/jenkinsci/plugins/oic/LogoutQueryParameterTest.java b/src/test/java/org/jenkinsci/plugins/oic/LogoutQueryParameterTest.java new file mode 100644 index 00000000..a3bf01f5 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/oic/LogoutQueryParameterTest.java @@ -0,0 +1,74 @@ +package org.jenkinsci.plugins.oic; + +import hudson.model.Descriptor; +import hudson.model.Descriptor.FormException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + +@WithJenkins +public class LogoutQueryParameterTest { + + @BeforeEach + public void setUp(JenkinsRule jenkinsRule) { + jenkinsRule + .jenkins + .getDescriptorList(LogoutQueryParameter.class) + .add(new LogoutQueryParameter.DescriptorImpl()); + } + + @Test + public void testInvalidKey() throws Exception { + FormException e = assertThrows( + Descriptor.FormException.class, () -> new LogoutQueryParameter("id_token_hint", "anything")); + assertThat(e.getMessage(), is("id_token_hint is a reserved word")); + assertThat(e.getFormField(), is("key")); + } + + @Test + public void testNullKeyOrValue() throws Exception { + FormException e = assertThrows(Descriptor.FormException.class, () -> new LogoutQueryParameter(null, null)); + assertThat(e.getMessage(), is("key must not be blank")); + assertThat(e.getFormField(), is("key")); + + new LogoutQueryParameter("test", null); + } + + @Test + public void testNoRestrictionsInValue() throws Exception { + new LogoutQueryParameter("anything", "id_token_hint"); + new LogoutQueryParameter("anything", ""); + } + + @Test + public void testValidKey() throws Exception { + LogoutQueryParameter lqp = new LogoutQueryParameter("myKey", "myValue"); + assertThat(lqp.getKey(), is("myKey")); + assertThat(lqp.getURLEncodedKey(), is("myKey")); + assertThat(lqp.getValue(), is("myValue")); + assertThat(lqp.getURLEncodedValue(), is("myValue")); + } + + @Test + public void testEmptyValue() throws Exception { + LogoutQueryParameter lqp = new LogoutQueryParameter("something", ""); + assertThat(lqp.getKey(), is("something")); + assertThat(lqp.getURLEncodedKey(), is("something")); + assertThat(lqp.getValue(), is("")); + assertThat(lqp.getURLEncodedValue(), is("")); + } + + @Test + public void testValidKeyWithEscaping() throws Exception { + LogoutQueryParameter lqp = new LogoutQueryParameter("my Key", "my/Value"); + assertThat(lqp.getKey(), is("my Key")); + assertThat(lqp.getURLEncodedKey(), is("my+Key")); + assertThat(lqp.getValue(), is("my/Value")); + assertThat(lqp.getURLEncodedValue(), is("my%2FValue")); + } +} diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java index af892f61..3f331f63 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java @@ -4,9 +4,12 @@ import hudson.util.Secret; import java.util.Collection; import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.WithoutJenkins; import org.jvnet.hudson.test.junit.jupiter.WithJenkins; import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.AuthenticationManager; @@ -142,4 +145,107 @@ void testShouldCheckEscapeHatchWithHashedPassword(JenkinsRule jenkinsRule) throw assertFalse(realm.doCheckEscapeHatch("otherUsername", escapeHatchPassword)); assertFalse(realm.doCheckEscapeHatch(escapeHatchUsername, "wrongPassword")); } + + @Test + @WithoutJenkins + public void testMaybeOpenIdLogoutEndpoint() throws Exception { + TestRealm realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults() + .WithLogout(Boolean.FALSE, "https://endpoint") + .build(); + Assertions.assertNull(realm.maybeOpenIdLogoutEndpoint("my-id-token", null, "https://localhost")); + + realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults().WithLogout(Boolean.TRUE, null).build(); + Assertions.assertNull(realm.maybeOpenIdLogoutEndpoint("my-id-token", null, "https://localhost")); + + realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults().WithLogout(Boolean.FALSE, null).build(); + Assertions.assertNull(realm.maybeOpenIdLogoutEndpoint("my-id-token", null, "https://localhost")); + + realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults() + .WithLogout(Boolean.TRUE, "https://endpoint?query-param-1=test") + .build(); + assertEquals( + "https://endpoint?query-param-1=test&id_token_hint=my-id-token&post_logout_redirect_uri=https%3A%2F%2Flocalhost", + realm.maybeOpenIdLogoutEndpoint("my-id-token", null, "https://localhost")); + } + + @Test + @WithoutJenkins + public void testMaybeOpenIdLogoutEndpointWithNoCustomLogoutQueryParameters() throws Exception { + TestRealm realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults().WithLogout(true, "https://endpoint").build(); + assertEquals( + "https://endpoint?id_token_hint=my-id-token&post_logout_redirect_uri=https%3A%2F%2Flocalhost", + realm.maybeOpenIdLogoutEndpoint("my-id-token", "null", "https://localhost")); + assertEquals( + "https://endpoint?id_token_hint=my-id-token&post_logout_redirect_uri=https%3A%2F%2Flocalhost", + realm.maybeOpenIdLogoutEndpoint("my-id-token", null, "https://localhost")); + assertEquals( + "https://endpoint?id_token_hint=my-id-token&state=test&post_logout_redirect_uri=https%3A%2F%2Flocalhost", + realm.maybeOpenIdLogoutEndpoint("my-id-token", "test", "https://localhost")); + assertEquals("https://endpoint", realm.maybeOpenIdLogoutEndpoint(null, null, null)); + } + + @Test + public void testMaybeOpenIdLogoutEndpointWithCustomLogoutQueryParameters(JenkinsRule jenkinsRule) throws Exception { + jenkinsRule + .jenkins + .getDescriptorList(LogoutQueryParameter.class) + .add(new LogoutQueryParameter.DescriptorImpl()); + TestRealm realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults() + .WithLogoutQueryParameters(List.of( + new LogoutQueryParameter("key1", " with-spaces "), + new LogoutQueryParameter("param-only", ""))) + .WithLogout(true, "https://endpoint") + .build(); + String result = realm.maybeOpenIdLogoutEndpoint("my-id-token", "test", "https://localhost"); + assertNotNull(result); + assertFalse(result.contains("overwrite-test")); + assertEquals( + "https://endpoint?key1=with-spaces&id_token_hint=my-id-token&state=test&post_logout_redirect_uri=https%3A%2F%2Flocalhost¶m-only", + result); + } + + @Test + public void testMaybeOpenIdLogoutEndpointWithLogoutQueryParameters(JenkinsRule jenkinsRule) throws Exception { + jenkinsRule + .jenkins + .getDescriptorList(LogoutQueryParameter.class) + .add(new LogoutQueryParameter.DescriptorImpl()); + TestRealm realm = new TestRealm.Builder(wireMock) + .WithMinimalDefaults() + .WithLogoutQueryParameters(List.of( + new LogoutQueryParameter("a/test#", "1"), + new LogoutQueryParameter("b", ","), + new LogoutQueryParameter("b+", "$other:new"), + new LogoutQueryParameter("&ersand", " 2@+ , ?"), + new LogoutQueryParameter("d=", " 2 "), + new LogoutQueryParameter("iamnull", null), + new LogoutQueryParameter(" e? ", " "))) + .WithLogout(true, "https://endpoint") + .build(); + String result = realm.maybeOpenIdLogoutEndpoint("my-id-token", "test", "https://localhost"); + assertNotNull(result); + assertFalse(result.contains("overwrite-test")); + String queryParams = result.replace("https://endpoint?", ""); + assertEquals( + Stream.of( + "b=%2C", + "id_token_hint=my-id-token", + "a%2Ftest%23=1", + "state=test", + "post_logout_redirect_uri=https%3A%2F%2Flocalhost", + "d%3D=2", + "iamnull", + "b%2B=%24other%3Anew", + "%26ampersand=2%40%2B+%2C+%3F", + "e%3F") + .sorted() + .toList(), + Stream.of(queryParams.split("&")).sorted().toList()); + } } diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java index 5451dc55..89c2856f 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java @@ -1,19 +1,30 @@ package org.jenkinsci.plugins.oic; +import com.nimbusds.jose.JWSAlgorithm; +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; import hudson.Util; +import hudson.model.Descriptor; import hudson.util.FormValidation; +import java.net.URISyntaxException; +import jenkins.security.FIPS140; import org.hamcrest.Matcher; import org.jenkinsci.plugins.oic.OicServerManualConfiguration.DescriptorImpl; import org.junit.jupiter.api.Test; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.WithoutJenkins; import org.jvnet.hudson.test.junit.jupiter.WithJenkins; +import org.mockito.MockedStatic; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasProperty; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.jvnet.hudson.test.JenkinsMatchers.hasKind; +import static org.mockito.Mockito.mockStatic; @WithJenkins class OicServerManualConfigurationTest { @@ -88,6 +99,31 @@ void doCheckEndSessionEndpoint(JenkinsRule jenkinsRule) { assertThat(descriptor.doCheckEndSessionUrl("http://localhost.jwks"), hasKind(FormValidation.Kind.OK)); } + @Test + @WithoutJenkins + public void testProviderMetadataWithFips() throws Descriptor.FormException { + OicServerManualConfiguration config = new OicServerManualConfiguration("issuer", "t-url", "a-url"); + try (MockedStatic fips140Mock = mockStatic(FIPS140.class)) { + JWSAlgorithm.Family ed = JWSAlgorithm.Family.ED; + JWSAlgorithm arbitraryEdAlgorithm = (JWSAlgorithm) ed.toArray()[0]; + + fips140Mock.when(FIPS140::useCompliantAlgorithms).thenReturn(true); + OIDCProviderMetadata data = config.toProviderMetadata(); + assertFalse(data.getIDTokenJWSAlgs().contains(arbitraryEdAlgorithm)); + + fips140Mock.when(FIPS140::useCompliantAlgorithms).thenReturn(false); + data = config.toProviderMetadata(); + assertTrue(data.getIDTokenJWSAlgs().contains(arbitraryEdAlgorithm)); + } + } + + @Test + @WithoutJenkins + public void testProviderMetadataWithInvalidURI() throws Descriptor.FormException, URISyntaxException { + OicServerManualConfiguration config = new OicServerManualConfiguration("issuer", "t-url", "inv%alid"); + assertThrows(IllegalStateException.class, () -> config.toProviderMetadata()); + } + private static DescriptorImpl getDescriptor(JenkinsRule jenkinsRule) { return (DescriptorImpl) jenkinsRule.jenkins.getDescriptor(OicServerManualConfiguration.class); } diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index c529330a..187ba8af 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -385,6 +385,25 @@ void testLoginWithAutoConfiguration() throws Exception { assertTestUserIsMemberOfTestGroups(user); } + @Test + public void testLoginWithCustomLoginParameters() throws Exception { + mockAuthorizationRedirectsToFinishLogin(); + mockTokenReturnsIdTokenWithGroup(); + mockUserInfoWithTestGroups(); + configureWellKnown(null, null); + jenkins.setSecurityRealm(new TestRealm.Builder(wireMock) + .WithMinimalDefaults() + .WithAutomanualconfigure(true) + .WithLoginQueryParameters( + List.of(new LoginQueryParameter("queryLoginParamName", "queryLoginParamValue"))) + .build()); + assertAnonymous(); + browseLoginPage(); + var user = assertTestUser(); + assertTestUserEmail(user); + assertTestUserIsMemberOfTestGroups(user); + } + @Test void testLoginWithAutoConfiguration_WithNoScope() throws Exception { mockAuthorizationRedirectsToFinishLogin(); @@ -1016,7 +1035,25 @@ void testLogoutShouldBeProviderURLWhenProviderLogoutConfigured() throws Exceptio logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest2(), Jenkins.ANONYMOUS2); return null; }); - assertEquals("http://provider/logout?state=null", logoutURL[0]); + assertEquals("http://provider/logout", logoutURL[0]); + } + + @Test + public void testLogoutShouldBeProviderURLWhenProviderLogoutConfiguredWithAdditionalLogoutQueryParameters() + throws Exception { + final TestRealm oicsr = new TestRealm.Builder(wireMock) + .WithLogoutQueryParameters(List.of( + new LogoutQueryParameter("hello", "world"), new LogoutQueryParameter("single", ""))) + .WithLogout(Boolean.TRUE, "http://provider/logout") + .build(); + jenkins.setSecurityRealm(oicsr); + + String[] logoutURL = new String[1]; + jenkinsRule.executeOnServer(() -> { + logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest2(), Jenkins.ANONYMOUS2); + return null; + }); + assertEquals("http://provider/logout?hello=world&single", logoutURL[0]); } @Test @@ -1034,7 +1071,7 @@ void testLogoutShouldBeProviderURLWithRedirectWhenProviderLogoutConfiguredWithPo return null; }); assertEquals( - "http://provider/logout?state=null&post_logout_redirect_uri=http%3A%2F%2Fsee.it%2F%3Fcat%26color%3Dwhite", + "http://provider/logout?post_logout_redirect_uri=http%3A%2F%2Fsee.it%2F%3Fcat%26color%3Dwhite", logoutURL[0]); } diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java index ffa02ec1..f86958d9 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java +++ b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java @@ -9,6 +9,7 @@ import java.io.ObjectStreamException; import java.io.Serial; import java.text.ParseException; +import java.util.List; import jenkins.model.IdStrategy; import org.kohsuke.stapler.StaplerRequest2; import org.kohsuke.stapler.StaplerResponse2; @@ -44,6 +45,8 @@ public static class Builder { public String fullNameFieldName = FULL_NAME_FIELD; public String emailFieldName = null; public String scopes = null; + public List loginQueryParameters = null; + public List logoutQueryParameters = null; public String groupsFieldName = null; public boolean disableSslVerification = false; public Boolean logoutFromOpenidProvider = false; @@ -121,6 +124,16 @@ public Builder WithScopes(String scopes) { return this; } + public Builder WithLoginQueryParameters(List values) { + this.loginQueryParameters = values; + return this; + } + + public Builder WithLogoutQueryParameters(List values) { + this.logoutQueryParameters = values; + return this; + } + public Builder WithMinimalDefaults() { return this.WithEmailFieldName(EMAIL_FIELD).WithGroupsFieldName(GROUPS_FIELD); } @@ -214,6 +227,8 @@ public TestRealm(Builder builder) throws Exception { this.setEscapeHatchSecret(builder.escapeHatchSecret); this.setEscapeHatchGroup(builder.escapeHatchGroup); this.setDisableTokenVerification(builder.disableTokenValidation); + this.setLoginQueryParameters(builder.loginQueryParameters); + this.setLogoutQueryParameters(builder.logoutQueryParameters); // need to call the following method annotated with @PostConstruct and called // from readResolve and as such // is only called in regular use not code use. diff --git a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml index fbd70922..50cc705f 100644 --- a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml +++ b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCode.yml @@ -25,3 +25,13 @@ jenkins: sendScopesInTokenRequest: true pkceEnabled: true nonceDisabled: true + loginQueryParameters: + - key: "loginkey1x\"xx@me" + value: "loginvalue1xxxx@you" + - key: "?loginneu&/test=" + value: "login?sunny%&/xx\"x" + logoutQueryParameters: + - key: "logoutkey1x\"xx@me" + value: "logoutvalue1xxxx@you" + - key: "?logoutneu&/test=" + value: "logout?sunny%&/xx\"x" diff --git a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml index feb52fe0..1f765411 100644 --- a/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml +++ b/src/test/resources/org/jenkinsci/plugins/oic/ConfigurationAsCodeExport.yml @@ -7,6 +7,16 @@ escapeHatchUsername: "escapeHatchUsername" fullNameFieldName: "fullNameFieldName" groupIdStrategy: "caseInsensitive" groupsFieldName: "groupsFieldName" +loginQueryParameters: +- key: "loginkey1x\"xx@me" + value: "loginvalue1xxxx@you" +- key: "?loginneu&/test=" + value: "login?sunny%&/xx\"x" +logoutQueryParameters: +- key: "logoutkey1x\"xx@me" + value: "logoutvalue1xxxx@you" +- key: "?logoutneu&/test=" + value: "logout?sunny%&/xx\"x" nonceDisabled: true pkceEnabled: true rootURLFromRequest: true