Skip to content
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

Wrong definition of AccessTokenBuilder #1209

Closed
CvekCoding opened this issue Feb 27, 2024 · 2 comments
Closed

Wrong definition of AccessTokenBuilder #1209

CvekCoding opened this issue Feb 27, 2024 · 2 comments

Comments

@CvekCoding
Copy link

Hello.

I have encountered a problem, and it seems that it is related to an incorrect definition in the file Resources/config/web_token_issuance.xml :

        <service id="lexik_jwt_authentication.access_token_builder" class="Lexik\Bundle\JWTAuthenticationBundle\Services\WebToken\AccessTokenBuilder" public="false">
            <argument type="service" id="Symfony\Contracts\EventDispatcher\EventDispatcherInterface" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWSBuilderFactory" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWEBuilderFactory" on-invalid="null" />
            <argument /> <!-- Signature algorithm -->
            <argument /> <!-- Signature key -->
            <argument on-invalid="null" /> <!-- Key encryption algorithm -->
            <argument on-invalid="null" /> <!-- Content encryption algorithm -->
            <argument on-invalid="null" /> <!-- Encryption key -->
        </service>

You have the 5th, 6th and 7th arguments, which must be nulls in case we don't use encryption. But it doesn't seem that Symfony works that way - for text arguments, on-invalid="null" directive just doesn't work, the value of these arguments is an empty string ''. So the service Services/WebToken/AccessTokenBuilder.php receives these arguments and does not work correctly.

As a solution, I would suggest in the file DependencyInjection/LexikJWTAuthenticationExtension.php explicitly prescribe values in these arguments for the case when we do not encrypt the keys. Like this:

            if ($config['access_token_issuance']['encryption']['enabled'] === true) {
                $accessTokenBuilderDefinition
                    ->replaceArgument(5, $config['access_token_issuance']['encryption']['key_encryption_algorithm'])
                    ->replaceArgument(6, $config['access_token_issuance']['encryption']['content_encryption_algorithm'])
                    ->replaceArgument(7, $config['access_token_issuance']['encryption']['key'])
                ;
            } else {
                $accessTokenBuilderDefinition
                    ->replaceArgument(5, null)
                    ->replaceArgument(6, null)
                    ->replaceArgument(7, null);
            }

BTW the same is true for the service Lexik\Bundle\JWTAuthenticationBundle\Services\WebToken\AccessTokenLoader. It has nullable arguments but receives empty arrays instead of nulls (see last arguments):

        <service id="lexik_jwt_authentication.access_token_loader" class="Lexik\Bundle\JWTAuthenticationBundle\Services\WebToken\AccessTokenLoader" public="false">
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWSLoaderFactory" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWELoaderFactory" on-invalid="null" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\ClaimCheckerManagerFactory" />
            <argument type="collection" /> <!-- Claim checkers -->
            <argument type="collection"/> <!-- JWS header checkers -->
            <argument type="collection"/> <!-- Mandatory claims -->
            <argument type="collection" /> <!-- Allowed signature algorithms -->
            <argument /> <!-- Signature keyset -->
            <argument on-invalid="null" /> <!-- Continue on decryption failure -->
            <argument type="collection" on-invalid="null" /> <!-- JWE header checkers -->
            <argument type="collection" on-invalid="null" /> <!-- Allowed key encryption algorithms -->
            <argument type="collection" on-invalid="null" /> <!-- Allowed content encryption algorithms -->
            <argument on-invalid="null" /> <!-- Encryption keyset -->
        </service>
@CvekCoding
Copy link
Author

CvekCoding commented Feb 28, 2024

I've investigated the problem a little deeper, and I think I've found a better solution. Firstly, you have incorrectly described the lexik_jwt_authentication.access_token_builder definition. If the default string value should be null, then it is necessary to pass null as a node value. And the collection, as I understand it, cannot be equated to null in definition at all.
Therefore, I propose the following changes:

Resources/config/web_token_issuance.xml
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

    <services>
        <service id="lexik_jwt_authentication.access_token_builder" class="Lexik\Bundle\JWTAuthenticationBundle\Services\WebToken\AccessTokenBuilder" public="false">
            <argument type="service" id="Symfony\Contracts\EventDispatcher\EventDispatcherInterface" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWSBuilderFactory" />
            <argument type="service" id="Jose\Bundle\JoseFramework\Services\JWEBuilderFactory" on-invalid="null" />
            <argument /> <!-- Signature algorithm -->
            <argument /> <!-- Signature key -->
            <argument>null</argument> <!-- Key encryption algorithm -->
            <argument>null</argument> <!-- Content encryption algorithm -->
            <argument>null</argument> <!-- Encryption key -->
        </service>
    </services>
</container>

and check emptiness of collections:

\Lexik\Bundle\JWTAuthenticationBundle\Services\WebToken\AccessTokenLoader::__construct

    public function __construct(
        JWSLoaderFactory            $jwsLoaderFactory,
        ?JWELoaderFactory           $jweLoaderFactory,
        ClaimCheckerManagerFactory  $claimCheckerManagerFactory,
        array                       $claimChecker,
        array                       $jwsHeaderChecker,
        array                       $mandatoryClaims,
        array                       $signatureAlgorithms,
        string                      $signatureKeyset,
        ?bool                       $continueOnDecryptionFailure,
        ?array                      $jweHeaderChecker,
        ?array                      $keyEncryptionAlgorithms,
        ?array                      $contentEncryptionAlgorithms,
        ?string                     $encryptionKeyset
    ) {
        $this->jwsLoader = $jwsLoaderFactory->create(['jws_compact'], $signatureAlgorithms, $jwsHeaderChecker);
        if ($jweLoaderFactory !== null && !empty($keyEncryptionAlgorithms) && !empty($contentEncryptionAlgorithms) && !empty($jweHeaderChecker)) {
            $this->jweLoader = $jweLoaderFactory->create(['jwe_compact'], $keyEncryptionAlgorithms, $contentEncryptionAlgorithms, [], $jweHeaderChecker);
            $this->continueOnDecryptionFailure = $continueOnDecryptionFailure;
        }
        $this->signatureKeyset = JWKSet::createFromJson($signatureKeyset);
        $this->encryptionKeyset = $encryptionKeyset ? JWKSet::createFromJson($encryptionKeyset) : null;
        $this->claimCheckerManager = $claimCheckerManagerFactory->create($claimChecker);
        $this->mandatoryClaims = $mandatoryClaims;
    }

@chalasr
Copy link
Collaborator

chalasr commented May 23, 2024

This has been fixed. Thanks

@chalasr chalasr closed this as completed May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants