Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Security][SecurityBundle] OIDC discovery#54932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedMay 15, 2024
Hey! Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "7.2". Cheers! Carsonbot |
@Spomky You might be interested by this PR |
00bf574
toc8c5d7a
ComparePlease note that I recently released the Web Token suite 4.0 and proposed#57694. |
Hi@Spomky, Yep I just saw that and fixed the conflicts on OidcTokenHandler and services declarations. Do you think it's a good thing to inject a JWSLoader and check the token through it, instead of creating multiple objects (checkers, managers, etc.) and checking the token with ClaimCheckerManager? |
853eaf3
tob379c48
Compare
I am not sure. To me, the way the tokens are loaded and their content depends on the context of their use i.e. the algorithms, the keys and the verified headers/claims should not be centralized. The JWSLoader service will then only be used by one token handler and have no advantages. |
fe1b665
to8b75d09
Compare8b75d09
toafc9b6e
Compareafc9b6e
to1a9fe5b
Comparewgorczyca commentedNov 4, 2024
any update about this feature? |
Still waiting for a review |
It is too late for 7.2 but I'd like this PR to make it into the next development phase, so expect some review from me asap. |
a5253b4
to7762b2b
Compare1060bbe
to434cf63
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This PR looks great. Many thanks.
Please refer to 7.3 instead of 7.2 and it's good to me.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
434cf63
to93f369a
CompareThank you@vincentchalamon. |
8b9ed36
intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
This PR was merged into the 7.3 branch.Discussion----------[Security] Fix typos in OIDC methods| Q | A| ------------- | ---| Branch? | 7.3| Bug fix? | yes| New feature? | no| License | MITRelated to recently merged [OIDC discovery](#54932), the DI is configured to call `enableDiscovery` method but it does not exist – but there is `enabledDiscovery`.Let's drop the extra `d`, and the same for `enabledJweSupport` too.Commits-------3d6cc19 Fix typos in OIDC methods
Uh oh!
There was an error while loading.Please reload this page.
This PR introducesOIDC discovery on
oidc
andoidc_user_info
token handlers.TODO
What is OIDC Discovery?
OIDC discovery is a generic endpoint on the OIDC server, which gives any public information such as signature public keys and endpoints URIs (userinfo, token, etc.). An example is available on the API Platform Demo:
https://demo.api-platform.com/oidc/realms/demo/.well-known/openid-configuration.
Using the OIDC discovery simplifies the
oidc
security configuration, allowing to just configure the discovery and let Symfony store the configuration and the keyset in cache. For instance, if theuserinfo_endpoint orsignature keyset change on the OIDC server, no need to update the environment variables in the Symfony application, just clear the corresponding cache and it'll retrieve the configuration and the keyset accordingly on the next request.In the
oidc_user_info
security configuration, it does the same logic but only aboutuserinfo_endpoint as this token handler doesn't need thekeyset.How Do I Use This New Feature in Symfony?
The current
oidc
token handler configuration requires akeyset
option which may change on the OIDC server. It is configured as following:With the
discovery
option, Symfony will retrieve thekeyset
directly from the OIDC discovery URI and store it in a cache:The current
oidc_user_info
token handler required abase_uri
corresponding to theuserinfo_endpoint URI on the OIDC server. This URI may change if it's changed on the OIDC server. Introducing the discovery helps to configure it dynamically.The current configuration looks like the following:
With the
discovery
, it will look like this: