Replace abandoned packages #47
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #47 +/- ##
============================================
+ Coverage 61.75% 64.26% +2.51%
+ Complexity 466 461 -5
============================================
Files 62 62
Lines 1561 1556 -5
============================================
+ Hits 964 1000 +36
+ Misses 597 556 -41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @cooperaj, thanks! |
|
There are a large number of failing static analysis points in files that have not been touched so I can only assume the upstream library has changed some interfaces etc. |
…erBuilderInterface breaks the return type
|
Looks like it should all be passing now but the failing 7.4 test matrix (removed in this PR) stops the rest from completing. |
|
@cooperaj you can fix that by changing the CI under the |
|
Can't believe I missed that. Thanks. EDIT. |
|
Closing and reopening to trigger actions, it got disabled. |
Until PHP-CS-Fixer/PHP-CS-Fixer#8300 is fixed it won't have full 8.4 support but if language features are kept at 8.1 levels (as they should be to ensure supported versions) then having this flag shouldn't cause issue.
| "ext-json": "*", | ||
| "facile-it/php-jose-verifier": "^0.3 || ^0.4.3", | ||
| "facile-it/php-jose-verifier": "^0.5.0-beta1", |
There was a problem hiding this comment.
TODO: if everything is ok, we should re-tag as stable and change the requirement here.
There was a problem hiding this comment.
Quite possibly. Though I've not used any of that library in anger apart from what this library touches so I can't vouch for it all.
There was a problem hiding this comment.
Is the 0.5 tag available on that repo yet? I've an issue open there but it is still open?
There was a problem hiding this comment.
Not yet, I'm thinking that we can use the beta version, and tag this PR in beta too
Apart from the dropping of 7.4 support I'd say that's the case. I don't believe any of the public interfaces were changed but our uses of the library don't touch all the functionality so I'm depending on the unit tests to flag others up. I've been unable to get the conformance suite running. I think the upstream APIs tondo it have changed since the tests were written (~2020) |
| @@ -15,7 +15,7 @@ public function unpack(ClientInterface $client, array $claims): array | |||
| $claimSources = $claims['_claim_sources'] ?? null; | |||
| $claimNames = $claims['_claim_names'] ?? null; | |||
|
|
|||
| if (! is_array($claimSources)) { | |||
| if (! array_key_exists('_claim_sources', $claims) || ! is_array($claimSources)) { | |||
There was a problem hiding this comment.
Why this check? If _claim_sources does not exist it will be null as for the check 3 lines before
There was a problem hiding this comment.
I think (it been a minute) that these extra checks were introduced to stop psalm from grumbling.
I'll verify.
| @@ -47,7 +47,7 @@ public function fetch(OpenIDClient $client, array $claims, array $accessTokens = | |||
| $claimSources = $claims['_claim_sources'] ?? null; | |||
| $claimNames = $claims['_claim_names'] ?? null; | |||
|
|
|||
| if (! is_array($claimSources)) { | |||
| if (! array_key_exists('_claim_sources', $claims) || ! is_array($claimSources)) { | |||
There was a problem hiding this comment.
Why this new check? If _claim_sources does not exist it will be null as for the check 3 lines before
| /** @var MetadataProviderBuilder|null */ | ||
| private $metadataProviderBuilder; | ||
| /** @psalm-suppress PropertyNotSetInConstructor */ | ||
| private ?MetadataProviderBuilder $metadataProviderBuilder; |
There was a problem hiding this comment.
If you set the nullable type you should set it to null without suppressing the error.
| /** @var JwksProviderBuilder|null */ | ||
| private $jwksProviderBuilder; | ||
| /** @psalm-suppress PropertyNotSetInConstructor */ | ||
| private ?JwksProviderBuilder $jwksProviderBuilder; |
There was a problem hiding this comment.
If you set the nullable type you should set it to null without suppressing the error.
Due the web-token libraries reorganisation there are abandoned packages. The upstream facile-it/php-jose-verifier library has a 0.5-beta version that works with the new web-token but needs changes.
These are included. Lots of changes from @p4veI 's branch and some addition work by me.
I've been unable to get the conformance tests to work with a timeout error and there potentially are a large number of other changes that need making.