Skip to content

Harden OIDC token refresh and callback recovery - #780

Open
xykong wants to merge 6 commits into
jenkinsci:masterfrom
xykong:fix/oidc-refresh-hardening
Open

xykong wants to merge 6 commits into
jenkinsci:masterfrom
xykong:fix/oidc-refresh-hardening

Conversation

@xykong

@xykong xykong commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

Harden the expired OIDC token path so concurrent Jenkins requests do not
trigger refresh storms or redirect AJAX calls into an interactive login, and
restart login safely when an old callback arrives with missing or mismatched
session state.

This supersedes the closed #748 and preserves @scheremisin's authorship on the
rebased refresh-hardening commit. It also incorporates the focused behavior
discussed in #753 and adds callback-state recovery observed while reproducing
the same failure family.

Related reports and proposals: #411, #467, #706, #748, #753.

Changes

  • Serialize refresh per Jenkins user and re-read credentials after acquiring
    the lock, so rotated refresh tokens are not reused concurrently.
  • Attempt refresh when a stored refresh token exists and discovery metadata
    omits the optional grant_types_supported field.
  • Return 401 Unauthorized to AJAX and other non-interactive requests instead
    of redirecting them into the OIDC authorization flow.
  • Use a Jenkins context-relative, root-based URL for interactive login
    redirects.
  • Clear stored credentials after an invalid_grant refresh response so polling
    requests do not retry a known-invalid refresh token.
  • Restart interactive login when the callback has missing or mismatched OIDC
    session state, without calling the token endpoint.
  • Add focused concurrency, fallback, invalid-token, redirect, and callback-state
    tests.

Why this is needed

Jenkins pages routinely issue several concurrent widget and AJAX requests. If
the access token expires while the Jenkins HTTP session remains valid, all of
those requests can observe the same credentials. Providers that rotate refresh
tokens may accept the first refresh and reject the others with
invalid_grant. If refresh is unavailable, redirecting an AJAX request to the
authorization endpoint can also produce CORS errors, stale crumbs, nested-path
login URLs, and multiple overlapping callbacks.

OIDC discovery defines grant_types_supported as optional. Its absence should
not override the stronger runtime signal that the provider already issued a
refresh token.

Testing done

mvn verify
Tests run: 172, Failures: 0, Errors: 0, Skipped: 2
Checkstyle violations: 0
SpotBugs errors/warnings: 0
Spotless: clean
BUILD SUCCESS

The new tests execute:

  • refresh capability with absent and explicit grant-type metadata;
  • same-user concurrent refresh;
  • interactive and non-interactive fallback behavior;
  • invalid refresh credential cleanup;
  • callback recovery for missing and mismatched session state.

Submitter checklist

  • Opened from a topic branch
  • PR title represents the desired changelog entry
  • Described the behavior and compatibility impact
  • Linked relevant issues and prior pull requests
  • Added automated tests that execute the changed paths

@xykong
xykong requested a review from a team as a code owner July 17, 2026 04:51
@codecov

codecov Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.72%. Comparing base (cd6a7a8) to head (43ad60d).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #780      +/-   ##
============================================
+ Coverage     76.06%   77.72%   +1.66%     
- Complexity      325      351      +26     
============================================
  Files            33       33              
  Lines          1291     1347      +56     
  Branches        178      191      +13     
============================================
+ Hits            982     1047      +65     
+ Misses          227      221       -6     
+ Partials         82       79       -3     

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants