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

[2.x] Fix user provider in sanctum guard #225

Merged
merged 3 commits into from Nov 23, 2020

Conversation

sebdesign
Copy link
Contributor

This PR provides the specified user provider to the sanctum guard.

Without that, the Illuminate\Auth\RequestGuard is constructed without a user provider by default, which is inconsistent with the Laravel\Sanctum\Guard callable which accepts the specified user provider.

I've added tests to prove that the sanctum guard is not properly configured:

The Laravel\Sanctum\Guard callable accepts the specified provider in the constructor from the configuration, and validates if the provider model matches the authenticated model. This part is working correctly.

In the service provider, the actual guard Illuminate\Auth\RequestGuard is constructed with the default application provider which is null, instead of the specified provider in the configuration.

So, calling Auth::guard('sanctum')->user() or Auth::guard('sanctum')->validate() will succeed because it's using the user provider configured in the service provider.

But Auth::guard('sanctum')->getProvider() will return null. With this fix it will return the correct user provider.

These tests prove that the `sanctum` guard is not properly configured:

The `Laravel\Sanctum\Guard` callable accepts the specified provider in the constructor from the configuration, and validates if the provider model matches the authenticated model. This part is working correctly.

In the service provider, the actual guard `Illuminate\Auth\RequestGuard` is constructed with the default application provider which is `null`, instead of the specified provider in the configuration.

So, calling `Auth::guard('sanctum')->user()` or `Auth::guard('sanctum')->validate()` will succeed because it's using the user provider configured in the service provider.

But `Auth::guard('sanctum')->getProvider()` returns `null`.
This provides the specified user provider to the `sanctum` guard.

Without that, the `Illuminate\Auth\RequestGuard` is constructed without a user provider by default, which is inconsistent with the `Laravel\Sanctum\Guard` callable which accepts the specified user provider.
@driesvints driesvints changed the title Fix user provider in sanctum guard [2.x] Fix user provider in sanctum guard Nov 23, 2020
@taylorotwell taylorotwell merged commit 3aabba1 into laravel:2.x Nov 23, 2020
@sebdesign
Copy link
Contributor Author

sebdesign commented Nov 23, 2020

@taylorotwell thanks for merging this. IMHO the multiple provider feature (#149) is a bit flaky. Right now the RequestGuard accepts an instance of the user provider but the Guard accepts the name of the provider. But the Guard callback is always invoked with the user provider instance as a second parameter, so there is no need to inject it in the constructor.

Also only the EloquentUserProvider has as model key in the config. The DatabaseUserProvider (or any custom provider) wouldn't have this key.

I think in this should be refactored. Would you accept a PR in 2.x or should I wait for 3.x?

@taylorotwell
Copy link
Member

I dunno. I don't really put a lot of thought into using things outside of Eloquent 🤷

If something needs to be refactored feel free to submit a PR.

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.

None yet

2 participants