search

laravel / sanctum

Laravel Sanctum provides a featherweight authentication system for SPAs and simple APIs.
https://laravel.com/docs/sanctum
MIT License
2.76k stars 298 forks source link

The "web" guard should be a configuration #20

Closed filhocodes closed 4 years ago

filhocodes commented 4 years ago

The cookies are handled by the SessionGuard, and you can have many as you want. I think most cases will be like me where we have one Guard for a UserProvider and a second Guard for a different UserProvider (like table users and customers).

Anyways, I think this value should not be hardcoded: https://github.com/laravel/airlock/blob/master/src/Guard.php#L46 https://github.com/laravel/airlock/blob/master/src/Guard.php#L77

mbougarne commented 4 years ago

Yes, something like this config('airlock.guard', 'web') The same thing with making request to /airlock/csrf-cookie it will be good if we have it configurable, we can change the PREFIX 'web' or 'api'.

mansouralex commented 4 years ago

@mbougarne does this help in having a 2nd guard by setting config('airlock.guard', 'web') on run time?

filhocodes commented 4 years ago

IMO it should not be an Airlock configuration, but an configuration to the Airlock guard.

By example, in the config/auth.php you can configure a TokenGuard to hash the tokens by changing the 'hash' => false, into 'hash' => true, or event use a column different than api_token by adding a storage_key value. Right now the guard is configured right here https://github.com/laravel/airlock/blob/master/src/AirlockServiceProvider.php#L22-L25, and I think this configuration should be flexible.

The problem is that the "airlock" driver is not a "full Guard", but an extension of the RequestGuard, that is just a callable (the Laravel\Airlock\Guard in this case). This callable does not receive any other configuration except the current request and the user provider, and the second is just ignored in Airlock in favor of using the provider from another guard. Airlock is also a "singleton" guard in the sense that the User model is statically defined in the Airlock class, so you can't create multiple guards for the driver "airlock" because would it be always the same User (except with some shenanigans on runtime).

So in order to be able to configure the Airlock guard, and even to allow multiple guards of the Airlock driver, it would need a refactoring in RequestGuard on the laravel/framework to pass extra config into the callable, or a refactoring in Laravel\Airlock\Guard to be a "full custom guard".

And on that note, setting up Airlock to use "configuration by guard" instead of "configuration in airlock.php" would (at least in my mind) require a lot of other changes like, one example, also removing the Airlock::userModel and Airlock::$personalAccessTokenModel to be retrieved from the guard, either by it's provider or by configurations on it. In the end this would change the current scope of Airlock.

That is kinda why this is an issue and not a direct PR on my part, I don't want to assume what Airlock aims to be (a simple authentication solution or a full flexible one)

The one thing I'm certain is that this line https://github.com/laravel/airlock/blob/master/src/AirlockServiceProvider.php#L24 can be removed with the current state. Airlock is not using a provider for itself, and even the Laravel Docs example for Request Guard has a configuration without a provider

driesvints commented 4 years ago

Airlock is meant for SPA's and apis and not meant for web based apps. If you need authentication for web based apps, consider using Passport: https://github.com/laravel/passport

mansouralex commented 4 years ago

@driesvints Passport also doesn't support multi UserProvider, even in SPA you had some use-cases where you need that....

driesvints commented 4 years ago

@mansouralex right, sorry I misread this.

callmez commented 4 years ago

any news?

Wedrix commented 4 years ago

The way I'm solving it now is to define a guard_map config that maps specified route patterns to the appropriate guards. Then in guards.php I fetch the appropriate guard based on the request path. Seems to work for spa but don't know how it affects tokens.

Wedrix commented 4 years ago

https://github.com/Wedrix/airlock/blob/356f382739b59205ec8d4d09e30015bacf8245a9/src/Guard.php#L45

https://github.com/Wedrix/airlock/blob/356f382739b59205ec8d4d09e30015bacf8245a9/src/Guard.php#L89

https://github.com/Wedrix/airlock/blob/356f382739b59205ec8d4d09e30015bacf8245a9/config/airlock.php#L57

dellow commented 4 years ago

Also using multiple guards would really like to see 'web' guard as a config option.

driesvints commented 4 years ago

Pr was merged and will be in the next release

wrabit commented 4 years ago

Taylor removed the config a day later, but kept the functionality there, I guess it's not certain this feature will remain or how it will be implemented, for now I am setting the sanctum.guard value in the route service provider.