fix: refresh authWellKnownEndpoints when the server updates them#2197
fix: refresh authWellKnownEndpoints when the server updates them#2197andsouto wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes stale/override persistence issues around OIDC discovery so that refreshed /.well-known/openid-configuration values are used authoritatively, and issuer mismatch checks can’t be bypassed by previously provided/stored endpoint overrides.
Changes:
- Stop hydrating
authWellknownEndpointsfrom storage during configuration loading. - Make discovery results authoritative by removing merge of config-provided endpoint overrides into mapped discovery endpoints.
- Add an “explicit endpoints” path that stores/returns
authWellknownEndpointsand skips the discovery network call.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/angular-auth-oidc-client/src/lib/config/config.service.ts | Removes loading/stamping well-known endpoints from storage during config initialization. |
| projects/angular-auth-oidc-client/src/lib/config/config.service.spec.ts | Updates unit tests to reflect that config is no longer hydrated from stored endpoints. |
| projects/angular-auth-oidc-client/src/lib/config/auth-well-known/auth-well-known.service.ts | Adds early return for explicitly configured authWellknownEndpoints (no discovery request). |
| projects/angular-auth-oidc-client/src/lib/config/auth-well-known/auth-well-known.service.spec.ts | Adds tests for explicit endpoints mode and discovery mode behavior. |
| projects/angular-auth-oidc-client/src/lib/config/auth-well-known/auth-well-known-data.service.ts | Removes merging of config overrides into discovery results; keeps strict issuer validation behavior. |
| projects/angular-auth-oidc-client/src/lib/config/auth-well-known/auth-well-known-data.service.spec.ts | Updates tests to ensure no merge occurs and issuer mismatch throws even with overrides present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (config.authWellknownEndpoints) { | ||
| this.storeWellKnownEndpoints(config, config.authWellknownEndpoints); | ||
|
|
||
| return of(config.authWellknownEndpoints); | ||
| } |
There was a problem hiding this comment.
This is an interesting point. IMO we need to take a design decision here to decide between two options:
- authWellknownEndpoints replaces everything and completely avoids the network request to discover endpoints.
- authWellknownEndpoints does not prevent the network request but just overrides things after it.
Personally I have not really an opinion as I'm not using that option. Currently, the PR implements option 1. Any opinion if this is right or if we should go with option 2? Reasons?
dafd9a9 to
f87994d
Compare
f87994d to
0638f4f
Compare
fix: refresh authWellKnownEndpoints when the server updates them
fix #1983
Problem
When using OIDC discovery,
authWellknownEndpointscould be influenced by previously persisted/config-provided values. That made it possible for stale overrides (or cached values) to “win” over the latest server discovery document, preventing the client from correctly refreshing endpoints after the IdP updates them.What this PR changes
Discovery results are authoritative
AuthWellKnownDataServiceno longer mergesconfig.authWellknownEndpointsinto the mapped discovery result./.well-known/openid-configurationdocument (i.e., fresh values), instead of being overwritten by potentially stale config overrides.Issuer mismatch is not bypassed by overrides
authWellknownEndpointscontains anissuervalue.Explicit endpoints mode is explicit (no discovery request)
authWellknownEndpointsis explicitly provided,AuthWellKnownServicereturns/stores them and skips the discovery network call.ConfigurationService no longer hydrates endpoints from storage
ConfigurationServicestops readingauthWellKnownEndPointsfromStoragePersistenceServiceand stamping them onto the runtime config.Why these decisions
This PR aims to fix #1983 with the smallest behavior change necessary to ensure refresh correctness:
authWellknownEndpoints(and the library won’t perform discovery in that case).Tests