Skip to content

Switching to upstream version of JWT package with JWK classes added #44

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
wants to merge 2 commits into from
Closed

Conversation

EricTendian
Copy link

When using this library, I came across the following error in doing a composer install:

Warning: Ambiguous class resolution, "Firebase\JWT\ExpiredException" was found in both "[redacted]/vendor/firebase/php-jwt/src/ExpiredException.php" and "[redacted]/vendor/fproject/php-jwt/src/ExpiredException.php", the first will be used.
Warning: Ambiguous class resolution, "Firebase\JWT\JWT" was found in both "[redacted]/vendor/firebase/php-jwt/src/JWT.php" and "[redacted]/vendor/fproject/php-jwt/src/JWT.php", the first will be used.
Warning: Ambiguous class resolution, "Firebase\JWT\SignatureInvalidException" was found in both "[redacted]/vendor/firebase/php-jwt/src/SignatureInvalidException.php" and "[redacted]/vendor/fproject/php-jwt/src/SignatureInvalidException.php", the first will be used.
Warning: Ambiguous class resolution, "Firebase\JWT\BeforeValidException" was found in both "[redacted]/vendor/firebase/php-jwt/src/BeforeValidException.php" and "[redacted]/vendor/fproject/php-jwt/src/BeforeValidException.php", the first will be used.

It turns out that this package conflicts with the google/auth library and other libraries which use the firebase version.

I believe the reason for using the fork is so that the JWK class exists, however I recently had a pull request merged and now released, which means that the JWK class is available in the latest version of firebase/php-jwt.

As a result of all this, I'm updating the composer.json to use this more popular version of the package, instead of the fork. Through code review there was one minor change I had to make to the fproject version, which means I did need to update this LTI package's usage of JWK::parseKey() to JWK::parseKeySet(). Apart from that I did not find other changes.

I did some basic manual testing and everything is still working as expected.

@MartinLenord
Copy link
Collaborator

I will merge this as part of #45 when that is approved, thanks for the update, i hadn't realised that the firebase library had been updated

@tijsverwest
Copy link

Hi guys, do you have a plan when this might be approved for release?

@FlorentTorregrosa
Copy link

Hello, I have tested it when evaluating https://www.drupal.org/project/lti_tool_provider/issues/3202964, on a project where firebase/php-jwt is already present, and so the wrong version of the library was called, ending with a call to a private method.

Thanks a lot for the PR.

@EricTendian
Copy link
Author

EricTendian commented Apr 19, 2021

Closing this PR as this repo appears abandoned, for folks looking for a fix to be released, check out my company's fork of the repo (described in this PR comment): #45 (comment)

snake pushed a commit to snake/lti-1-3-php-library that referenced this pull request Jan 19, 2022
Clean up logic for putting a grade to a line item, add helper functions to extract default line item, and avoid extra line item lookups when not needed
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

Successfully merging this pull request may close these issues.

4 participants