Skip to content
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

Fix support for lcobucci/jwt v3.4 and 4.0 #797

Merged
merged 2 commits into from
Nov 28, 2020
Merged

Fix support for lcobucci/jwt v3.4 and 4.0 #797

merged 2 commits into from
Nov 28, 2020

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented Nov 27, 2020 •
edited
Loading

Fixes #795
Fixes #793

@chalasr chalasr mentioned this pull request Nov 28, 2020
Closed
@chalasr
Copy link
Collaborator Author

chalasr commented Nov 28, 2020

I'm going to merge this one in order to fix some openssl-related issues on PHP 8.
If you see something wrong here, please tell me.

@lcobucci
Copy link

lcobucci commented Nov 28, 2020 •
edited
Loading

@chalasr have you considered the usage of the configuration object? That removes the need for method_exists() checks you're using here. You'd need to change the requirements to ^3.4 || ^4.0 (which should be fine)

@lcobucci
Copy link

Here's an example thephpleague/oauth2-server#1146

@chalasr
Copy link
Collaborator Author

chalasr commented Nov 28, 2020

@lcobucci Thanks for the link, that should simplify things considerably indeed.
I plan to merge this one as-is as a bugfix and tag a point release for it, then open a new PR removing support for <3.4 and switching to that configuration object (it will be part of the next minor release, along with symfony 3.4 support removal)

@lcobucci
Copy link

Sounds like a plan then! Feel free to ping me on the upcoming one and thanks for your work!

@chalasr
Copy link
Collaborator Author

chalasr commented Nov 28, 2020

Thanks for your great work on lcobucci/jwt, the new API is definitely worth the hassle.

@chalasr chalasr merged commit 4d22398 into lexik:master Nov 28, 2020
@Tomanhez Tomanhez mentioned this pull request Nov 30, 2020
Merged
@chalasr chalasr deleted the lcobucci-34 branch November 30, 2020 10:55
pamil added a commit to Sylius/Sylius that referenced this pull request Nov 30, 2020
@pamil
bug #12101 Remove lcobucci/jwt v3.4 conflict (Tomanhez)
28b6db9 
This PR was merged into the 1.8 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| License         | MIT

Fixed after this PR: lexik/LexikJWTAuthenticationBundle#797
And new LexikJWTAuthenticationBundle tag v2.10.1

<!--
 - Bug fixes must be submitted against the 1.7 or 1.8 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

8de9f22 Remove lcobucci/jwt v3.4 conflict
@theofidry
Copy link
Contributor

@chalasr still had an issue (the one reported by #799) with lexik/jwt-authentication-bundle v2.10.4 & lcobucci/jwt 3.3.3. Upgrading to lcobucci/jwt 4.0 fixed it.

Up to you if you really want to fix it though since at least in my case updating was easier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jochen-jung jochen-jung jochen-jung left review comments

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

Successfully merging this pull request may close these issues.

Compatibility with lcobucci/jwt v3.4 php8 compatibility
4 participants
@chalasr @lcobucci @theofidry @jochen-jung