diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index cc2be653..195bf28d 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -372,43 +372,49 @@ public OicSecurityRealm( } @SuppressWarnings("deprecated") - protected Object readResolve() throws IOException, FormException { + protected Object readResolve() throws ObjectStreamException { if (properties == null) { properties = new DescribableList<>(Saveable.NOOP); } // Fail if migrating to a FIPS non-compliant config - if (FIPS140.useCompliantAlgorithms()) { - if (isDisableSslVerification()) { - throw new IllegalStateException(Messages.OicSecurityRealm_DisableSslVerificationFipsMode()); + if (FIPS140.useCompliantAlgorithms() && isDisableSslVerification()) { + throw new IllegalStateException(Messages.OicSecurityRealm_DisableSslVerificationFipsMode()); + } + try { + if (nonceDisabled) { + properties.replace(new DisableNonce()); } - if (isDisableTokenVerification() || properties.get(DisableTokenVerification.class) != null) { - throw new IllegalStateException(Messages.OicSecurityRealm_DisableTokenVerificationFipsMode()); + if (pkceEnabled) { + properties.replace(new Pkce()); } - if (isEscapeHatchEnabled() || properties.get(EscapeHatch.class) != null) { - throw new IllegalStateException(Messages.OicSecurityRealm_EscapeHatchFipsMode()); + if (disableTokenVerification) { + properties.replace(new DisableTokenVerification()); } + if (allowedTokenExpirationClockSkewSeconds != null) { + var value = allowedTokenExpirationClockSkewSeconds.intValue(); + if (value != 60) { + properties.replace(new AllowedTokenExpirationClockSkew(value)); + } + } + if (loginQueryParameters != null) { + properties.replace(new LoginQueryParameters(loginQueryParameters)); + } + if (logoutQueryParameters != null) { + properties.replace(new LogoutQueryParameters(logoutQueryParameters)); + } + if (escapeHatchEnabled) { + properties.replace(new EscapeHatch(escapeHatchUsername, escapeHatchGroup, escapeHatchSecret)); + } + } catch (IOException e) { + var ose = new InvalidObjectException("Error while migrating properties"); + ose.initCause(e); + throw ose; + } catch (FormException e) { + var ose = new InvalidObjectException(e.getFormField() + ": " + e.getMessage()); + ose.initCause(e); + throw ose; } - if (nonceDisabled) { - properties.replace(new DisableNonce()); - } - if (pkceEnabled) { - properties.replace(new Pkce()); - } - if (disableTokenVerification) { - properties.replace(new DisableTokenVerification()); - } - if (allowedTokenExpirationClockSkewSeconds != null && allowedTokenExpirationClockSkewSeconds != 60L) { - properties.replace(new AllowedTokenExpirationClockSkew(allowedTokenExpirationClockSkewSeconds.intValue())); - } - if (loginQueryParameters != null) { - properties.replace(new LoginQueryParameters(loginQueryParameters)); - } - if (logoutQueryParameters != null) { - properties.replace(new LogoutQueryParameters(logoutQueryParameters)); - } - if (escapeHatchEnabled) { - properties.replace(new EscapeHatch(escapeHatchUsername, escapeHatchGroup, escapeHatchSecret)); - } + if (!Strings.isNullOrEmpty(endSessionUrl)) { this.endSessionEndpoint = endSessionUrl + "/"; } @@ -463,26 +469,6 @@ protected Object readResolve() throws IOException, FormException { return this; } - @Deprecated - public void setLoginQueryParameters(List values) { - this.loginQueryParameters = values; - } - - @Deprecated - public List getLoginQueryParameters() { - return loginQueryParameters; - } - - @Deprecated - public void setLogoutQueryParameters(List values) { - this.logoutQueryParameters = values; - } - - @Deprecated - public List getLogoutQueryParameters() { - return logoutQueryParameters; - } - public String getClientId() { return clientId; } @@ -555,26 +541,6 @@ public String getPostLogoutRedirectUrl() { return postLogoutRedirectUrl; } - @Deprecated - public boolean isEscapeHatchEnabled() { - return escapeHatchEnabled; - } - - @Deprecated - public String getEscapeHatchUsername() { - return escapeHatchUsername; - } - - @Deprecated - public Secret getEscapeHatchSecret() { - return escapeHatchSecret; - } - - @Deprecated - public String getEscapeHatchGroup() { - return escapeHatchGroup; - } - public boolean isRootURLFromRequest() { return rootURLFromRequest; } @@ -583,18 +549,6 @@ public boolean isSendScopesInTokenRequest() { return sendScopesInTokenRequest; } - public boolean isPkceEnabled() { - return pkceEnabled; - } - - public boolean isDisableTokenVerification() { - return disableTokenVerification; - } - - public boolean isNonceDisabled() { - return nonceDisabled; - } - public boolean isTokenExpirationCheckDisabled() { return tokenExpirationCheckDisabled; } @@ -603,15 +557,6 @@ public boolean isAllowTokenAccessWithoutOicSession() { return allowTokenAccessWithoutOicSession; } - /** - * @deprecated use {@link AllowedTokenExpirationClockSkew} instead. - * @return - */ - @Deprecated - public Long getAllowedTokenExpirationClockSkewSeconds() { - return allowedTokenExpirationClockSkewSeconds; - } - public DescribableList getProperties() { return properties; } @@ -737,19 +682,6 @@ public void setPostLogoutRedirectUrl(String postLogoutRedirectUrl) { this.postLogoutRedirectUrl = Util.fixEmptyAndTrim(postLogoutRedirectUrl); } - @Deprecated - public void setEscapeHatchEnabled(boolean escapeHatchEnabled) throws FormException { - if (FIPS140.useCompliantAlgorithms() && escapeHatchEnabled) { - throw new FormException("Escape Hatch cannot be enabled in FIPS environment", "escapeHatchEnabled"); - } - this.escapeHatchEnabled = escapeHatchEnabled; - } - - @Deprecated - public void setEscapeHatchUsername(String escapeHatchUsername) { - this.escapeHatchUsername = Util.fixEmptyAndTrim(escapeHatchUsername); - } - @DataBoundSetter public void setEscapeHatchSecret(Secret escapeHatchSecret) { if (escapeHatchSecret != null) { @@ -765,18 +697,6 @@ public void setEscapeHatchSecret(Secret escapeHatchSecret) { this.escapeHatchSecret = escapeHatchSecret; } - @Deprecated - protected boolean checkEscapeHatch(String username, String password) { - final boolean isUsernameMatch = username.equals(this.escapeHatchUsername); - final boolean isPasswordMatch = BCrypt.checkpw(password, Secret.toString(this.escapeHatchSecret)); - return isUsernameMatch & isPasswordMatch; - } - - @Deprecated - public void setEscapeHatchGroup(String escapeHatchGroup) { - this.escapeHatchGroup = Util.fixEmptyAndTrim(escapeHatchGroup); - } - @DataBoundSetter public void setRootURLFromRequest(boolean rootURLFromRequest) { this.rootURLFromRequest = rootURLFromRequest; @@ -787,26 +707,6 @@ public void setSendScopesInTokenRequest(boolean sendScopesInTokenRequest) { this.sendScopesInTokenRequest = sendScopesInTokenRequest; } - @Deprecated - public void setPkceEnabled(boolean pkceEnabled) { - this.pkceEnabled = pkceEnabled; - } - - @DataBoundSetter - @Deprecated - public void setDisableTokenVerification(boolean disableTokenVerification) throws FormException { - if (FIPS140.useCompliantAlgorithms() && disableTokenVerification) { - throw new FormException( - Messages.OicSecurityRealm_DisableTokenVerificationFipsMode(), "disableTokenVerification"); - } - this.disableTokenVerification = disableTokenVerification; - } - - @Deprecated - public void setNonceDisabled(boolean nonceDisabled) { - this.nonceDisabled = nonceDisabled; - } - @DataBoundSetter public void setTokenExpirationCheckDisabled(boolean tokenExpirationCheckDisabled) { this.tokenExpirationCheckDisabled = tokenExpirationCheckDisabled; @@ -817,14 +717,6 @@ public void setAllowTokenAccessWithoutOicSession(boolean allowTokenAccessWithout this.allowTokenAccessWithoutOicSession = allowTokenAccessWithoutOicSession; } - /** - * @deprecated use {@link AllowedTokenExpirationClockSkew} instead. - */ - @Deprecated - public void setAllowedTokenExpirationClockSkewSeconds(Long allowedTokenExpirationClockSkewSeconds) { - this.allowedTokenExpirationClockSkewSeconds = allowedTokenExpirationClockSkewSeconds; - } - @Override public String getLoginUrl() { // Login begins with our doCommenceLogin(String,String) method @@ -1279,7 +1171,7 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th refreshToken != null ? refreshToken.getValue() : null, accessToken == null ? 0 : accessToken.getLifetime(), CLOCK.millis(), - getAllowedTokenExpirationClockSkewSeconds()); + (long) client.getConfiguration().getMaxClockSkew()); loginAndSetUserData(username, idToken, profile.getAttributes(), oicCredentials); @@ -1456,7 +1348,7 @@ private boolean refreshExpiredToken( refreshToken.getValue(), accessToken.getLifetime(), CLOCK.millis(), - getAllowedTokenExpirationClockSkewSeconds()); + (long) client.getConfiguration().getMaxClockSkew()); loginAndSetUserData(username, idToken, profile.getAttributes(), refreshedCredentials); return true; diff --git a/src/main/java/org/jenkinsci/plugins/oic/properties/DisableTokenVerification.java b/src/main/java/org/jenkinsci/plugins/oic/properties/DisableTokenVerification.java index 44f3b085..b99906a3 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/properties/DisableTokenVerification.java +++ b/src/main/java/org/jenkinsci/plugins/oic/properties/DisableTokenVerification.java @@ -2,6 +2,7 @@ import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; +import java.io.Serial; import jenkins.security.FIPS140; import org.apache.commons.lang.Validate; import org.jenkinsci.plugins.oic.AnythingGoesTokenValidator; @@ -30,6 +31,15 @@ public OidcPropertyExecution newExecution(@NonNull OicServerConfiguration server return new ExecutionImpl(serverConfiguration); } + @Serial + protected Object readResolve() { + if (FIPS140.useCompliantAlgorithms()) { + throw new IllegalStateException( + org.jenkinsci.plugins.oic.Messages.OicSecurityRealm_DisableTokenVerificationFipsMode()); + } + return this; + } + private record ExecutionImpl(OicServerConfiguration serverConfiguration) implements OidcPropertyExecution { @Override public void customizeConfiguration(@NonNull OidcConfiguration configuration) { @@ -54,6 +64,7 @@ public void customizeClient(@NonNull OidcClient client) { @Extension public static class DescriptorImpl extends OidcPropertyDescriptor { + @Override public boolean isApplicable() { return !FIPS140.useCompliantAlgorithms(); diff --git a/src/main/java/org/jenkinsci/plugins/oic/properties/EscapeHatch.java b/src/main/java/org/jenkinsci/plugins/oic/properties/EscapeHatch.java index d7cef9be..eabcf75b 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/properties/EscapeHatch.java +++ b/src/main/java/org/jenkinsci/plugins/oic/properties/EscapeHatch.java @@ -15,6 +15,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; +import java.io.Serial; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -22,6 +23,7 @@ import java.util.regex.Pattern; import jenkins.security.FIPS140; import jenkins.security.SecurityListener; +import org.jenkinsci.plugins.oic.Messages; import org.jenkinsci.plugins.oic.OicUserDetails; import org.jenkinsci.plugins.oic.OidcProperty; import org.jenkinsci.plugins.oic.OidcPropertyDescriptor; @@ -41,7 +43,7 @@ public class EscapeHatch extends OidcProperty { public static final Pattern B_CRYPT_PATTERN = Pattern.compile("\\A\\$[^$]+\\$\\d+\\$[./0-9A-Za-z]{53}"); - @CheckForNull + @NonNull private final String username; @CheckForNull @@ -54,7 +56,7 @@ public class EscapeHatch extends OidcProperty { public EscapeHatch(@NonNull String username, @CheckForNull String group, @NonNull Secret secret) throws Descriptor.FormException { if (FIPS140.useCompliantAlgorithms()) { - throw new IllegalStateException("Cannot use Escape Hatch in FIPS-140 mode"); + throw new Descriptor.FormException("Cannot use Escape Hatch in FIPS-140 mode", "escapeHatch"); } var sanitizedUsername = Util.fixEmptyAndTrim(username); if (sanitizedUsername == null) { @@ -104,8 +106,9 @@ private void randomWait() { public Optional authenticate(@NonNull Authentication authentication) { if (authentication instanceof UsernamePasswordAuthenticationToken) { randomWait(); // to slowdown brute forcing - if (authentication.getPrincipal().toString().equals(this.username) - && BCrypt.checkpw(authentication.getCredentials().toString(), Secret.toString(this.secret))) { + if (check( + authentication.getPrincipal().toString(), + authentication.getCredentials().toString())) { List grantedAuthorities = new ArrayList<>(); grantedAuthorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2); if (isNotBlank(group)) { @@ -124,11 +127,21 @@ public Optional authenticate(@NonNull Authentication authenticat return Optional.empty(); } - @Extension + /** + * Check a given username and password against the configured ones. + */ + public boolean check(@NonNull String username, @CheckForNull String password) { + return username.equals(this.username) && BCrypt.checkpw(password, Secret.toString(this.secret)); + } + public static class DescriptorImpl extends OidcPropertyDescriptor { - @Override - public boolean isApplicable() { - return !FIPS140.useCompliantAlgorithms(); + @Extension + @CheckForNull + public static DescriptorImpl createIfApplicable() { + if (FIPS140.useCompliantAlgorithms()) { + return null; + } + return new DescriptorImpl(); } @NonNull @@ -138,6 +151,14 @@ public String getDisplayName() { } } + @Serial + protected Object readResolve() { + if (FIPS140.useCompliantAlgorithms()) { + throw new IllegalStateException(Messages.OicSecurityRealm_EscapeHatchFipsMode()); + } + return this; + } + /** * Excluding the escapeHatch login from CSRF protection as the crumb is calculated based on the authentication * mirroring behavior of the normal login page. diff --git a/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java b/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java index 9e6b058a..e231cfc2 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/ConfigurationAsCodeTest.java @@ -8,6 +8,7 @@ import static io.jenkins.plugins.casc.misc.Util.toStringFromYamlFile; import static io.jenkins.plugins.casc.misc.Util.toYamlString; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -87,7 +88,6 @@ void testConfig(JenkinsConfiguredWithCodeRule j) { assertEquals("userNameField", oicSecurityRealm.getUserNameField()); assertTrue(oicSecurityRealm.isRootURLFromRequest()); assertEquals("http://localhost/jwks", serverConf.getJwksServerUrl()); - assertFalse(oicSecurityRealm.isDisableTokenVerification()); var loginQueryParameters = oicSecurityRealm.getProperties().get(LoginQueryParameters.class); assertThat(loginQueryParameters, notNullValue()); assertEquals( @@ -145,7 +145,6 @@ void testMinimal(JenkinsConfiguredWithCodeRule j) { assertEquals("clientSecret", Secret.toString(oicSecurityRealm.getClientSecret())); assertFalse(oicSecurityRealm.isDisableSslVerification()); assertNull(oicSecurityRealm.getEmailFieldName()); - assertFalse(oicSecurityRealm.isEscapeHatchEnabled()); assertNull(oicSecurityRealm.getFullNameFieldName()); assertNull(oicSecurityRealm.getGroupsFieldName()); assertEquals("openid email", serverConf.getScopes()); @@ -155,9 +154,7 @@ void testMinimal(JenkinsConfiguredWithCodeRule j) { assertTrue(oicSecurityRealm.isLogoutFromOpenidProvider()); assertFalse(oicSecurityRealm.isRootURLFromRequest()); assertNull(serverConf.getJwksServerUrl()); - assertFalse(oicSecurityRealm.isDisableTokenVerification()); - assertNull(oicSecurityRealm.getLoginQueryParameters()); - assertNull(oicSecurityRealm.getLogoutQueryParameters()); + assertThat(oicSecurityRealm.getProperties(), empty()); } @Test @@ -175,7 +172,6 @@ void testMinimalWellKnown(JenkinsConfiguredWithCodeRule j) { assertFalse(oicSecurityRealm.isDisableSslVerification()); assertNull(oicSecurityRealm.getEmailFieldName()); - assertFalse(oicSecurityRealm.isEscapeHatchEnabled()); assertNull(oicSecurityRealm.getFullNameFieldName()); assertNull(oicSecurityRealm.getGroupsFieldName()); @@ -184,12 +180,9 @@ void testMinimalWellKnown(JenkinsConfiguredWithCodeRule j) { assertEquals("sub", oicSecurityRealm.getUserNameField()); assertTrue(oicSecurityRealm.isLogoutFromOpenidProvider()); - assertFalse(oicSecurityRealm.isDisableTokenVerification()); assertEquals(urlBase + "/well.known", serverConf.getWellKnownOpenIDConfigurationUrl()); - - assertNull(oicSecurityRealm.getLoginQueryParameters()); - assertNull(oicSecurityRealm.getLogoutQueryParameters()); + assertThat(oicSecurityRealm.getProperties(), empty()); } /** 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/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java index 53ecf3fe..a9e1eafc 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java @@ -7,10 +7,8 @@ import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import hudson.util.Secret; @@ -32,7 +30,6 @@ import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; -import org.springframework.security.crypto.bcrypt.BCrypt; @WithJenkins class OicSecurityRealmTest { @@ -118,41 +115,6 @@ void testShouldReturnRootUrlWhenRedirectUrlIsInvalid(JenkinsRule jenkinsRule) th assertEquals(rootUrl, realm.getValidRedirectUrl("http://localhost/jenkins/../bar/")); } - @Test - void testShouldCheckEscapeHatchWithPlainPassword(JenkinsRule jenkinsRule) throws Exception { - final String escapeHatchUsername = "aUsername"; - final String escapeHatchPassword = "aSecretPassword"; - - TestRealm realm = new TestRealm.Builder(wireMock) - .WithMinimalDefaults() - .WithEscapeHatch(true, escapeHatchUsername, escapeHatchPassword, "Group") - .build(); - - assertEquals(escapeHatchUsername, realm.getEscapeHatchUsername()); - assertNotEquals(escapeHatchPassword, Secret.toString(realm.getEscapeHatchSecret())); - assertTrue(realm.doCheckEscapeHatch(escapeHatchUsername, escapeHatchPassword)); - assertFalse(realm.doCheckEscapeHatch("otherUsername", escapeHatchPassword)); - assertFalse(realm.doCheckEscapeHatch(escapeHatchUsername, "wrongPassword")); - } - - @Test - void testShouldCheckEscapeHatchWithHashedPassword(JenkinsRule jenkinsRule) throws Exception { - final String escapeHatchUsername = "aUsername"; - final String escapeHatchPassword = "aSecretPassword"; - final String escapeHatchCryptedPassword = BCrypt.hashpw(escapeHatchPassword, BCrypt.gensalt()); - - TestRealm realm = new TestRealm.Builder(wireMock) - .WithMinimalDefaults() - .WithEscapeHatch(true, escapeHatchUsername, escapeHatchCryptedPassword, "Group") - .build(); - - assertEquals(escapeHatchUsername, realm.getEscapeHatchUsername()); - assertEquals(escapeHatchCryptedPassword, Secret.toString(realm.getEscapeHatchSecret())); - assertTrue(realm.doCheckEscapeHatch(escapeHatchUsername, escapeHatchPassword)); - assertFalse(realm.doCheckEscapeHatch("otherUsername", escapeHatchPassword)); - assertFalse(realm.doCheckEscapeHatch(escapeHatchUsername, "wrongPassword")); - } - @Test @WithoutJenkins public void testMaybeOpenIdLogoutEndpoint() throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java index d8518005..dc8fea6a 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/SecurityRealmConfigurationFIPSTest.java @@ -1,11 +1,13 @@ package org.jenkinsci.plugins.oic; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertThrows; import hudson.model.Descriptor; +import hudson.util.Secret; import jenkins.security.FIPS140; +import org.jenkinsci.plugins.oic.properties.EscapeHatch; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -31,22 +33,22 @@ static void tearDown() { @Test void escapeHatchThrowsException() { assertThrows( - Descriptor.FormException.class, - () -> new OicSecurityRealm("clientId", null, null, null, null, null).setEscapeHatchEnabled(true)); + Descriptor.FormException.class, () -> new OicSecurityRealm("clientId", null, null, null, null, null) + .getProperties() + .add(new EscapeHatch("admin", null, Secret.fromString("very-secret")))); } @Test void escapeHatchToFalse() throws Exception { OicSecurityRealm oicSecurityRealm = new OicSecurityRealm("clientId", null, null, null, null, null); - oicSecurityRealm.setEscapeHatchEnabled(false); - assertThat(oicSecurityRealm.isEscapeHatchEnabled(), is(false)); + assertThat(oicSecurityRealm.getProperties().get(EscapeHatch.class), nullValue()); } @Test void readResolve() throws Exception { OicSecurityRealm oicSecurityRealm = new OicSecurityRealm("clientId", null, null, null, null, null); - oicSecurityRealm.setEscapeHatchEnabled(false); - assertThat(oicSecurityRealm.isEscapeHatchEnabled(), is(false)); + assertThat(oicSecurityRealm.getProperties().get(EscapeHatch.class), nullValue()); oicSecurityRealm.readResolve(); + assertThat(oicSecurityRealm.getProperties().get(EscapeHatch.class), nullValue()); } } diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java index 84097af2..63d94f6a 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java +++ b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java @@ -6,12 +6,15 @@ import hudson.util.Secret; import io.burt.jmespath.Expression; import java.io.IOException; +import java.io.ObjectStreamException; import java.io.Serial; import java.text.ParseException; import java.util.ArrayList; import java.util.List; import jenkins.model.IdStrategy; +import org.jenkinsci.plugins.oic.properties.DisableNonce; import org.jenkinsci.plugins.oic.properties.DisableTokenVerification; +import org.jenkinsci.plugins.oic.properties.EscapeHatch; import org.jenkinsci.plugins.oic.properties.LoginQueryParameters; import org.jenkinsci.plugins.oic.properties.LogoutQueryParameters; import org.kohsuke.stapler.StaplerRequest2; @@ -53,10 +56,6 @@ public static class Builder { public Boolean logoutFromOpenidProvider = false; public String endSessionEndpoint = null; public String postLogoutRedirectUrl = null; - public boolean escapeHatchEnabled = false; - public String escapeHatchUsername = null; - public Secret escapeHatchSecret = null; - public String escapeHatchGroup = null; public boolean automanualconfigure = false; public IdStrategy userIdStrategy; public IdStrategy groupIdStrategy; @@ -150,11 +149,14 @@ public Builder WithEscapeHatch( boolean escapeHatchEnabled, String escapeHatchUsername, String escapeHatchSecret, - String escapeHatchGroup) { - this.escapeHatchEnabled = escapeHatchEnabled; - this.escapeHatchUsername = escapeHatchUsername; - this.escapeHatchSecret = escapeHatchSecret == null ? null : Secret.fromString(escapeHatchSecret); - this.escapeHatchGroup = escapeHatchGroup; + String escapeHatchGroup) + throws Descriptor.FormException { + if (escapeHatchEnabled) { + this.properties.add( + new EscapeHatch(escapeHatchUsername, escapeHatchGroup, Secret.fromString(escapeHatchSecret))); + } else { + this.properties.removeIf(EscapeHatch.class::isInstance); + } return this; } @@ -233,10 +235,6 @@ public TestRealm(Builder builder) throws Exception { this.setGroupsFieldName(builder.groupsFieldName); this.setLogoutFromOpenidProvider(builder.logoutFromOpenidProvider); this.setPostLogoutRedirectUrl(builder.postLogoutRedirectUrl); - this.setEscapeHatchEnabled(builder.escapeHatchEnabled); - this.setEscapeHatchUsername(builder.escapeHatchUsername); - this.setEscapeHatchSecret(builder.escapeHatchSecret); - this.setEscapeHatchGroup(builder.escapeHatchGroup); this.setProperties(builder.properties); // need to call the following method annotated with @PostConstruct and called // from readResolve and as such @@ -306,7 +304,8 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th /* * PluginTest uses a hardCoded nonce "nonce" */ - if (!isNonceDisabled()) { + + if (getProperties().get(DisableNonce.class) == null) { // only hack the nonce if the nonce is enabled FrameworkParameters parameters = new JEEFrameworkParameters(request, response); WebContext webContext = JEEContextFactory.INSTANCE.newContext(parameters); @@ -327,11 +326,7 @@ public String getStringFieldFromJMESPath(Object object, String jmespathField) { @Serial @Override - public Object readResolve() throws IOException, Descriptor.FormException { + public Object readResolve() throws ObjectStreamException { return super.readResolve(); } - - public boolean doCheckEscapeHatch(String username, String password) { - return super.checkEscapeHatch(username, password); - } } diff --git a/src/test/java/org/jenkinsci/plugins/oic/properties/EscapeHatchTest.java b/src/test/java/org/jenkinsci/plugins/oic/properties/EscapeHatchTest.java index 3f230aea..858cfcbd 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/properties/EscapeHatchTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/properties/EscapeHatchTest.java @@ -1,14 +1,18 @@ package org.jenkinsci.plugins.oic.properties; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import hudson.util.Secret; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import org.jenkinsci.plugins.oic.MockHttpServletRequest; import org.junit.jupiter.api.Test; +import org.springframework.security.crypto.bcrypt.BCrypt; class EscapeHatchTest { private EscapeHatch.CrumbExclusionImpl crumb = new EscapeHatch.CrumbExclusionImpl(); @@ -47,4 +51,27 @@ void process_WithGoodPath() throws IOException, ServletException { MockHttpServletRequest request = newRequestWithPath("/securityRealm/escapeHatch"); assertTrue(crumb.process(request, response, chain)); } + + @Test + void testShouldCheckEscapeHatchWithPlainPassword() throws Exception { + final String escapeHatchUsername = "aUsername"; + final String escapeHatchPassword = "aSecretPassword"; + var escapeHatch = new EscapeHatch(escapeHatchUsername, null, Secret.fromString(escapeHatchPassword)); + assertEquals(escapeHatchUsername, escapeHatch.getUsername()); + assertNotEquals(escapeHatchPassword, Secret.toString(escapeHatch.getSecret())); + assertTrue(escapeHatch.check(escapeHatchUsername, escapeHatchPassword)); + assertFalse(escapeHatch.check("otherUsername", escapeHatchPassword)); + assertFalse(escapeHatch.check(escapeHatchUsername, "wrongPassword")); + } + + @Test + void testShouldCheckEscapeHatchWithHashedPassword() throws Exception { + final String escapeHatchUsername = "aUsername"; + final String escapeHatchPassword = "aSecretPassword"; + final String escapeHatchCryptedPassword = BCrypt.hashpw(escapeHatchPassword, BCrypt.gensalt()); + + var escapeHatch = new EscapeHatch(escapeHatchUsername, null, Secret.fromString(escapeHatchCryptedPassword)); + assertEquals(escapeHatchUsername, escapeHatch.getUsername()); + assertEquals(escapeHatchCryptedPassword, Secret.toString(escapeHatch.getSecret())); + } }