Skip to content

bump to php-jwt 6.2 and remove temporary/deprecated code #507

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 25, 2022
Merged

bump to php-jwt 6.2 and remove temporary/deprecated code #507

merged 1 commit into from
May 25, 2022

Conversation

swiffer
Copy link
Contributor

@swiffer swiffer commented Jan 24, 2022

No description provided.

markstory
markstory reviewed Jan 24, 2022
View reviewed changes
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good if we can get the lint error fixed up.

@ndm2
Copy link
Contributor

ndm2 commented Jan 25, 2022

I'm surprised that this didn't break any tests, given the breaking change in JWT::decode(). Seems like there never were any tests for anything other than the default algorithm(s) config.

@ADmad
Copy link
Member

ADmad commented Jan 25, 2022 •
edited
Loading

I'm surprised that this didn't break any tests, given the breaking change in JWT::decode(). Seems like there never were any tests for anything other than the default algorithm(s) config.

Me too. Good thing that we have static analysis 🙂.

Also this will be a breaking change for users and even a minor version bump will not be enough IMO. The JWT authentication will simply break unless users upgrade the firebase/php-jwt packages in their apps.

@swiffer swiffer marked this pull request as draft January 31, 2022 16:20
@ADmad ADmad mentioned this pull request Apr 23, 2022
Closed
@cnizzardini
Copy link
Contributor

Docs should be updated as part of this, especially since this PR: #531

@swiffer
Copy link
Contributor Author

swiffer commented May 14, 2022

I've updated this PR.

I merged 2.x into 2.next as 2.next was several commits behind.
Docs have been updated, deprecated code has been removed.
I bumped php-jwt to ^6.2 as this will add support for cached JWKS keysets (to be used in an upcoming PR)

As suggested by @ADmad should this become v 3.0 instead ?

@swiffer swiffer requested a review from markstory May 14, 2022 07:20
@swiffer swiffer marked this pull request as ready for review May 14, 2022 07:23
@swiffer swiffer changed the title bump to php-jwt 6.0 and remove temporary code bump to php-jwt 6.2 and remove temporary/deprecated code May 14, 2022
@swiffer swiffer mentioned this pull request May 14, 2022
Closed
@swiffer swiffer force-pushed the 2.next branch 2 times, most recently from ac6f069 to fd63425 Compare May 14, 2022 11:55
ADmad
ADmad approved these changes May 14, 2022
View reviewed changes
Copy link
Member

@ADmad ADmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this would be a new major release, potentially along with changes in #535.

@ADmad
Copy link
Member

ADmad commented May 19, 2022

@markstory ping

@markstory markstory merged commit 134bf2c into cakephp:2.next May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ADmad ADmad ADmad approved these changes

@markstory markstory Awaiting requested review from markstory

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@swiffer @ndm2 @ADmad @cnizzardini @markstory