cakephp / authentication

Authentication plugin for CakePHP. Can also be used in PSR7 based applications.
MIT License
116 stars 100 forks source link

JWKS is broken with firebase/php-jwt >= 6.0 #530

Closed cnizzardini closed 2 years ago

cnizzardini commented 2 years ago

When a user installs firebase via composer req firebase/php-jwt they will get 6.0 or higher. This is not compatible with JwtAuthenticator when using JWKS.

1) Authentication\Test\TestCase\Authenticator\JwtAuthenticatorTest::testGetPayloadRS256
TypeError: Key material must be a string, resource, or OpenSSLAsymmetricKey

Reproduce:

  1. In this project simply run composer req --dev firebase/php-jwt:^6.0
  2. Run composer test

Problem lies here: https://github.com/cakephp/authentication/blob/2.x/src/Authenticator/JwtAuthenticator.php#L181

Solution is simply this:

$keyMaterial = new Key($keyMaterial->getMaterial(), $keyAlgorithms[$k]);

But that is not compatible with ^5.5

Something like this works but IDK how to test this or if its a good practice...

                if ($keyMaterial instanceof Key) {
                    $keyMaterial = new Key($keyMaterial->getKeyMaterial(), $keyAlgorithms[$k]);
                } else {
                    $keyMaterial = new Key($keyMaterial, $keyAlgorithms[$k]);
                }

I guess at the least documentation can get updated, but that is easy to miss...

cnizzardini commented 2 years ago

Docs currently don't specify ^5.5:

You need to add the lib firebase/php-jwt v5.5 or above to your app to use the JwtAuthenticator.

I also see that there is an open PR to bump firebase/php-jwt to 6.0 and it will be a breaking change...

ADmad commented 2 years ago

PR welcome to update the docs for the current version of the plugin.

cnizzardini commented 2 years ago

@ADmad I can do that, what about the conditionally on $keyMaterial? Too hacky?

ADmad commented 2 years ago

As commented in #507 call to JWT::decode() needs to be updated too for v6 of php-jwt.