TheNetworg/oauth2-azure

Bug in Firebase/php-jwt

Summersyard opened this issue · 14 comments

Summersyard commented

Our implementation base on the current versions (using Firebase/php-jwt 5.x) works fine. Composer does not update to the latest 6.x as the latest change (added supporting version 6.x) does not have a version tag.
When we manually update to Firebase/php-jwt 6.x we get this error:
SSO error php-jwt-6

Unfortunately Firebase/php-jwt 5.x has a major security issue so updating is very much recommended. Although called by oauth-azure the problem might be in the firebase lib. Thus, I will post this issue there as well.

Including the support of the latest firebase lib version should maybe delayed until this issue is fixed.

hajekj commented

Could you please test with the current version in master? I have merged a bunch of hanging PRs, primarily #159 should fix it. I aim to release a new version of the library on Friday.

Summersyard commented

The change will be deployed on a test env in the next hours. Then I can give feedback.

hajekj commented

This seems to be related to the configuration, have you configured scopes like shown here: https://github.com/TheNetworg/oauth2-azure#authorization-code-flow?

Summersyard commented

When switching to 2.0 some weeks ago we found a scope definition is mandatory and specified 'scope' => 'openid profile email' which worked then. Now we are deploying the latest version with 'scopes' => ['openid','profile','email'] and will come back with the result soon.

  • 👍1
spackmat commented

I took a look into the code. The culprit seems to be the method getJwtVerificationKeys (line 336) in https://github.com/TheNetworg/oauth2-azure/blob/master/src/Provider/Azure.php that returns an array of string public keys instead of instances of Firebase\JWT\Key. When I change the two lines with $keys[$keyinfo['kid']] = $publicKey; to $keys[$keyinfo['kid']] = new Key($publicKey, 'RS256'); (and import the Firebase\JWT\Key class), the error vanishes. I don't know if this is the best solution, but at least it works for me.

  • 👍1
ganey commented

The provider keys could also converted to a Firebase\JWT\Key before this call:

oauth2-azure/src/Token/AccessToken.php

Line 28 in 56e0776

$idTokenClaims = (array)JWT::decode($this->idToken, $keys, ['RS256']);

not sure where is best to do it though

Summersyard commented

The error remains with the latest versions:
main_acc_entering_the_page_anon

grafickepracecom commented

The decode() JWT function now expects a Key instance or array of Key instances. Possible quick fix is to change Provider/Azure.php.

  1. add import:
    use Firebase\JWT\Key;

  2. change the getJwtVerificationKeys():
    ...
    375: $keys[$keyinfo['kid']] = new Key($publicKey, 'RS256');
    ...
    392: $keys[$keyinfo['kid']] = new Key($publicKey, 'RS256');
    ...

hajekj commented

Do you think you could submit a PR for this? I am leaving for the next 4 days and will be on a very limited connection. Thanks!

hajekj commented

I have merged this, can you please test against dev-master branch, once you confirm, I will release as 2.1.1. Thanks!

terrencegf commented

The proposed hotfix fixed the error for me. Thanks for the quick update!

hajekj commented

Thanks for confirming! I have released v2.1.1 - https://github.com/TheNetworg/oauth2-azure/releases/tag/v2.1.1 with the fix. Thanks for the PR @spackmat!

  • 👍2