search

TheNetworg / oauth2-azure

Azure AD provider for the OAuth 2.0 Client.
https://packagist.org/packages/thenetworg/oauth2-azure
MIT License
229 stars 108 forks source link

Bug in Firebase/php-jwt #161

Closed Summersyard closed 2 years ago

Summersyard commented 2 years ago

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 2 years ago

Could you please test with the current version in master? I have merged a bunch of hanging PRs, primarily https://github.com/TheNetworg/oauth2-azure/pull/159 should fix it. I aim to release a new version of the library on Friday.

Summersyard commented 2 years ago

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

Summersyard commented 2 years ago

main_acc_entering_the_page - anon

hajekj commented 2 years ago

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 2 years ago

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.

spackmat commented 2 years ago

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.

ganey commented 2 years ago

The provider keys could also converted to a Firebase\JWT\Key before this call: https://github.com/TheNetworg/oauth2-azure/blob/56e0776776476da0b2de8cb7c201bfe7dec7aee1/src/Token/AccessToken.php#L28

not sure where is best to do it though

Summersyard commented 2 years ago

The error remains with the latest versions: main_acc_entering_the_page_anon

grafickepracecom commented 2 years ago

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 2 years ago

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!

spackmat commented 2 years ago

@hajekj sure: #163

hajekj commented 2 years ago

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 2 years ago

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

hajekj commented 2 years ago

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!