Skip to content

[Security] Document access_token.token_handler.oidc_user_info #17463

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 10, 2023
Merged

[Security] Document access_token.token_handler.oidc_user_info #17463

merged 1 commit into from
May 10, 2023

Conversation

vincentchalamon
Copy link
Contributor

@vincentchalamon vincentchalamon commented Nov 25, 2022 •
edited
Loading

Refs symfony/symfony#48272

  • document oidc_user_info token handler
  • document oidc token handler

@carsonbot carsonbot added this to the 6.2 milestone Nov 25, 2022
@vincentchalamon vincentchalamon mentioned this pull request Nov 25, 2022
Merged
11 tasks
@vincentchalamon vincentchalamon changed the base branch from 6.2 to 6.3 November 25, 2022 13:40
@wouterj wouterj modified the milestones: 6.2, next Nov 26, 2022
@OskarStark OskarStark changed the title [Security] Document access_token.token_handler.oidc_user_info [Security] Document access_token.token_handler.oidc_user_info Nov 28, 2022
@OskarStark OskarStark added the Waiting Code Merge Docs for features pending to be merged label Nov 28, 2022
@vincentchalamon vincentchalamon marked this pull request as draft February 15, 2023 07:56
@vincentchalamon vincentchalamon marked this pull request as ready for review February 15, 2023 08:48
@carsonbot carsonbot modified the milestones: next, 6.3 Feb 15, 2023
@xabbuh xabbuh modified the milestones: 6.3, next Feb 16, 2023
OskarStark
OskarStark requested changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Versionadded directive is missing

OskarStark
OskarStark reviewed Apr 12, 2023
View reviewed changes
fabpot added a commit to symfony/symfony that referenced this pull request Apr 14, 2023
@fabpot
feature #48272 [Security] Add OidcUserInfoTokenHandler and OidcUser (…
1c79c79 
…vincentchalamon)

This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[Security] Add OidcUserInfoTokenHandler and OidcUser

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | N/A
| License       | MIT
| Doc PR        | symfony/symfony-docs#17463

Hi,

This PR aims to complete [the previous one](#46428) from `@Spomky` with an AccessTokenHandler ready-to-use with an OIDC server (Keycloak, Auth0).

## TODO

- [x] Rebase from 6.3
- [x] Rebase from #48285
- [x] Rebase from #48594
- [x] Write doc (symfony/symfony-docs#17463)
- [x] Add TokenHandlerFactory
- [x] Add ServiceTokenHandlerFactory for BC layer
- [x] Add OidcUserInfoTokenHandlerFactory
- [x] Add OidcTokenHandlerFactory (using web-token/jwt-*)
- [x] Implement OidcUser to keep user claims from OIDC server
- [x] Update doc PR about claims usage in a custom UserProvider
- [x] ~Update doc PR about OidcUserProvider usage~ (abandonned)

## Usage

```yaml
# usage with a custom client
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc_user_info:
                        client: oidc.client
```
```yaml
# usage with generic HttpClient
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc_user_info:
                        claim: email
                        client:
                            base_uri: https://www.example.com/realms/demo/protocol/openid-connect/userinfo
```
```yaml
# usage with token decode (no call to OIDC server)
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc:
                        signature:
                            # Algorithm used to sign the JWS
                            algorithm: 'HS256'
                            # A JSON-encoded JWK
                            key: '{"kty":"...","k":"..."}'
```
```php
# usage with a custom UserProvider
class CustomUserProvider implements UserProviderInterface
{
    public function loadUserByIdentifier(string $identifier, array $claims = []): UserInterface
    {
        // do some magic
    }
}
```

Commits
-------

99a35f0 [Security] Add OidcUserInfoTokenHandler and OidcUser
symfony-splitter pushed a commit to symfony/security-bundle that referenced this pull request Apr 14, 2023
@fabpot
feature #48272 [Security] Add OidcUserInfoTokenHandler and OidcUser (…
8029820 
…vincentchalamon)

This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[Security] Add OidcUserInfoTokenHandler and OidcUser

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | N/A
| License       | MIT
| Doc PR        | symfony/symfony-docs#17463

Hi,

This PR aims to complete [the previous one](symfony/symfony#46428) from `@Spomky` with an AccessTokenHandler ready-to-use with an OIDC server (Keycloak, Auth0).

## TODO

- [x] Rebase from 6.3
- [x] Rebase from #48285
- [x] Rebase from #48594
- [x] Write doc (symfony/symfony-docs#17463)
- [x] Add TokenHandlerFactory
- [x] Add ServiceTokenHandlerFactory for BC layer
- [x] Add OidcUserInfoTokenHandlerFactory
- [x] Add OidcTokenHandlerFactory (using web-token/jwt-*)
- [x] Implement OidcUser to keep user claims from OIDC server
- [x] Update doc PR about claims usage in a custom UserProvider
- [x] ~Update doc PR about OidcUserProvider usage~ (abandonned)

## Usage

```yaml
# usage with a custom client
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc_user_info:
                        client: oidc.client
```
```yaml
# usage with generic HttpClient
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc_user_info:
                        claim: email
                        client:
                            base_uri: https://www.example.com/realms/demo/protocol/openid-connect/userinfo
```
```yaml
# usage with token decode (no call to OIDC server)
security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler:
                    oidc:
                        signature:
                            # Algorithm used to sign the JWS
                            algorithm: 'HS256'
                            # A JSON-encoded JWK
                            key: '{"kty":"...","k":"..."}'
```
```php
# usage with a custom UserProvider
class CustomUserProvider implements UserProviderInterface
{
    public function loadUserByIdentifier(string $identifier, array $claims = []): UserInterface
    {
        // do some magic
    }
}
```

Commits
-------

99a35f0fc3 [Security] Add OidcUserInfoTokenHandler and OidcUser
@OskarStark OskarStark removed the Waiting Code Merge Docs for features pending to be merged label Apr 14, 2023
OskarStark
OskarStark reviewed Apr 14, 2023
View reviewed changes
@OskarStark OskarStark requested a review from wouterj April 14, 2023 07:41
@OskarStark OskarStark removed this from the next milestone Apr 14, 2023
@OskarStark OskarStark added this to the 6.3 milestone Apr 14, 2023
@chalasr
Copy link
Member

chalasr commented Apr 25, 2023

This should probably mention the additional dependency that is needed to use the OIDCTokenHandler (applies to the blogpost too)

chalasr
chalasr reviewed Apr 25, 2023
View reviewed changes
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of documenting the feature. Some suggestions

@javiereguiluz javiereguiluz merged commit 9193745 into symfony:6.3 May 10, 2023
@javiereguiluz
Copy link
Member

Vincent, this was a really great contribution. Thanks a lot ... and thanks to reviewers too!

@vincentchalamon vincentchalamon deleted the feat/security/access-token-handler-factory branch May 10, 2023 16:06
@cifren
Copy link

cifren commented Jun 7, 2023 •
edited
Loading

Hi, I am not sure where I should put that, but it seems that the documentation is not up to date.

I will do the list here

  • The property 'signature' gives an error that it does not exist
  • The properties 'audience', 'claim' are required but the documentation says option, 'issuers' is not even mentionned but required.
  • The property 'algorithm' says it can't accept anything that is not 'ES', but the documentation does not say anything about it and even shows an example with 'HS256'

I haven't got on this because I am going to the change my code for a handler, so I might miss some other stuff.

Thank you for the improvement though.

See https://github.com/symfony/symfony/blob/v6.3.0/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php

@vincentchalamon
Copy link
Contributor Author

@cifren see #18354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalasr chalasr chalasr approved these changes

@wouterj wouterj Awaiting requested review from wouterj

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
Security Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
8 participants
@vincentchalamon @chalasr @javiereguiluz @cifren @OskarStark @wouterj @xabbuh @carsonbot