Fix OAuth issuer discovery to comply with RFC 8414 and RFC 9728#1839
Merged
Fix OAuth issuer discovery to comply with RFC 8414 and RFC 9728#1839
Conversation
96b7a6b to
bc172ab
Compare
jhrozek
reviewed
Sep 11, 2025
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1839 +/- ##
==========================================
- Coverage 42.66% 42.64% -0.02%
==========================================
Files 184 184
Lines 21853 22046 +193
==========================================
+ Hits 9323 9402 +79
- Misses 11809 11921 +112
- Partials 721 723 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jhrozek
reviewed
Sep 11, 2025
Collaborator
51dda60 to
805df43
Compare
- Add support for RFC 9728 Protected Resource Metadata discovery - Fix issuer detection to properly handle cases where the metadata URL differs from the actual issuer (e.g., Stripe's case) - Add resource_metadata parameter parsing from WWW-Authenticate header - Implement FetchResourceMetadata to retrieve protected resource metadata - Add ValidateAndDiscoverAuthServer to handle issuer validation and discovery - Update OAuth flow to use pre-discovered endpoints when available - Fix DeriveIssuerFromRealm to validate realm as a proper HTTPS URL - Add DiscoverActualIssuer in OIDC package to handle issuer mismatch cases - Add workaround for resource metadata that incorrectly lists authorization servers that don't match the actual issuer (validates each server and uses the discovered issuer) - Update tests to reflect the new DeriveIssuerFromRealm function behavior This fixes the bug where issuer detection was incorrectly trying to derive the issuer from the remote URL instead of using the realm parameter or fetching resource metadata as specified in the RFCs. The implementation now properly handles edge cases like Stripe's where the resource metadata URL hosts the authorization server metadata but the actual issuer identifier is different. Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
805df43 to
21c3ae3
Compare
jhrozek
approved these changes
Sep 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes the bug where issuer detection was incorrectly trying to derive
the issuer from the remote URL instead of using the realm parameter or
fetching resource metadata as specified in the RFCs.
The implementation now properly handles edge cases like Stripe's where the
resource metadata URL hosts the authorization server metadata but the actual
issuer identifier is different.