Skip to content

[Security] Add Bearer Authenticator #45844

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vincentchalamon opened this issue Mar 25, 2022 · 14 comments · Fixed by #46428
Closed

[Security] Add Bearer Authenticator #45844

vincentchalamon opened this issue Mar 25, 2022 · 14 comments · Fixed by #46428

Comments

@vincentchalamon
Copy link
Contributor

vincentchalamon commented Mar 25, 2022

Description

I'm implementing an OIDC architecture in an API first project, and wanted to secure my API with an authenticator. I first took a look at bundles like hwi/oauth-bundle or knpuniversity/oauth2-client-bundle, but they're not adapted to an API as they provide bunch of features not needed in an API, and provides their own JWT generation and storage (e.g.: cookie).

I talked about it with @dunglas who shares the idea to implement a Bearer authenticator directly in the Symfony Security bundle. The idea behind this authenticator would be to retrieve the token from the Authorization header (prefixed with Bearer), and validate and decode it in a BearerToken using lcobucci/jwt.

Example

# config/packages/security.yaml
security:
    firewall:
        main:
            bearer:
                signature: '/path/to/signature/key' # Could be a file path, plaintext, or even an url
                algorithm: 'hmac.sha256'
                key: 'customUserId' # Which key from the JWT should be sent to the UserProvider (default: 'sub')

Note: I'm not happy about the signature and key configuration keys, they should be renamed

@dunglas
Copy link
Member

dunglas commented Mar 25, 2022

+1 on my side. Having a support for Bearer tokens containing a JWT (similarly to the HTTP Basic support we already have) will be useful in a lot of contexts, even when not using OIDC.
This will also enable interoperability with OIDC providers such as Keycloak and Auth0 natively.

@derrabus
Copy link
Member

Keep in mind that JWT is only one way to implement bearer tokens. Adding this feature is fine as long as we remain open to different implementations.

@vincentchalamon
Copy link
Contributor Author

Also, question discussed with @dunglas and @chalasr: which data from the JWT should be send to the UserProvider?

The sub included in the JWT should be the best and easiest option, but in some cases the develop may want to use another key (e.g.: customUserId key corresponding to the user id in the database).

To handle this case, a configuration key would allow to change which data will be sent to the UserProvider (default: sub).

@mahono
Copy link

mahono commented Mar 30, 2022

I think it's a good idea to have such an authenticator. Would it be possible to allow overriding "who" decodes the JWT? For example provide a decoder service that can be decorated so that it is possible to use another library if it is needed.

@wouterj
Copy link
Member

wouterj commented Mar 31, 2022

Yes! We are lacking a "simple" built-in token/bearer authenticator, so I would love to see this one :)

Maybe we can use the same approach as form login: provide an AbstractTokenAuthenticator class (with e.g. an abstract extractToken(Request): string method) that everyone can use for their custom API tokens and a ready to use BearerAuthenticator implementation with support for RFC 6750 (with opt-in support for form-encoded param and query param, as outlined in the spec?).
Given the spec doesn't specify the token format, we should probably have some sort of BearerDecoderInterface. (I like the idea of providing a lcobucci/jwt implementation for this in Symfony).

To handle this case, a configuration key would allow to change which data will be sent to the UserProvider (default: sub).

Agreed, something like user_identifier_claim/username_claim would be useful (all built-in authenticator have such an option IIRC).


All of this is just some ideas from me. Feel free to discuss them or ping for help (here or on Slack).

@ThomasBeauchataud
Copy link

ThomasBeauchataud commented May 9, 2022

As i really needed that kind of authenticator and didn't see any implementation proposition, here is mine :

The Authenticator

class BearerAuthenticator extends AbstractAuthenticator

private TokenExtractorInterface $tokenExtractor;
private TokenEncoderInterface $tokenEncoder;
private UserProviderInterface $userProvider;

public function __construct(TokenExtractorInterface $tokenExtractor, TokenEncoderInterface $tokenEncoder, UserProviderInterface $userProvider)
{
    $this->tokenExtractor = $tokenExtractor;
    $this->tokenEncoder = $tokenEncoder;
    $this->userProvider = $userProvider;
}


public function supports(Request $request): bool
{
    return $this->tokenExtractor->support($request);
}

public function authenticate(Request $request): Passport
{
    $encodedToken = $this->tokenExtractor->extract($request);

    if (!$encodedToken) {
        throw new AuthenticationException();
    }

    $userIdentifier = $this->tokenEncoder->decode($encodedToken);

    return new SelfValidatingPassport(new UserBadge($userIdentifier, $this->userProvider->loadUserByIdentifier(...)));
}

public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
{
    return null;
}

public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response
{
    if ($exception instanceof TokenEncoderException) {
        return new Response("invalid_token", 401);
    }

    return new Response("invalid_request", 400);
}

An exemple of a TokenEncoderInterface

@mahono feel free to create yours
@vincentchalamon here is the place where we will use the configurations signature, algorithm and key

class JWTTokenEncoder implements TokenEncoderInterface

private Configuration $config;

public function __construct(string $secret)
{
    $this->config = Configuration::forSymmetricSigner(new Sha256(), InMemory::base64Encoded($secret));
    $this->config->setValidationConstraints(new SignedWith(new Sha256(), InMemory::base64Encoded($secret)));
}


public function encode(UserInterface $user): string
{
    $token = $this->config->builder()
        ->withClaim('userIdentifier', $user->getUserIdentifier())
        ->getToken($this->config->signer(), $this->config->signingKey());

    return $token->toString();
}

public function decode(string $encodedToken): string
{
    try {

        $token = $this->config->parser()->parse($encodedToken);

        assert($token instanceof UnencryptedToken);

        $constraints = $this->config->validationConstraints();
        $this->config->validator()->assert($token, ...$constraints);

    } catch (\Exception $e) {
        throw new TokenEncoderException($e->getMessage(), 0, $e);
    }

    return $token->claims()->get('userIdentifier');
}

An exemple of a TokenExtractor

Tthe authenticator has a ChainTokenExtractor iterating over all extractors such as header, query parameter, body parameter (as suggested by @wouterj)

class AuthorizationHeaderTokenExtractor implements TokenExtractorInterface

public const HEADER_NAME = 'Authorization';
public const PREFIX = 'Bearer ';

public function extract(Request $request): ?string
{
    if (!$this->support($request)) {
        return false;
    }

    $authorizationHeader = $request->headers->get(self::HEADER_NAME);
    return str_replace(self::PREFIX, '', $authorizationHeader);
}

public function support(Request $request): bool
{
    return $request->headers->has(self::HEADER_NAME)
        && str_starts_with($request->headers->get(self::HEADER_NAME), self::PREFIX);
}

I will create a dedicated Symfony\Component\Security\Core\Authentication\Token\TokenInterface

Any first opinion about this solution ? (Obviously it's not completely finished)

@vincentchalamon
Copy link
Contributor Author

Hi @ThomasBeauchataud

Thanks for the examples. Actually, I've been working on something similar on API Platform here: api-platform/demo#265 and will open a PR on Symfony very soon.

@chalasr
Copy link
Member

chalasr commented May 21, 2022

Putting my lexik/jwt-authentication-bundle maintainer's hat here as this is one of the bundle's main features.

Let me start with some background:
In #30914, built-in jwt support was suggested (as well as other authentication methods such as 2FA). In #30914 (comment), I said I was going take a look, and I did.

Why I didn't contribute anything in this regard: JWT creation and verification implies to rely on a third party library such as lcobucci/jwt (which is the default one in the lexik bundle), as suggested by this issue.
Fact is that depending on lcobucci/jwt is a non trivial task.
No one is to blame, the maintainer has its own policy regarding BC, which includes hard BC breaks in major releases with no prior notice other than @deprecated tags. And that's perfectly fine.
But from my experience, I don't think we want to have a dependency on that library, for the simple reason that it means getting part of its maintenance burden on our shoulders. Take a look at the bundle's 2.x branch, it is currently full of ugly BC layers meant to handle the changes that happened in lcobucci/jwt's minor and major versions.
I'm maintaining this bundle on my own, it is well integrated in the Symfony ecosystem, and I think things are fine as-is. Hence I didn't push more to move it in core.

Back to the current topic: This proposes to add a way to authenticate any kind of Bearer token in a generic way, without handling the token creation part.
The idea is cool, but it comes with a lot of unknowns. I've already shared some to Vincent privately and on his PR:

The list is not exhaustive and I'm really not sold we will be able to provide something generic enough to cover everyone's use case. Moreover, the JWT library integration is a blocking concern to me. So what we could do is to let this responsibility to the developer, but then is it worth it at all?

All in all, I'm not sure we should do this. We abandoned the idea of implementing 2FA in core for the very same reason in #28868 (comment): we already have a well-maintained bundle for this. And if lexik/jwt is not enough to handle e.g. keycloack tokens (in fact it is, the token verification part can be used solely, independently from the token creation part, and it's highly customizable thanks to a bunch of events and abstractions), writing a custom authenticator is probably easier than it is for Symfony to provide a flexible, generic enough authenticator.
But if we do, we should make sure that impacted parties are in the loop.

Also

An exemple of a TokenExtractor

This looks familiar :) https://github.com/lexik/LexikJWTAuthenticationBundle/blob/2.x/TokenExtractor/TokenExtractorInterface.php

@vincentchalamon
Copy link
Contributor Author

Thanks for your constructive feedback @chalasr.

The JWT library concern and questions you provided are critical indeed, and I won't be able to handle all the use cases related to them just on my own.

The problem revealed with this issue is that even if there are a lot of bundles implementing OAuth2 & OIDC, they are all related to non-API applications: they provide a redirect instead of a proper 401/403 HTTP response in the authenticator, they require Twig + form for authentication, etc. I didn't find a clean API compatible authenticator.

I don't like the idea to let the developer write its own authenticator, as it's a security topic and could easily create a lot of security failures (especially with OAuth2 & OIDC).

@vincentchalamon
Copy link
Contributor Author

I see in the comments this issue is of interest to some people who already provide their own implementation, so here is the idea I had when I created this issue: #46429

@Spomky
Copy link
Contributor

Spomky commented May 21, 2022

Hi there,

I saw this issue few days ago and today I decided to propose a PR.
My approach in #46428 is token-type agnostic.

Somes uses cases:

  • tokens are random strings generated by an identity provider (Auth0, Okla...) and the application will hit the provider API to get the token details.
  • tokens are JWT with a non-generic claim user_id. The verification key is retrieved from env vars.
  • tokens are JWT and the identifier is the standard sub claim. The verification keys come from a JWKS loaded from a distant URL
  • token are SAML2 (XML format) from a company SSO. The SSO will exchange the token and give user details from the LDAP server.

For all this use cases, the approach is the same: the authenticator I propose just need a token handler. This handler should be able to load that token, perform the verification steps and return the corresponding user identifier
The application configuration is quite simple:

security:
    firewalls:
        main:
            pattern: ^/
            access_token:
                token_handler: 'App\Security\AccessTokenHandler'

With the listener system and the specific AccessTokenCredentials class in the passport, it could also be possible to check if the token is not blacklisted.

EDIT: Example of a fictive token handler.

<?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) {
            throw new BadCredentialsException('Invalid credentials.');
        }

        return $accessToken->getUserId();
    }
}

@chalasr
Copy link
Member

chalasr commented May 22, 2022

Thanks for your constructive feedback @chalasr.

Thank you for rising this discussion.

I didn't find a clean API compatible authenticator.

I support your wiling to improve the situation, I think we can.

I don't like the idea to let the developer write its own authenticator, as it's a security topic and could easily create a lot of security failures (especially with OAuth2 & OIDC).

I see your point. But I would arg that anyway, most of the security concerns that come with these protocols are out of the scope of this feature as it does not aim to know about which flow is used, how tokens transit, where they are stored nor what data they contain. So actually, such an integration would reduce mistakes at the signature verification level only.
Which rises the concern pointed out by Alexander above: Is it realistic to attempt supporting any kind of Bearer token as per https://datatracker.ietf.org/doc/html/rfc6750?

Even just supporting JWT from unknown sources is not going to be easy given the vagueness of the JWT spec.
See e.g. lcobucci/jwt#229, it's the kind of issues we will have to deal with as soon as we claim to support authenticating any JWT created elsewhere.

That's why I think we would better provide a good extension point rather than trying to cover such a broad scope, so that one can build on it by implementing their own verification logic (in core, api-platform and auth-related community bundles/recipes).

@vincentchalamon
Copy link
Contributor Author

That's why I think we would better provide a good extension point rather than trying to cover such a broad scope, so that one can build on it by implementing their own verification logic (in core, api-platform and auth-related community bundles/recipes).

I agree. I like the approach from @Spomky with the token handler. Even if it requires a bit of code from the developer comparing to my approach, it looks like a good compromise between security failures and JWT concerns.

If we agree with his approach, I would be happy to help on his PR :-) (I just think it misses a bit of RFC 6750)

@Spomky
Copy link
Contributor

Spomky commented May 22, 2022

If we agree with his approach, I would be happy to help on his PR :-) (I just think it misses a bit of RFC 6750)

Many thanks @vincentchalamon. I think this PR is mature enough now so let me know if you see anything I missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants