[Security][SecurityBundle] OIDC discovery

[vincentchalamon]

This PR introduces OIDC discovery on oidc and oidc_user_info token handlers.

Q	A
Branch?	7.2
Bug fix?	no
New feature?	yes
Deprecations?	yes
Issues	Fix #50433 Fix #50434
License	MIT

TODO

• [x] use JWSLoader in OidcTokenHandler
• [x] introduce OidcUserInfoDiscoveryTokenHandler
• [x] introduce OidcDiscoveryTokenHandler
• [ ] is there a way to retrieve algorithms and other parameters from OIDC discovery?
• [x] update src/**/CHANGELOG.md files
• [x] update UPGRADE-*.md files
• [ ] fix some TODOs
• [ ] add tests on AccessTokenFactoryTest with discovery
• [ ] fix tests
• [ ] 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 the oidc security configuration, allowing to just configure the discovery and let Symfony store the configuration and the keyset in cache. For instance, if the userinfo_endpoint or signature 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 about userinfo_endpoint as this token handler doesn't need the keyset.

Note About Introducing OIDC Discovery in OIDC Token Handlers

I first thought of adding this feature directly in the related token handler classes, but it would drastically increase the code complexity.

Then I thought adding oidc_discovery and oidc_user_info_discovery token handler configurations, but it would introduce some useless complexity with Symfony usage.

Finally, adding the discovery token handler classes but keep the oidc and oidc_user_info token handler configurations, and select the right class accordingly, seemed the cleanest solution IMHO.

How Do I Use This New Feature in Symfony?

The current oidc token handler configuration requires a keyset option which may change on the OIDC server. It is configured as following:

# config/packages/security.yaml
security:
    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 the discovery option, Symfony will retrieve the keyset directly from the OIDC discovery URI and store it in a cache:

# config/packages/security.yaml
security:
    firewalls:
        main:
            access_token:
                oidc:
                    # 'keyset' option is not necessary anymore as it's retrieved from OIDC discovery and stored in cache
                    claim: '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
                            default_lifetime: 600 # default: 86400 (1 day)

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

The current oidc_user_info token handler required a base_uri corresponding to the userinfo_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.yaml
security:
    firewalls:
        main:
            access_token:
                oidc_user_info:
                    # 'base_uri' is the userinfo_endpoint URI
                    base_uri: 'https://example.com/oidc/realms/master/protocol/openid-connect/userinfo'
                    claim: 'email'

With the discovery, it will look like this:

# config/packages/security.yaml
security:
    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' option
                    base_uri: 'https://example.com/oidc/realms/master/'
                    claim: 'email'
                    discovery:
                        cache:
                            id: cache.app # require to create this cache in framework.yaml
                            default_lifetime: 600 # default: 86400 (1 day)

Note: all these configurations will be kept, with and without discovery, allowing the developer to use the prefered one!

Why using JWSLoader in OidcTokenHandler?

Currently, the OidcTokenHandler creates its own dependencies such as JWSVerifier, JWSSerializerManager, Checkers, etc. The web-token/jwt-library has a JWSLoader which does the same job: unserialize the token, verify its signature, check its headers and claims.

Using JWSLoader instead of doing the job manually introduces the possibility to the developer to use custom checkers or custom configuration in the OidcTokenHandler (when using the component), simplifies the code and improve its maintenance.

[carsonbot]

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]

@Spomky You might be interested by this PR

[Spomky]

Hi @vincentchalamon,

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

[vincentchalamon]

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]

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.

[wgorczyca]

any update about this feature?

[vincentchalamon]

any update about this feature?

Still waiting for a review

[chalasr]

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.