Skip to content

feat(iam): retry OIDC discovery with issuer URL slash variants#2360

Merged
overtrue merged 5 commits into
rustfs:mainfrom
GatewayJ:feat/oidc
Apr 3, 2026
Merged

feat(iam): retry OIDC discovery with issuer URL slash variants#2360
overtrue merged 5 commits into
rustfs:mainfrom
GatewayJ:feat/oidc

Conversation

@GatewayJ
Copy link
Copy Markdown
Member

@GatewayJ GatewayJ commented Apr 1, 2026

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other:

Related Issues

2349

Summary of Changes

OIDC provider discovery could fail when the issuer derived from config_url differed from the IdP discovery document’s canonical issuer only by a trailing slash (common across providers). discover_provider now tries both variants (no trailing slash, then with trailing slash) before failing, logs each failed attempt, and returns an aggregated error when all variants fail. Added issuer_candidates unit tests and async tests with a minimal local HTTP mock for validate_oidc_provider_config.

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit
  • Added/updated necessary tests
  • Documentation updated (if needed)
  • CI/CD passed (if applicable)

Impact

  • Breaking change (compatibility)
  • Requires doc/config/deployment update
  • Other impact:

N/A — no intentional API or configuration breaking changes. Successful discovery still stores the same CoreProviderMetadata as returned by the IdP; JWT issuer matching via normalize_issuer is unchanged.

Additional Notes

Verification (local):

cargo test -p rustfs-iam oidc

make pre-commit was not run in this session; please rely on CI for fmt/clippy/full tests.


Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

@GatewayJ GatewayJ requested a review from loverustfs April 1, 2026 03:05
@GatewayJ GatewayJ force-pushed the feat/oidc branch 3 times, most recently from e5decff to 836a352 Compare April 1, 2026 05:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves OIDC provider discovery robustness when the configured issuer differs from the IdP’s canonical issuer only by a trailing slash, by retrying discovery with slash/no-slash issuer variants and returning a more detailed aggregated error when all attempts fail.

Changes:

  • Retry CoreProviderMetadata::discover_async using issuer URL candidates with and without a trailing slash.
  • Log per-attempt discovery failures and return an aggregated “all variants failed” error message.
  • Add unit tests for issuer candidate generation and async validation tests using a minimal local HTTP mock server.

Comment thread crates/iam/src/oidc.rs Outdated
Comment thread crates/iam/src/oidc.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread crates/iam/src/oidc.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread crates/iam/src/oidc.rs
Comment thread crates/iam/src/oidc.rs Outdated
Comment thread crates/iam/src/oidc.rs
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

CLA requirements are satisfied for this pull request.

@GatewayJ
Copy link
Copy Markdown
Member Author

GatewayJ commented Apr 2, 2026

I have read and agree to the CLA.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread crates/iam/src/oidc.rs Outdated
GatewayJ added 3 commits April 3, 2026 00:12
- Exit mock thread after idle timeout once at least one request completed
- Read HTTP request until header terminator or 8KB cap
- Rename request_limit to max_requests; keep absolute 500ms safety cap

Made-with: Cursor
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@overtrue overtrue merged commit c244943 into rustfs:main Apr 3, 2026
8 checks passed
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.

4 participants