passport icon indicating copy to clipboard operation
passport copied to clipboard

Published 20 hours ago • verified publisher icon laravel

[13.x] Support OAuth2 Server v9

Open hafezdivandari opened this issue 1 year ago • 14 comments

Support OAuth2 Server v9.

  • [x] thephpleague/oauth2-server#1398
  • [x] thephpleague/oauth2-server#1402

hafezdivandari avatar Mar 29 '24 15:03 hafezdivandari

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

github-actions[bot] avatar Mar 29 '24 15:03 github-actions[bot]

@driesvints this repo doesn't have master branch?

hafezdivandari avatar Mar 29 '24 16:03 hafezdivandari

Great work on creating this PR so quickly seeing v9rc just launched a couple of days ago.

I know this PR is still a WIP but do you plan to include the new Device Authorization?

jayan-blutui avatar Mar 29 '24 21:03 jayan-blutui

@jayan-blutui thanks.

I know this PR is still a WIP but do you plan to include the new Device Authorization?

Not in this PR, as it makes this hard to review.

hafezdivandari avatar Mar 30 '24 11:03 hafezdivandari

Thanks a lot for this PR @hafezdivandari. We should indeed try to support the new Device flow in v13 but let's indeed tackle that in a subsequent PR.

driesvints avatar Apr 02 '24 08:04 driesvints

@driesvints This PR is ready, but it should target master, which doesn't exist on this repo.

hafezdivandari avatar May 15 '24 13:05 hafezdivandari

@hafezdivandari that's okay. We're using a different approach these days. I'll talk to the team to prepare a new major version. Ideally we'd also take in the new device flow (separate PR).

Thanks for working on this one!

driesvints avatar May 15 '24 14:05 driesvints

@hafezdivandari I retargeted your PR to 13.x now

driesvints avatar May 16 '24 07:05 driesvints

@hafezdivandari @driesvints why not to 12.x?

siarheipashkevich avatar May 16 '24 09:05 siarheipashkevich

@siarheipashkevich there's just too many breaking changes in here.

@hafezdivandari reminds me: could you make a list of upgrade steps in UPGRADE.md? I think we only need to reference the OAuth Server client v9 upgrade and a link to its own release notes, telling people to review those as well: https://github.com/thephpleague/oauth2-server/blob/master/CHANGELOG.md#900---released-2024-05-13

driesvints avatar May 16 '24 11:05 driesvints

Can someone give me an idea of the full upgrade steps and mark as ready for review?

taylorotwell avatar May 16 '24 21:05 taylorotwell

@taylorotwell those are already added to the upgrade guide in this PR. The main reason for this new major version is the oauth server v9 bump and the method signature changes.

driesvints avatar May 16 '24 22:05 driesvints

@driesvints We are not going to release 13.x immediately after merging this right? I have 1 or 2 PRs for 13.x before release. I don't want to add too many changes at once, that makes this one hard to review.

hafezdivandari avatar May 16 '24 23:05 hafezdivandari

@hafezdivandari no I'll wait for those

driesvints avatar May 16 '24 23:05 driesvints

@driesvints I guess I'm looking for a general summary of the changes someone will need to make. What changes will a typical Laravel Passport app need to make? Earlier it was mentioned there are too many breaking changes for patch - what are those breaking changes specifically?

taylorotwell avatar May 17 '24 21:05 taylorotwell

@taylorotwell nothing, just PHP 8.1+ and removed message property from OAuthException HTTP response. Now just use error_description as per the OAuth 2 spec.

The braking change is that most of the methods signatures are changed.

hafezdivandari avatar May 17 '24 21:05 hafezdivandari