search

sfelix-martins / passport-multiauth

Add support to multi-authentication to Laravel Passport
MIT License
288 stars 51 forks source link

Compatibility Update #123

Closed Megachill closed 4 years ago

Megachill commented 4 years ago

This allows laravel 6.18+ / 7.x support. Replaces deprecated DiactorosFactory. Made downwards compatible, should the version installed have Diactoros instead.

sfelix-martins commented 4 years ago

@Megachill Thanks for this good PR. I will review it right now.

Megachill commented 4 years ago

@sfelix-martins Any news on this? Some of the functionality that was introduced in 7.x is crucial to the work we do. Thanks

sfelix-martins commented 4 years ago

@Megachill I started the review on src/Facades/ServerRequest.php file. Can you answer it?

Megachill commented 4 years ago

Yes sorry, so the Diactoros References need to be removed when using anything below 6.18. My fault.

Hence me making a new version 7.x, I'll update the pull request.

Megachill commented 4 years ago

I'll update the PR

On Mon, Mar 9, 2020 at 9:17 AM Samuel Martins notifications@github.com wrote:

@Megachill https://github.com/Megachill I started the review on src/Facades/ServerRequest.php file. Can you answer it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sfelix-martins/passport-multiauth/pull/123?email_source=notifications&email_token=AAOM32S2TMUYOOVCFKL5IL3RGSX3HA5CNFSM4LBJ54ZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOGJEEQ#issuecomment-596414994, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOM32QBDPS6H43HIHSSBW3RGSX3HANCNFSM4LBJ54ZA .

Megachill commented 4 years ago

@sfelix-martins Updated, this should be good now, Just testing right now on a new Laravel 7.x install

juhasev commented 4 years ago

@sfelix-martins @Megachill What is the status of this PR? I actually made the same changes locally and they work perfectly with all of my unit tests. I guess version 7.x should not support the older versions of Passport and only work with 8.0 to preserve backwards compatibility.

Megachill commented 4 years ago

@sfelix-martins @Megachill What is the status of this PR? I actually made the same changes locally and they work perfectly with all of my unit tests. I guess version 7.x should not support the older versions of Passport and only work with 8.0 to preserve backwards compatibility.

Agreed, I am currently waiting on @sfelix-martins to review and merge in. Currently using own fork on systems and we have seen no issues. This should be a new branch / release that targets Laravel ^7.0 & Passport ^8.0

juhasev commented 4 years ago

@sfelix-martins @Megachill Just wanted to chime in that I wouldn't make Laravel 7 is requirement as you can use the latest version of Passport also with Laravel 6.x which is my case. We are not ready yet to upgrade Laravel 7.

isynergeek commented 4 years ago

Works on "laravel/framework": "^6.0", "laravel/passport": "^8.4",

juhasev commented 4 years ago

@isynergeek Which exact version of Laravel are you running? I believe this broke in the mid stream i.e. Laravel 6.12 or something like that.

isynergeek commented 4 years ago

@isynergeek Which exact version of Laravel are you running? I believe this broke in the mid stream i.e. Laravel 6.12 or something like that.

Laravel Framework 6.18.2

mengdodo commented 4 years ago

good , this work for me 

        "laravel/framework": "^6.0",
        "laravel/passport": "^8.0",
        "smartins/passport-multiauth": "^6.0",
zeroover commented 4 years ago

new,How should I use this 7.x version? tell me please!

sfelix-martins commented 4 years ago

New release published fix it: https://github.com/sfelix-martins/passport-multiauth/releases/tag/v7.0.0