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] Access Token Authenticator#46428

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
chalasr merged 1 commit intosymfony:6.2fromSpomky:features/access-token-authenticator
Aug 10, 2022
Merged

[Security] Access Token Authenticator#46428

chalasr merged 1 commit intosymfony:6.2fromSpomky:features/access-token-authenticator
Aug 10, 2022

Conversation

Spomky
Copy link
Contributor

@SpomkySpomky commentedMay 21, 2022•
edited
Loading

QA
Branch?6.2
Bug fix?yes
New feature?yes
Deprecations?no
TicketsFix#45844
LicenseMIT
Doc PRsymfony/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...

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_handlertoken_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

<?phpnamespaceApp\Security;useApp\Repository\AccessTokenRepository;useSymfony\Component\Security\Core\Exception\BadCredentialsException;useSymfony\Component\Security\Http\Authenticator\AccessTokenHandlerasAccessTokenHandlerAliasInterface;class AccessTokenHandlerimplements AccessTokenHandlerAliasInterface{publicfunction__construct(privatereadonlyAccessTokenRepository$repository)    {    }publicfunctiongetUserIdentifierFrom(string$token):string    {$accessToken =$this->repository->findOneByValue($token);if ($accessToken ===null || !$accessToken->isValid()) {thrownewBadCredentialsException('Invalid credentials.');        }return$accessToken->getUserId();    }}

Example: from a JWT

<?phpnamespaceApp\Security;useApp\Security\JWTLoader;useApp\Security\JWTValidator;useSymfony\Component\Security\Core\Exception\BadCredentialsException;useSymfony\Component\Security\Http\Authenticator\AccessTokenHandlerasAccessTokenHandlerAliasInterface;class AccessTokenHandlerimplements AccessTokenHandlerAliasInterface{publicfunction__construct(privatereadonlyJWTLoader$loader,privatereadonlyJWTValidator$validator    )    {    }publicfunctiongetUserIdentifierFrom(string$token):string    {try {$token =$this->loader->loadJWT($token);$this->validator->validate($token);return$token->getClaim('sub');        }catch (\Throwable$e) {thrownewBadCredentialsException('Invalid credentials.',$e->getCode,$e);        }    }}

chalasr, Jeroeny, welcoMattic, and mleczakm reacted with thumbs up emojiLustmored, welcoMattic, mleczakm, haroldiedema, and jschaedl reacted with heart emojiapfelbox, welcoMattic, mleczakm, and jschaedl reacted with rocket emoji
@chalasr
Copy link
Member

I like this approach.

Spomky reacted with heart emoji

@SpomkySpomky marked this pull request as ready for reviewMay 21, 2022 19:12
@Spomky
Copy link
ContributorAuthor

  • 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

@SpomkySpomky marked this pull request as draftMay 22, 2022 07:40
@wouterj
Copy link
Member

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

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

@SpomkySpomky marked this pull request as ready for reviewMay 22, 2022 16:19
@Spomky
Copy link
ContributorAuthor

Spomky commentedMay 23, 2022•
edited
Loading

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 implementSymfony\Component\Security\Http\Authenticator\AccessToken\AccessTokenExtractorInterface and the methodextractToken.

Other parameters are also present:

security:firewalls:main:pattern:^/access_token:token_handler:access_token.access_token_handlersuccess_handler:access_token.success_handlerfailure_handler:access_token.failure_handleruser_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.

Copy link
Member

@derrabusderrabus left a comment•
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member

@wouterjwouterj left a comment

Choose a reason for hiding this comment

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

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.

@Spomky
Copy link
ContributorAuthor

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
Copy link
Member

@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 🙂

@lyrixx
Copy link
Member

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

@Spomky
Copy link
ContributorAuthor

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.

Copy link
Member

@dunglasdunglas left a comment

Choose a reason for hiding this comment

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

🤩

Copy link
Member

@lyrixxlyrixx left a comment

Choose a reason for hiding this comment

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

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

Comment on lines 34 to 33
public function __construct(private readonly string $parameter = 'access_token')
{
}
Copy link
Member

Choose a reason for hiding this comment

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

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

publicfunction__construct(privatereadonlyiterable$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)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

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.

Copy link
Member

@wouterjwouterj left a comment

Choose a reason for hiding this comment

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

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!

Spomky reacted with hooray emojiSpomky reacted with rocket emoji
@Spomky
Copy link
ContributorAuthor

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.

lyrixx, yceruto, and chalasr reacted with heart emoji

@wouterjwouterj added the Ready labelAug 9, 2022
@chalasr
Copy link
Member

Thank you@Spomky.

vincentchalamon, 94noni, a-menshchikov, lyrixx, welcoMattic, wouterj, yceruto, walva, and derrabus reacted with hooray emoji

@chalasrchalasr merged commitdfcf900 intosymfony:6.2Aug 10, 2022
@Spomky
Copy link
ContributorAuthor

So cool! Many thanks for merging this PR.

chalasr, apfelbox, fd6130, and haroldiedema reacted with heart emoji

@SpomkySpomky deleted the features/access-token-authenticator branchAugust 11, 2022 11:10
@derrabus
Copy link
Member

Awesome! 🚀

@fabpotfabpot mentioned this pull requestOct 24, 2022
wouterj added a commit to symfony/symfony-docs that referenced this pull requestNov 26, 2022
This PR was merged into the 6.2 branch.Discussion----------[Security] Access TokensDocumentation page related to the PRsymfony/symfony#46428Commits-------f3f47ad Update docs for 6.2.0-RC1 changes to TokenHandlerInterfaceb65c136 Finish the docs for the new Access token authenticator600bb80 Access Token Documentation
@vincentchalamon
Copy link
Contributor

@Spomky Question regarding the configuration of this feature: why addinguser_provider option, and not rely on the firewalluser provider?

@Spomky
Copy link
ContributorAuthor

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

@chalasr
Copy link
Member

See#48385

fabpot added a commit that referenced this pull requestApr 14, 2023
…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#17463Hi,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 clientsecurity:    firewalls:        main:            pattern: ^/            access_token:                token_handler:                    oidc_user_info:                        client: oidc.client``````yaml# usage with generic HttpClientsecurity:    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 UserProviderclass 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 requestApr 14, 2023
…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#17463Hi,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 clientsecurity:    firewalls:        main:            pattern: ^/            access_token:                token_handler:                    oidc_user_info:                        client: oidc.client``````yaml# usage with generic HttpClientsecurity:    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 UserProviderclass CustomUserProvider implements UserProviderInterface{    public function loadUserByIdentifier(string $identifier, array $claims = []): UserInterface    {        // do some magic    }}```Commits-------99a35f0fc3 [Security] Add OidcUserInfoTokenHandler and OidcUser
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@vincentchalamonvincentchalamonvincentchalamon requested changes

@lyrixxlyrixxlyrixx left review comments

@stofstofstof requested changes

@94noni94noni94noni left review comments

@derrabusderrabusderrabus requested changes

@dunglasdunglasdunglas approved these changes

@wouterjwouterjwouterj approved these changes

@chalasrchalasrchalasr approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
6.2
Development

Successfully merging this pull request may close these issues.

[Security] Add Bearer Authenticator
10 participants
@Spomky@chalasr@wouterj@lyrixx@derrabus@vincentchalamon@dunglas@stof@94noni@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp