OidcProperty follow-ups; Fix application of clock skew to generated credentials

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

@@ -372,43 +372,49 @@
     }
 
     @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 @@
         return this;
     }
 
-    @Deprecated
-    public void setLoginQueryParameters(List<LoginQueryParameter> values) {
-        this.loginQueryParameters = values;
-    }
-
-    @Deprecated
-    public List<LoginQueryParameter> getLoginQueryParameters() {
-        return loginQueryParameters;
-    }
-
-    @Deprecated
-    public void setLogoutQueryParameters(List<LogoutQueryParameter> values) {
-        this.logoutQueryParameters = values;
-    }
-
-    @Deprecated
-    public List<LogoutQueryParameter> getLogoutQueryParameters() {
-        return logoutQueryParameters;
-    }
-
     public String getClientId() {
         return clientId;
     }
@@ -555,26 +541,6 @@
         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 @@
         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 @@
         return allowTokenAccessWithoutOicSession;
     }
 
-    /**
-     * @deprecated use {@link AllowedTokenExpirationClockSkew} instead.
-     * @return
-     */
-    @Deprecated
-    public Long getAllowedTokenExpirationClockSkewSeconds() {
-        return allowedTokenExpirationClockSkewSeconds;
-    }
-
     public DescribableList<OidcProperty, OidcPropertyDescriptor> getProperties() {
         return properties;
     }
@@ -737,19 +682,6 @@
         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 @@
         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 @@
         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 @@
         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 @@
                     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 @@
                     refreshToken.getValue(),
                     accessToken.getLifetime(),
                     CLOCK.millis(),
-                    getAllowedTokenExpirationClockSkewSeconds());
+                    (long) client.getConfiguration().getMaxClockSkew());
 
             loginAndSetUserData(username, idToken, profile.getAttributes(), refreshedCredentials);
             return true;

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 @@
         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 @@
 
     @Extension
     public static class DescriptorImpl extends OidcPropertyDescriptor {
+
         @Override
         public boolean isApplicable() {
             return !FIPS140.useCompliantAlgorithms();

src/main/java/org/jenkinsci/plugins/oic/properties/EscapeHatch.java

@@ -15,13 +15,15 @@
 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;
 import java.util.Random;
 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 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 @@
     public Optional<Authentication> 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<GrantedAuthority> grantedAuthorities = new ArrayList<>();
                 grantedAuthorities.add(SecurityRealm.AUTHENTICATED_AUTHORITY2);
                 if (isNotBlank(group)) {
@@ -124,11 +127,21 @@
         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 @@
         }
     }
 
+    @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.