Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
fabpot merged 1 commit intosymfony:7.3fromvincentchalamon:feat/oidc-discovery
Mar 1, 2025

Conversation

vincentchalamon
Copy link
Contributor

@vincentchalamonvincentchalamon commentedMay 15, 2024
edited
Loading

This PR introducesOIDC discovery onoidc andoidc_user_info token handlers.

QA
Branch?7.2
Bug fix?no
New feature?yes
Deprecations?yes
IssuesFix#50433Fix#50434
LicenseMIT
Doc PRsymfony/symfony-docs#20579

TODO

  • use JWSLoader in OidcTokenHandler
  • introduce OidcUserInfoDiscoveryTokenHandler
  • introduce OidcDiscoveryTokenHandler
  • update src/**/CHANGELOG.md files
  • update UPGRADE-*.md files
  • add tests on AccessTokenFactoryTest with discovery
  • create documentation PR

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 theoidc 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 theoidc_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 currentoidc token handler configuration requires akeyset option which may change on the OIDC server. It is configured as following:

# config/packages/security.yamlsecurity:firewalls:main:access_token:oidc:claim:'email'audience:'symfony'issuers:['https://example.com/']algorithms:['RS256']keyset:'{"keys":[{"kty":"EC",...}]}'

Note: those parameters should be configured with environment variables.

With thediscovery option, Symfony will retrieve thekeyset directly from the OIDC discovery URI and store it in a cache:

# config/packages/security.yamlsecurity:firewalls:main:access_token:oidc:# 'keyset' option is not necessary anymore as it's retrieved from OIDC discovery and stored in cacheclaim:'email'audience:'symfony'issuers:['https://example.com/']algorithms:['RS256']discovery:base_uri:'https://example.com/oidc/realms/master/'cache:id:cache.app# require to create this cache in framework.yaml

Note: some other parameters might be retrieven from the OIDC discovery, maybe 'algorithm' or 'issuers'. To discuss.

The currentoidc_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:

# config/packages/security.yamlsecurity:firewalls:main:access_token:oidc_user_info:# 'base_uri' is the userinfo_endpoint URIbase_uri:'https://example.com/oidc/realms/master/protocol/openid-connect/userinfo'claim:'email'

With thediscovery, it will look like this:

# config/packages/security.yamlsecurity:firewalls:main:access_token:oidc_user_info:# 'base_uri' can be the userinfo_endpoint for backward compatibility# and can be the OIDC server url in addition of 'discovery' optionbase_uri:'https://example.com/oidc/realms/master/'claim:'email'discovery:cache:id:cache.app# require to create this cache in framework.yaml

chalasr, welcoMattic, JustDylan23, and jdreesen reacted with hooray emojidgoosens, maxbeckers, valtzu, soyuka, Spomky, petski, turegjorup, welcoMattic, JustDylan23, MilanPol, and 4 more reacted with rocket emoji
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "7.2".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@vincentchalamon
Copy link
ContributorAuthor

@Spomky You might be interested by this PR

Spomky reacted with thumbs up emoji

@xabbuhxabbuh modified the milestones:7.1,7.2May 15, 2024
@carsonbotcarsonbot changed the title[Security] OIDC discovery[Security][SecurityBundle] OIDC discoveryMay 15, 2024
@vincentchalamonvincentchalamonforce-pushed thefeat/oidc-discovery branch 4 times, most recently from00bf574 toc8c5d7aCompareJuly 10, 2024 07:08
@Spomky
Copy link
Contributor

Hi@vincentchalamon,

Please note that I recently released the Web Token suite 4.0 and proposed#57694.
The migration is normally simple, but it impacts the claims/header checking classes ofiat,nbf,exp. And may therefore have an impact on this PR (to be confirmed).

vincentchalamon reacted with thumbs up emoji

@vincentchalamon
Copy link
ContributorAuthor

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?

@Spomky
Copy link
Contributor

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?

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.

@vincentchalamonvincentchalamonforce-pushed thefeat/oidc-discovery branch 2 times, most recently fromfe1b665 to8b75d09CompareJuly 15, 2024 12:37
@wgorczyca
Copy link

any update about this feature?

@vincentchalamon
Copy link
ContributorAuthor

any update about this feature?

Still waiting for a review

@chalasr
Copy link
Member

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.

vincentchalamon, petski, Hecke29, and valtzu reacted with thumbs up emoji

@vincentchalamonvincentchalamonforce-pushed thefeat/oidc-discovery branch 2 times, most recently froma5253b4 to7762b2bCompareNovember 5, 2024 07:49
@fabpotfabpot removed this from the7.2 milestoneNov 20, 2024
Copy link
Contributor

@SpomkySpomky left a 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.

@fabpot
Copy link
Member

Thank you@vincentchalamon.

valtzu reacted with rocket emoji

@fabpotfabpot merged commit8b9ed36 intosymfony:7.3Mar 1, 2025
8 of 11 checks passed
@vincentchalamonvincentchalamon deleted the feat/oidc-discovery branchMarch 1, 2025 18:16
fabpot added a commit that referenced this pull requestMar 15, 2025
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
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@SpomkySpomkySpomky approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Security] Importoidc.signature.key JWK from OIDC server [Security] OIDC Discovery
8 participants
@vincentchalamon@carsonbot@Spomky@wgorczyca@chalasr@fabpot@derrabus@xabbuh

[8]ページ先頭

©2009-2025 Movatter.jp