diff --git a/docs/configuration/README.md b/docs/configuration/README.md index 53c2a839..891dcf93 100644 --- a/docs/configuration/README.md +++ b/docs/configuration/README.md @@ -21,7 +21,7 @@ which will also help discovering your settings From 1.5 and onward the well known configuration location may be used to populate the configuration simplifying the configuration greatly. -The switch between modes is controled by the `serverConfiguration` field +The switch between modes is controlled by the `serverConfiguration` field | field | format | description | |----------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------| @@ -154,7 +154,7 @@ jenkins: tokenExpirationCheckDisabled: # escape hatch escapeHatchEnabled: - escapeHatchUsername: escapeHatchUsername + escapeHatchUsername: escapeHatchSecret: escapeHatchGroup: ``` diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 847275ef..c0f3dac2 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -183,23 +183,23 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { @Deprecated private transient String wellKnownOpenIDConfigurationUrl; - /** @deprecated see {@link OicServerConfiguration#getTokenServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getTokenServerUrl()} */ @Deprecated private transient String tokenServerUrl; - /** @deprecated see {@link OicServerConfiguration#getJwksServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getJwksServerUrl()} */ @Deprecated private transient String jwksServerUrl; - /** @deprecated see {@link OicServerConfiguration#getTokenAuthMethod()} */ + /** @deprecated see {@link OicServerManualConfiguration#getTokenAuthMethod()} */ @Deprecated private transient TokenAuthMethod tokenAuthMethod; - /** @deprecated see {@link OicServerConfiguration#getAuthorizationServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getAuthorizationServerUrl()} */ @Deprecated private transient String authorizationServerUrl; - /** @deprecated see {@link OicServerConfiguration#getUserInfoServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getUserInfoServerUrl()} */ @Deprecated private transient String userInfoServerUrl; @@ -218,14 +218,14 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { private transient String simpleGroupsFieldName = null; private transient String nestedGroupFieldName = null; - /** @deprecated see {@link OicServerConfiguration#getScopes()} */ + /** @deprecated see {@link OicServerManualConfiguration#getScopes()} */ @Deprecated private transient String scopes = null; private final boolean disableSslVerification; private boolean logoutFromOpenidProvider = true; - /** @deprecated see {@link OicServerConfiguration#getEndSessionUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getEndSessionUrl()} */ @Deprecated private transient String endSessionEndpoint = null; @@ -294,7 +294,7 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { SystemProperties.getBoolean(OicSecurityRealm.class.getName() + ".checkNonceInRefreshFlow", false); /** old field that had an '/' implicitly added at the end, - * transient because we no longer want to have this value stored + * transient because we no longer want to have this value stored, * but it's still needed for backwards compatibility */ @Deprecated private transient String endSessionUrl; @@ -627,12 +627,6 @@ private void filterEncryptionMethods(@NonNull OIDCProviderMetadata oidcProviderM oidcProviderMetadata.setUserInfoJWEEncs(userInfoJWEEncs); } - if (oidcProviderMetadata.getRequestObjectJWEEncs() != null) { - List requestObjectJweEncs = OicAlgorithmValidatorFIPS140.getFipsCompliantEncryptionMethod( - oidcProviderMetadata.getRequestObjectJWEEncs()); - oidcProviderMetadata.setRequestObjectJWEEncs(requestObjectJweEncs); - } - if (oidcProviderMetadata.getAuthorizationJWEEncs() != null) { List authJweEncs = OicAlgorithmValidatorFIPS140.getFipsCompliantEncryptionMethod( oidcProviderMetadata.getAuthorizationJWEEncs()); @@ -723,9 +717,9 @@ private void filterJwsAlgorithms(@NonNull OIDCProviderMetadata oidcProviderMetad } if (oidcProviderMetadata.getClientRegistrationAuthnJWSAlgs() != null) { - List clientRegisterationAuth = OicAlgorithmValidatorFIPS140.getFipsCompliantJWSAlgorithm( + List clientRegistrationAuth = OicAlgorithmValidatorFIPS140.getFipsCompliantJWSAlgorithm( oidcProviderMetadata.getClientRegistrationAuthnJWSAlgs()); - oidcProviderMetadata.setClientRegistrationAuthnJWSAlgs(clientRegisterationAuth); + oidcProviderMetadata.setClientRegistrationAuthnJWSAlgs(clientRegistrationAuth); } } @@ -791,14 +785,10 @@ protected static Expression compileJMESPath(String str, String logCommen return null; } - private Object applyJMESPath(Expression expression, Object map) { - return expression.search(map); - } - @DataBoundSetter public void setGroupsFieldName(String groupsFieldName) { this.groupsFieldName = Util.fixEmptyAndTrim(groupsFieldName); - this.groupsFieldExpr = this.compileJMESPath(this.groupsFieldName, "groups field"); + this.groupsFieldExpr = compileJMESPath(this.groupsFieldName, "groups field"); } @DataBoundSetter @@ -962,7 +952,7 @@ public Authentication authenticate(Authentication authentication) throws Authent * Validate post-login redirect URL * * For security reasons, the login must not redirect outside Jenkins - * realm. For useablility reason, the logout page should redirect to + * realm. For usability reason, the logout page should redirect to * root url. */ protected String getValidRedirectUrl(String url) { @@ -987,7 +977,7 @@ protected String getValidRedirectUrl(String url) { } /** - * Handles the the securityRealm/commenceLogin resource and sends the user off to the IdP + * Handles the securityRealm/commenceLogin resource and sends the user off to the IdP * @param from the relative URL to the page that the user has just come from * @param referer the HTTP referer header (where to redirect the user back to after login has finished) * @throws URISyntaxException if the provided data is invalid @@ -1011,7 +1001,6 @@ public void doCommenceLogin(@QueryParameter String from, @Header("Referer") fina // store the redirect url for after the login. sessionStore.set(webContext, SESSION_POST_LOGIN_REDIRECT_URL_KEY, redirectOnFinish); JEEHttpActionAdapter.INSTANCE.adapt(redirectionAction, webContext); - return; } @SuppressFBWarnings( @@ -1115,11 +1104,8 @@ private String determineStringField(Expression fieldExpr, JWT idToken, M } } if (idToken != null) { - String fieldValue = Util.fixEmptyAndTrim( + return Util.fixEmptyAndTrim( getStringField(idToken.getJWTClaimsSet().getClaims(), fieldExpr)); - if (fieldValue != null) { - return fieldValue; - } } } return null; @@ -1141,7 +1127,7 @@ private List determineAuthorities(JWT idToken, MapemptyList(); } else if (field instanceof String) { - // if its a String, the original value was not a json array. + // if it's a String, the original value was not a json array. // We try to convert the string to list based on comma while ignoring whitespaces and square brackets. // Example value "[demo-user-group, demo-test-group, demo-admin-group]" String sField = (String) field; @@ -1203,9 +1189,9 @@ private List ensureString(Object field) { if (group instanceof String) { result.add(group.toString()); } else if (group instanceof Map) { - // if its a Map, we use the nestedGroupFieldName to grab the groups + // if it's a Map, we use the nestedGroupFieldName to grab the groups Map groupMap = (Map) group; - if (nestedGroupFieldName != null && groupMap.keySet().contains(nestedGroupFieldName)) { + if (nestedGroupFieldName != null && groupMap.containsKey(nestedGroupFieldName)) { result.add(groupMap.get(nestedGroupFieldName)); } } @@ -1390,7 +1376,6 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th } catch (HttpAction e) { // this may be an OK flow for logout login is handled upstream. JEEHttpActionAdapter.INSTANCE.adapt(e, webContext); - return; } } @@ -1526,8 +1511,8 @@ private boolean refreshExpiredToken( User u = User.get2(a); LOGGER.log( Level.FINE, - "Token refresh. Current Authentitcation principal: " + a.getName() + " user id:" - + (u == null ? "null user" : u.getId()) + " newly retreived username would have been: " + "Token refresh. Current Authentication principal: " + a.getName() + " user id:" + + (u == null ? "null user" : u.getId()) + " newly retrieved username would have been: " + username); } username = expectedUsername; diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicServerManualConfiguration.java b/src/main/java/org/jenkinsci/plugins/oic/OicServerManualConfiguration.java index c11577f7..9849bd3e 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicServerManualConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicServerManualConfiguration.java @@ -153,17 +153,16 @@ public OIDCProviderMetadata toProviderMetadata() { // should really be a UI option, but was not previously // server is mandated to support HS256 but if we do not specify things that it produces // then they would never be checked. - // rather we just say "I support anything, and let the check for the specific ones fail and fall through - ArrayList allAlgorthms = new ArrayList<>(); - allAlgorthms.addAll(JWSAlgorithm.Family.HMAC_SHA); + // rather we just say "I support anything, and let the check for the specific ones fail and fall through" + ArrayList allAlgorithms = new ArrayList<>(JWSAlgorithm.Family.HMAC_SHA); if (FIPS140.useCompliantAlgorithms()) { // In FIPS-140 Family.ED is not supported - allAlgorthms.addAll(JWSAlgorithm.Family.RSA); - allAlgorthms.addAll(JWSAlgorithm.Family.EC); + allAlgorithms.addAll(JWSAlgorithm.Family.RSA); + allAlgorithms.addAll(JWSAlgorithm.Family.EC); } else { - allAlgorthms.addAll(JWSAlgorithm.Family.SIGNATURE); + allAlgorithms.addAll(JWSAlgorithm.Family.SIGNATURE); } - providerMetadata.setIDTokenJWSAlgs(allAlgorthms); + providerMetadata.setIDTokenJWSAlgs(allAlgorithms); return providerMetadata; } catch (URISyntaxException e) { throw new IllegalStateException("could not create provider metadata", e); diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicServerManualConfigurationTest.java index 5451dc55..2561d085 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); }