[Security] Access Token Authenticator by Spomky · Pull Request #46428 · symfony/symfony

Spomky · 2022-05-21T16:09:04Z

Q	A
Branch?	6.2
Bug fix?	yes
New feature?	yes
Deprecations?	no
Tickets	Fix #45844
License	MIT
Doc PR	symfony/symfony-docs#16819

Hi,

This PR aims at fixing #45844.
It adds a new authenticator that is able to fetch a token in the request header and retrieve the associated user identifier.

The authenticator delegates the token loading to a handler. This handler could manage opaque tokens (random strings stored in a database) or self-contained tokens such as JWT, Paseto, SAML...

RFC6750, section 2: Authenticated Requests
- Token in the request header (section 2.1)
- Token in the query string (section 2.2)
- Token in the request body (section 2.3)
RFC6750, section 3: The WWW-Authenticate Response Header Field
- RFC6750, section 3.1: Error Codes
Documentation: see [Security] Access Tokens symfony-docs#16819
Tests

Firewall Configuration

This PR adds a new authenticator that covers the RFC6750: access_token.
Also, it adds the possibility to extract the token from anywhere in the request.

Basic Configuration

security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler: access_token.access_token_handler

Complete Configuration

security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                user_provider: 'dedicate_user_provider_for_this_firewall'
                success_handler: 'custom_success_handler'
                failure_handler: 'custom_failure_handler'
                token_handler: access_token.access_token_handler
                token_extractors:
                    - 'security.access_token_extractor.query_string'
                    - 'security.access_token_extractor.request_body'
                    - 'security.access_token_extractor.header'
                    - 'custom_access_token_extractor'

Token Handler

This authenticator relies on a Token Handler. Its responsability is to

load the token
check the token (revocation, expiration time, digital signature...)
return the user ID associated to it

Tokens could be of any kind: opaque strings or self-contained tokens such as JWT, Paseto, SAML2...

Example: from a repository

<?php

namespace App\Security;

use App\Repository\AccessTokenRepository;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Http\Authenticator\AccessTokenHandler as AccessTokenHandlerAliasInterface;

class AccessTokenHandler implements AccessTokenHandlerAliasInterface
{
    public function __construct(private readonly AccessTokenRepository $repository)
    {
    }

    public function getUserIdentifierFrom(string $token): string
    {
        $accessToken = $this->repository->findOneByValue($token);
        if ($accessToken === null || !$accessToken->isValid()) {
            throw new BadCredentialsException('Invalid credentials.');
        }

        return $accessToken->getUserId();
    }
}

Example: from a JWT

<?php

namespace App\Security;

use App\Security\JWTLoader;
use App\Security\JWTValidator;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Http\Authenticator\AccessTokenHandler as AccessTokenHandlerAliasInterface;

class AccessTokenHandler implements AccessTokenHandlerAliasInterface
{
    public function __construct(
        private readonly JWTLoader $loader,
        private readonly JWTValidator $validator
    )
    {
    }

    public function getUserIdentifierFrom(string $token): string
    {
        try {
            $token = $this->loader->loadJWT($token);
            $this->validator->validate($token);

            return $token->getClaim('sub');
        } catch (\Throwable $e) {
            throw new BadCredentialsException('Invalid credentials.', $e->getCode, $e);
        }
    }
}

chalasr · 2022-05-21T17:20:24Z

I like this approach.

Spomky · 2022-05-21T19:48:49Z

Static analysis / Psalm (pull_request): does not seem relevant
continuous-integration/appveyor/pr: the error seems to be related to a timeout issue
PHPUnit / Tests (8.2) (pull_request): the error reported here: https://github.com/symfony/symfony/runs/6538975401?check_suite_focus=true#step:8:2110 is related to another (unchanged) component. Should I fix it?
PHPUnit / Tests (8.1, low-deps) (pull_request): Error: Class "Symfony\Component\Security\Http\Authenticator\AccessTokenAuthenticator" not found I don't understand this issue. The class is present in the PR

wouterj · 2022-05-22T11:47:09Z

Thanks for checking all tests! (btw, I'll look at the PR somewhere after 6.1 is released)

PHPUnit / Tests (8.2) (pull_request): the error reported here: https://github.com/symfony/symfony/runs/6538975401?check_suite_focus=true#step:8:2110 is related to another (unchanged) component. Should I fix it?

We'll ignore it for this PR (but if you want, a separate PR with a fix is always welcome 😉 )

PHPUnit / Tests (8.1, low-deps) (pull_request): Error: Class "Symfony\Component\Security\Http\Authenticator\AccessTokenAuthenticator" not found I don't understand this issue. The class is present in the PR

See https://symfony.com/doc/current/contributing/code/pull_requests.html#automated-tests You must update the minimum versions in e.g. the SecurityBundle.

src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php

...e/SecurityBundle/Tests/DependencyInjection/Security/Factory/HeaderAccessTokenFactoryTest.php

...dle/SecurityBundle/Tests/DependencyInjection/Security/Factory/BodyAccessTokenFactoryTest.php

...le/SecurityBundle/Tests/DependencyInjection/Security/Factory/QueryAccessTokenFactoryTest.php

...dle/SecurityBundle/Tests/DependencyInjection/Security/Factory/BodyAccessTokenFactoryTest.php

...le/SecurityBundle/Tests/DependencyInjection/Security/Factory/QueryAccessTokenFactoryTest.php

src/Symfony/Component/Security/Http/Authenticator/AbstractAccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/BodyAccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/QueryAccessTokenAuthenticator.php

Spomky · 2022-05-23T19:28:46Z

Hi,

Many thanks for your comments.
I updated the authenticator that is now based on token "extractors".

security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler: access_token.access_token_handler
#                token_extractors:
#                    header: 'security.access_token_extractor.header' # This extrator is set by default

You can add any extractor you want. Three already exist:

security.access_token_extractor.header
security.access_token_extractor.query_string
security.access_token_extractor.request_body

You can create as many extractors you need. They shall implement Symfony\Component\Security\Http\Authenticator\AccessToken\AccessTokenExtractorInterface and the method extractToken.

Other parameters are also present:

security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler: access_token.access_token_handler
                success_handler: access_token.success_handler
                failure_handler: access_token.failure_handler
                user_provider: 'another.user_provider'
                token_extractors:
                    - 'security.access_token_extractor.header'
                    - 'security.access_token_extractor.query_string'
                    - 'security.access_token_extractor.request_body'
                    - 'App\Security\CustomExtractor'

I have not created Exceptions. I revised my mind as the existing ones seem sufficient.
WDYT?

Best regards.

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AccessTokenFactory.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenHandlerInterface.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/BodyAccessTokenExtractor.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/HeaderAccessTokenExtractor.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/QueryAccessTokenExtractor.php

derrabus

Thank you for your PR. In this review, I focussed mainly on code style and best practises. I'll do a more in-depth review later.

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/HeaderAccessTokenExtractor.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/QueryAccessTokenExtractor.php

...ymfony/Component/Security/Http/Authenticator/Passport/Credentials/AccessTokenCredentials.php

src/Symfony/Component/Security/Http/Tests/Authenticator/BodyAccessTokenAuthenticatorTest.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php

src/Symfony/Component/Security/Core/Authentication/Token/AccessToken.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/BodyAccessTokenExtractor.php

...ny/Component/Security/Http/Authenticator/AccessToken/DefaultAuthenticationFailureHandler.php

wouterj

Very cool. Thanks for taking on this adventure @Spomky! (and wow, never knew this would be your first code contribution to Symfony core)

I've reviewed the component part for now. I really like how this PR is strict enough to completely follow the RFC, but yet keep enough open to be useful for many custom-build API authenticators.

src/Symfony/Component/Security/Core/Authentication/Token/AccessToken.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenExtractorInterface.php

...ny/Component/Security/Http/Authenticator/AccessToken/DefaultAuthenticationFailureHandler.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/BodyAccessTokenExtractor.php

...ymfony/Component/Security/Http/Authenticator/Passport/Credentials/AccessTokenCredentials.php

src/Symfony/Component/Security/Http/Authenticator/AccessToken/HeaderAccessTokenExtractor.php

Spomky · 2022-07-16T14:55:51Z

Hi all,

Would you mind to review the changes I made and if the comments you raised have correctly been addressed?
Let me know if you see other modifications I should consider.

Many thanks.
Regards.

wouterj · 2022-07-16T16:23:19Z

@Spomky if you click on the "load more ...", you'll see a review from me. I guess GitHub immediately hid it, so it got lost 🙂

src/Symfony/Component/Security/Http/AccessToken/HeaderAccessTokenExtractor.php

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AccessTokenFactory.php

src/Symfony/Component/Security/Http/AccessToken/AccessTokenHandlerInterface.php

src/Symfony/Component/Security/Http/AccessToken/FormEncodedBodyExtractor.php

src/Symfony/Component/Security/Http/AccessToken/QueryAccessTokenExtractor.php

src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php

...ymfony/Component/Security/Http/Authenticator/Passport/Credentials/AccessTokenCredentials.php

src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php

lyrixx · 2022-08-01T16:31:46Z

Super feature 👍🏼 Thanks for the work 👏🏼 . I let some comments

Spomky · 2022-08-01T20:48:44Z

Many thanks for the review and comments.
I think I addressed all of them (including those hidden by github).
Let me know if you have any others.

dunglas

🤩

lyrixx

Some comments about the CS, but I'm OK with the global design 👍🏼

lyrixx · 2022-08-02T08:33:56Z

src/Symfony/Component/Security/Http/AccessToken/FormEncodedBodyExtractor.php

+    public function __construct(private readonly string $parameter = 'access_token')
+    {
+    }


You don't always use the same CS. This one ⬆️ VS

public function __construct( private readonly iterable $accessTokenExtractors, ) { }

In my applications (at work), I use the latest: more scalable, and reduce the diff when adding a new param.

However, I don't know what's the standard in Symfony (cc @nicolas-grekas @fabpot)

I ran php-cs-fixer but many files got updated. I just focussed on the one from this PR but the gaps you spotted were not found by the tool.
Anyway, I updated them to be consistent.

src/Symfony/Component/Security/Http/AccessToken/HeaderAccessTokenExtractor.php

src/Symfony/Component/Security/Http/AccessToken/RFC6750AuthenticationFailureHandler.php

wouterj

Hi there! I finally got the chance to review this a bit more in-depth and I'm really liking the feature :)

I've pushed 2 commits to your branch that fix the last 2 big remaining comments. It would be nice if you could squash all your commits into one and after that, it's ready to be merged in my opinion.

Thanks a lot for all the work!

src/Symfony/Component/Security/Http/AccessToken/FormEncodedBodyExtractor.php

src/Symfony/Component/Security/Http/AccessToken/QueryAccessTokenExtractor.php

Spomky · 2022-08-09T17:06:40Z

Hi there! I finally got the chance to review this a bit more in-depth and I'm really liking the feature :)

I've pushed 2 commits to your branch that fix the last 2 big remaining comments. It would be nice if you could squash all your commits into one and after that, it's ready to be merged in my opinion.

Thanks a lot for all the work!

Excellent! Many thanks to you and all the other reviewers.
I edited the few things you spotted and squashed the PR.
This is a big step for me and I really hope it will help many developers to integrate token-based authentication.

src/Symfony/Component/Security/Http/AccessToken/AccessTokenHandlerInterface.php

src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php

chalasr · 2022-08-10T10:19:58Z

Thank you @Spomky.

Spomky · 2022-08-11T11:10:03Z

So cool! Many thanks for merging this PR.

derrabus · 2022-08-11T18:16:49Z

Awesome! 🚀

This PR was merged into the 6.2 branch. Discussion ---------- [Security] Access Tokens Documentation page related to the PR symfony/symfony#46428 Commits ------- f3f47ad Update docs for 6.2.0-RC1 changes to TokenHandlerInterface b65c136 Finish the docs for the new Access token authenticator 600bb80 Access Token Documentation

vincentchalamon · 2022-11-28T14:21:47Z

@Spomky Question regarding the configuration of this feature: why adding user_provider option, and not rely on the firewall user provider?

Spomky · 2022-11-29T06:13:20Z

@vincentchalamon this allows setting a specific user provider for this firewall. If not set, the default user provider is used.

chalasr · 2022-11-29T11:22:52Z

See #48385

…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

…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

Spomky requested review from chalasr and wouterj as code owners May 21, 2022 16:09

carsonbot added Status: Needs Review Bug Feature Security labels May 21, 2022

carsonbot added this to the 6.2 milestone May 21, 2022

Spomky marked this pull request as draft May 21, 2022 16:09

Spomky mentioned this pull request May 21, 2022

[Security] Add Bearer Authenticator #45844

Closed

Spomky marked this pull request as ready for review May 21, 2022 19:12

Spomky marked this pull request as draft May 22, 2022 07:40

chalasr reviewed May 22, 2022

View reviewed changes

src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php Outdated Show resolved Hide resolved

Spomky marked this pull request as ready for review May 22, 2022 16:19

Spomky mentioned this pull request May 22, 2022

[Security] Access Tokens symfony/symfony-docs#16819

Merged

vincentchalamon suggested changes May 22, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels May 22, 2022

vincentchalamon mentioned this pull request May 24, 2022

[Security] Implement HttpBearerAuthenticator #46429

Closed

4 tasks

stof requested changes May 31, 2022

View reviewed changes

derrabus requested changes Jun 1, 2022

View reviewed changes

derrabus requested changes Jun 6, 2022

View reviewed changes

src/Symfony/Component/Security/Http/Authenticator/AccessToken/AccessTokenAuthenticator.php Outdated Show resolved Hide resolved

dunglas requested changes Jun 14, 2022

View reviewed changes

wouterj reviewed Jun 14, 2022

View reviewed changes

vincentchalamon mentioned this pull request Jun 16, 2022

Add Keycloak authentication api-platform/demo#265

Merged

4 tasks

chalasr reviewed Jul 28, 2022

View reviewed changes

src/Symfony/Component/Security/Http/AccessToken/HeaderAccessTokenExtractor.php Outdated Show resolved Hide resolved

lyrixx reviewed Aug 1, 2022

View reviewed changes

dunglas approved these changes Aug 1, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Aug 1, 2022

lyrixx reviewed Aug 2, 2022

< 4E22 span class="Button-label">View reviewed changes

wouterj approved these changes Aug 9, 2022

View reviewed changes

src/Symfony/Component/Security/Http/AccessToken/FormEncodedBodyExtractor.php Outdated Show resolved Hide resolved

src/Symfony/Component/Security/Http/AccessToken/QueryAccessTokenExtractor.php Outdated Show resolved Hide resolved

wouterj BD9E added the Ready label Aug 9, 2022

94noni reviewed Aug 9, 2022

View reviewed changes

src/Symfony/Component/Security/Http/AccessToken/AccessTokenHandlerInterface.php Outdated Show resolved Hide resolved

94noni reviewed Aug 9, 2022

View reviewed changes

src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php Show resolved Hide resolved

94noni reviewed Aug 9, 2022

View reviewed changes

src/Symfony/Component/Security/Http/Authenticator/AccessTokenAuthenticator.php Outdated Show resolved Hide resolved

[Security] Access Token Authenticator

e5873e8

chalasr approved these changes Aug 10, 2022

View reviewed changes

chalasr merged commit dfcf900 into symfony:6.2 Aug 10, 2022

Spomky deleted the features/access-token-authenticator branch August 11, 2022 11:10

Spomky mentioned this pull request Aug 27, 2022

SF6.2 Access Token Support lexik/LexikJWTAuthenticationBundle#1062

Open

4 tasks

fabpot mentioned this pull request Oct 24, 2022

Release v6.2.0-BETA1 #47972

Merged

vincentchalamon mentioned this pull request Nov 21, 2022

[Security] Add OidcUserInfoTokenHandler and OidcUser #48272

Merged

11 tasks

Uh oh!

Conversation

Uh oh!

Firewall Configuration

Basic Configuration

Complete Configuration

Token Handler

Example: from a repository

Example: from a JWT

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!