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.

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

Fix inconsistent persistDeviceCode usage #1446

Closed

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Sep 25, 2024

On "Device Authorization" grant, the \League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface::persistDeviceCode() method has been used 3 times with 3 different purposes:

  1. For persisting the new device code (when requesting a device code)
  2. For updating the "User ID" and "User Approved" (when completing the device auth request)
  3. For updating "Last polled at" (when responding to access token request)

This makes the implementation of this method too hard, as we have to manually determine that for which purpose this method has been called!

You may check the workaround I had to use to be able to implement this on Laravel Passport here.

This PR adds 2 new methods to fix this:

  • \League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface::persistUser()
  • \League\OAuth2\Server\Repositories\DeviceCodeRepositoryInterface::persistLastPolledAt()

@hafezdivandari
Copy link
Contributor Author

hafezdivandari commented Oct 16, 2024

@Sephster Would you please let me know if this PR is something that will be considered, or should we keep the current workaround in the implementation?

I could also use upsert for this, but I believe it's safer not to update all attributes each time.

@hafezdivandari
Copy link
Contributor Author

Never mind! I've used upsert instead at laravel/passport@eca145a

@hafezdivandari hafezdivandari deleted the device-grant-persist branch October 17, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant