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

[13.x] Fix user-token relations #1773

Merged

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Jul 19, 2024

  1. $user->getProviders() has been added recently on PR [13.x] Configure the user provider for PAT #1768, this method actually must not return null in a healthy environment / configuration. Also, it is not possible to use Passport with a database provider. This PR fixes these 2 situations on this method.
  2. The relation between the authenticable model (User model by default) and Token model depends on the client's provider. This PR makes sure we are retrieving the tokens associated with the right authenticable model and adds integration tests for this.
    • Note: On Sanctum user->tokens() relation is morphable, so we know which model is the owner of the key, on Passport we should check the client's provider to know which model is the owner of the key.

Copy link

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.

@hafezdivandari hafezdivandari marked this pull request as ready for review July 19, 2024 17:47
return $provider;
}
}

return null;
throw new LogicException('Unable to determine authentication provider for this model from configuration.');
Copy link
Member

Choose a reason for hiding this comment

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

So what should they do in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should never happen, the authenticatable model should have a provider registered on auth configuration as we instruct the developer when installing Passport

@taylorotwell taylorotwell merged commit f712492 into laravel:13.x Jul 22, 2024
9 checks passed
@hafezdivandari hafezdivandari deleted the 13.x-fix-user-token-relations branch July 22, 2024 15:06
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.

2 participants