Introduce `OidcProperty` extension point by Vlatombe · Pull Request #644 · jenkinsci/oic-auth-plugin

Vlatombe · 2025-08-22T13:26:58Z

In the context of CloudBees CI, I needed the ability to implement new
optional behaviour customizing the OIDC configuration and client.

The new extension point, named OidcProperty, allows to capture a state
through OidcPropertyExecution that can be used for customize both the
OIDC configuration and client.

I have also taken the opportunity to fold some advanced options under
their own class. Ultimately, all advanced fields could be implemented
as OidcProperty but for some of them it can be challenging due to the
way they are currently integrated in the plugin.

Migrated fields

Allowed token expiration clock skew
Disable nonce
Disable token verification
Escape hatch
Login query parameters
Logout query parameters
Enable PKCE

Screenshots

Testing done

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests that demonstrate the feature works or the issue is fixed

In the context of CloudBees CI, I needed the ability to implement new optional behaviour customizing the OIDC configuration and client. The new extension point, named `OicProperty`, allows to capture a state through `OicPropertyExecution` that can be used for customize both the OIDC configuration and client. I have also taken the opportunity to fold some advanced options under their own class. Ultimately, all advanced fields could be implemeneted as `OicProperty` but for some of them it can be challenging due to the way they are currently integrated in the plugin. Migrated fields * Allowed token expiration clock skew * Disable nonce * Disable token verification * Escape hatch * Login query parameters * Logout query parameters * Enable PKCE

Vlatombe · 2025-08-22T13:37:29Z

Actually moved to EscapeHatch

Vlatombe · 2025-08-22T13:38:14Z

     * Convert the OicServerConfiguration to {@link OIDCProviderMetadata} for use by the client.
     */
-    public abstract OIDCProviderMetadata toProviderMetadata();
+    public final OIDCProviderMetadata toProviderMetadata() {


Moved from OicSecurityRealm

codecov · 2025-08-27T14:56:13Z

Codecov Report

❌ Patch coverage is 75.25773% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.74%. Comparing base (5e6e0ae) to head (0d6d19c).
⚠️ Report is 23 commits behind head on master.

Files with missing lines	Patch %	Lines
.../jenkinsci/plugins/oic/properties/EscapeHatch.java	42.55%	24 Missing and 3 partials ⚠️
...va/org/jenkinsci/plugins/oic/OicSecurityRealm.java	64.51%	12 Missing and 10 partials ⚠️
.../jenkinsci/plugins/oic/OicServerConfiguration.java	90.21%	6 Missing and 3 partials ⚠️
...ic/properties/AllowedTokenExpirationClockSkew.java	69.23%	4 Missing ⚠️
...org/jenkinsci/plugins/oic/LoginQueryParameter.java	0.00%	1 Missing and 1 partial ⚠️
...rg/jenkinsci/plugins/oic/LogoutQueryParameter.java	0.00%	1 Missing and 1 partial ⚠️
...i/plugins/oic/properties/LoginQueryParameters.java	85.71%	1 Missing and 1 partial ⚠️
.../plugins/oic/properties/LogoutQueryParameters.java	77.77%	1 Missing and 1 partial ⚠️
.../jenkinsci/plugins/oic/OidcPropertyDescriptor.java	75.00%	1 Missing ⚠️
...ugins/oic/properties/DisableTokenVerification.java	95.45%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #644      +/-   ##
============================================
- Coverage     74.97%   74.74%   -0.23%     
- Complexity      289      326      +37     
============================================
  Files            24       33       +9     
  Lines          1179     1299     +120     
  Branches        167      179      +12     
============================================
+ Hits            884      971      +87     
- Misses          202      241      +39     
+ Partials         93       87       -6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

PereBueno

From the part of the plugin I'm familiar with (FIPS and nonce checks mainly) everything seems fine.
Only problem I found is migrating the clockskew property

Vlatombe · 2025-09-02T08:40:17Z

        // set many more as needed...

        OIDCProviderMetadata oidcProviderMetadata = serverConfiguration.toProviderMetadata();
-        filterNonFIPS140CompliantAlgorithms(oidcProviderMetadata);


Moved to OicServerConfiguration

Vlatombe · 2025-09-02T08:40:43Z

        OIDCProviderMetadata oidcProviderMetadata = serverConfiguration.toProviderMetadata();
-        filterNonFIPS140CompliantAlgorithms(oidcProviderMetadata);
-        OidcOpMetadataResolver opMetadataResolver;
-        if (this.isDisableTokenVerification()) {


Moved to DisableTokenVerification

Vlatombe · 2025-09-02T08:40:54Z

            // auto configuration does not need to supply scopes
            conf.setScope(oidcProviderMetadata.getScopes().toString());
        }
-        conf.setUseNonce(!this.nonceDisabled);


Moved to DisableNonce.

Vlatombe · 2025-09-02T08:41:20Z

            conf.setScope(oidcProviderMetadata.getScopes().toString());
        }
-        conf.setUseNonce(!this.nonceDisabled);
-        if (allowedTokenExpirationClockSkewSeconds != null) {


Moved AllowedTokenExpirationClockSkew

Vlatombe · 2025-09-02T08:41:27Z

-            conf.setMaxClockSkew(allowedTokenExpirationClockSkewSeconds.intValue());
-        }
        conf.setResourceRetriever(getResourceRetriever());
-        if (this.isPkceEnabled()) {


Moved to Pkce

Vlatombe · 2025-09-02T08:41:41Z

-            conf.setDisablePkce(true);
-        }
-        opMetadataResolver.init();
-        if (loginQueryParameters != null && !loginQueryParameters.isEmpty()) {


Moved to LoginQueryParameters

Vlatombe · 2025-09-02T08:42:17Z

        return conf;
    }

-    // Visible for testing


Moved to OicServerConfiguration

Vlatombe · 2025-09-02T08:42:37Z

        return new SecurityComponents(new AuthenticationManager() {
            public Authentication authenticate(Authentication authentication) throws AuthenticationException {
                if (authentication instanceof AnonymousAuthenticationToken) return authentication;
-


Moved to EscapeHatch

Vlatombe · 2025-09-02T08:43:20Z

+                uriComponentsBuilder.queryParam(
                        "post_logout_redirect_uri", URLEncoder.encode(postLogoutRedirectUrl, StandardCharsets.UTF_8));
            }
-            if (logoutQueryParameters != null && !logoutQueryParameters.isEmpty()) {


Moved to LogoutQueryParameters

jtnord

comments inline, but also the docs will need updating for the casc changes

jtnord · 2025-09-05T11:09:30Z

+     * Allows a property to authenticate the user.
+     * @see org.jenkinsci.plugins.oic.properties.EscapeHatch
+     */
+    public Optional<Authentication> authenticate(Authentication authentication) {


I'm confused as to how this can work generically in practice with multiple authenticators?

Would this work with say bearer tokens?

For bearer token, I think you would need to add a way to plug a new filter into the authentication chain (doesn't exist currently, I only added the bare minumum for EscapeHatch to work), to convert the http headers to a BearerTokenAuthentication (to be written). And then an implementation of this method does what https://github.com/jenkinsci/oic-auth-plugin/pull/632/files#diff-9c2c6436b143fde652866ea92ee9bec48671b094dbb23ae4805621c5d40ed59bR1514-R1587 currently does I guess ?

jtnord · 2025-09-08T09:47:39Z

spotless needs to be applied, otherwise LGTM

Vlatombe · 2025-09-09T12:08:46Z

Still need to update the doc...

As the on disk format has changed, update the compatibleSinceVersion last release was 4.524.v38858a_b_1c6a_4 so 4.525 will be enough.

jglick · 2025-09-09T16:33:41Z

+/**
+ * Represents a property that can be configured for OIDC authentication.
+ */
+public abstract class OidcProperty extends AbstractDescribableImpl<OidcProperty> {


note jenkinsci/jenkins#10426

jglick · 2025-09-09T16:34:41Z

+import hudson.model.Descriptor;
+import org.pac4j.oidc.config.OidcConfiguration;
+
+public abstract class OidcPropertyDescriptor extends Descriptor<OidcProperty> implements ExtensionPoint {


Did you mean to narrow-override getDescriptor in OidcProperty?

jglick · 2025-09-09T16:35:40Z

+        return new ExecutionImpl(valueSeconds);
+    }
+
+    private record ExecutionImpl(int valueSeconds) implements OidcPropertyExecution {


Is there some advantage over just using an anonymous inner class?

jglick · 2025-09-09T16:37:06Z

+        }
+    }
+
+    @CheckForNull


jglick · 2025-09-09T16:39:36Z

+- "pkce"
+- loginQueryParameters:
+    items:
+    - key: "loginkey1x\"xx@me"


(ignore WS, even though it is significant)

jglick · 2025-09-09T17:14:34Z

BTW I find it helpful for both the author and reviewers to mark conversations resolved as promptly as possible, so it is easy to get an overview of all outstanding threads. (Enforcing this in branch protection is too draconian; sometimes there is a conversation you want to keep visible but still go ahead and merge.)

Vlatombe requested a review from a team as a code owner August 22, 2025 13:26

Vlatombe requested review from PereBueno and jtnord August 22, 2025 13:27

Missing getter

033b036

Vlatombe commented Aug 22, 2025

View reviewed changes

Vlatombe added 4 commits August 22, 2025 15:52

Javadoc

f146ea3

Fix copy/paste error

0f1bad9

Add javadoc

e37b046

Fix javadoc

efcabaf

PereBueno reviewed Aug 29, 2025

View reviewed changes

Comment thread src/main/java/org/jenkinsci/plugins/oic/properties/AllowedTokenExpirationClockSkew.java

Vlatombe added 2 commits September 1, 2025 14:30

Add missing getter

6749a67

Improve test assertions

e7fe954

jtnord added developer breaking and removed breaking labels Sep 1, 2025

Merge remote-tracking branch 'upstream/master' into oicproperty

a7a66e9

Vlatombe commented Sep 2, 2025

View reviewed changes

Revert visibility

5bbabdf

jtnord reviewed Sep 5, 2025

View reviewed changes

Comment thread src/main/java/org/jenkinsci/plugins/oic/AbstractKeyValueDescribable.java Outdated

Reduce diff

ac88b2a

jtnord reviewed Sep 5, 2025

View reviewed changes

Vlatombe added 4 commits September 8, 2025 10:23

Oic -> Oidc for new classes

33a0e8e

value -> valueSeconds for clarity

f10abec

EscapeHatch username must not be blank

76399d6

Minimum 1 query parameter per property

0ca5d07

Vlatombe added 2 commits September 8, 2025 13:41

Spotless

a1a8c5e

Forgot one occurrence

0ba995f

Vlatombe changed the title ~~Introduce OicProperty extension point~~ Introduce OidcProperty extension point Sep 8, 2025

Vlatombe requested a review from jtnord September 9, 2025 09:13

Vlatombe and others added 2 commits September 9, 2025 15:24

Update doc

0f36904

Update hpi.compatibleSinceVersion to 4.525

0d6d19c

As the on disk format has changed, update the compatibleSinceVersion last release was 4.524.v38858a_b_1c6a_4 so 4.525 will be enough.

jtnord approved these changes Sep 9, 2025

View reviewed changes

jtnord enabled auto-merge September 9, 2025 15:16

jtnord merged commit d5a61e0 into jenkinsci:master Sep 9, 2025
17 of 18 checks passed

jglick reviewed Sep 9, 2025

View reviewed changes

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

}

}

@CheckForNull

Copy link
Copy Markdown

Member

jglick Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

jglick reviewed Sep 9, 2025

View reviewed changes

Vlatombe mentioned this pull request Sep 10, 2025

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

Merged

6 tasks

PiersRBME mentioned this pull request Sep 10, 2025

"Introduce OidcProperty extension point" is a breaking change for CasC #655

Closed

jtnord added the breaking label Sep 11, 2025

jglick mentioned this pull request Sep 17, 2025

Fix help display behaviors in the trait list UI jenkinsci/scm-api-plugin#328

Merged

6 tasks

Uh oh!

Conversation

Vlatombe commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing done

Submitter checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PereBueno left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vlatombe Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jtnord left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtnord commented Sep 8, 2025

Uh oh!

Vlatombe commented Sep 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglick commented Sep 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Vlatombe commented Aug 22, 2025 •

edited

Loading

codecov Bot commented Aug 27, 2025 •

edited

Loading

Vlatombe Sep 2, 2025 •

edited

Loading