Since https://github.com/laravel/passport/issues/161 has been locked we are no longer able to discuss the issue. Before I submit a PR I want to make sure we have a common ground on what the expected behavior is for multi-provider support in Passport. More specifically, should the provider be defined at the oauth client level? This would limit that client id/secret pair to always return from a specific provider. If the gate requested another provider than what the client has defined it would return a 401.
This is the functionality I have built into my current project and I would be more than willing to issue a PR for it but I want to make sure that is what everyone is expecting.
EDIT: I should note, that if no provider is set on the oauth client then it will follow the same logic as what is in place now, so it should be backward compatible outside of the migration.
I agree that this issue might be a good place to restart the discussion on how to implement this. I currently don't have time myself to look into this but feel free to provide thoughts here.
Warning: I'll be removing all comments which aren't related to looking for an actual implementation to prevent this issue from de-railing like the other one. Please only discuss the possible implementation for this.
I think to get this started we should first determine if the multi-auth implementation should be enforced or not. Meaning, should the gate simply return the user against its own provider or should the client have a provider that enforces the user must come from a specific provider?
Or to make it more dynamic, the provider can be supplied in the request and the guard check can be made against that request parameter to make it compatible with as many models as required
Hello everyone.
Take a look at my proposal #987
Personally, I think each oauth_client should be limited to 1 provider but I can see how this may be limiting in some use cases. If we open it up to multiple providers per client then the developer will always be responsible for returning the correct user against any of the allowed providers.
Personally, I think each
oauth_clientshould be limited to 1 provider but I can see how this may be limiting in some use cases. If we open it up to multiple providers per client then the developer will always be responsible for returning the correct user against any of the allowed providers.
But isn't that the point of having multi-auth, to have the flexibility of choosing a provider but at the same time, using fallbacks for default behavior, like using the User model as is the case now.
Personally, I too think that each oauth_client should have one provider but its better to have the implementation for multiple providers as it is might offer more functionality.
We should probably gather more opinions on this and implement the consensus then.
Please remember to keep this discussion on topic.
@billriess When can we expect a PR with the functionality that you have implemented?
I was hoping for some more feedback as to what way would make the most sense for everyone. I can put a PR together later this week with my implementation and see how that goes.
I think the provider should be based on the routes... therefore different routes will have different providers.
Suppose there is an admin model. We can set different routes in the boot() method of AuthServiceProvider like so...
// Admin passport routes
Passport::routes(
null,
[
'as' => 'admin.',
'prefix' => 'admin/oauth',
'middleware' => 'passport.guard:admin',
]
);
Here I am assigning a middleware(which needs to be implemented accordingly) to the the routes as to tell passport to use the admin guard which decides the provider to be used.
What do you folks think? @billriess @driesvints
I think that if you're using Passport then it should be relatively safe to assume that you're not a beginner developer, and for that reason it should be as flexible as possible even if that means that a little work is required of the developer.
I think it makes sense to have separate endpoints per your suggestion @nikugogoi for the various models so long as there is flexibility in the way those models are authenticated -- some models may not use email / password for example, we may use uuid/id and secret key or some such authentication.
As long as the implementation allows us to attach multiple guards to a single api endpoint, e.g. if i wanted either users, or admins, or whoever to be able to access the resource.
@zagreusinoz for customizing the way how the models are authenticated, there is a way here https://laravel.com/docs/5.8/passport#customizing-the-username-field
Using guards, with passport as the authentication driver, on endpoints work well to restrict access resource... The problem arises during issue of tokens as passport is currently only configured for one guard. I feel there should be a way to assign guards according to different routes.
Also the I think the passport tokens table will need a change.
The implementation I have in place locally in my project is that oauth_clients now has a provider column that determines the given provider. If it is null it defaults back to users but if it is set then it will only authenticate against with its defined provider. In my routes you can specify auth:api for standard user-based authorization while something like auth:customers will only authorize the customers provider.
@billriess are you able to share even a draft PR with your current implementation? This would solve some issues I'm having in a project.
I agree that tying an oauth_client to a specific provider would be the desired functionality.
@tmcnicholls I've been a little busy recently with a project but I will put something together and submit it. I know PRs are viewed pretty closely here so I want to make sure the implementation I suggest and the way I implement it will be the desired approach. The way I got this working locally was changing the TokenGuard's construct to accept a $requestedProvider instead of passing an already formed Provider this is what lets me do the check. Then I can create the appropriate Provider and pass that along.
@billriess I'd like to see you go ahead with your current implementation around each oauth_client having a provider. Over the last few years my team has been looking at ways to implement multi-auth with passport - all seem a hack as there is no native support.
+1 for your implementation. I think to get things moving a fully fledged PR would go along-way. Also be very appreciated by the community.
So I started a PR for this but one thing I'm stuck on is how to retrieve the string name of the provider inside the TokenGuard without passing it directly. I don't see a method on the UserProvider to retrieve which provider is currently being used.
Otherwise, the only way around it is to pass the $config['provider'] to the TokenGuard from the service provider which feels wrong.
@billriess Would it be possible to see your work so far please? I checked your forked but can't seem to find your work related to this issue :/
I think keeping the option open to specify the provider dynamically would be nice, to allow a token request like this:
grant_type => password,
client_id => someclientid
client_secret => somesecret,
username => someuser,
password => somepass,
client_scope => *,
provider => 'admins'
I'm currently working on something very closely related to this. And will be working on testing some changes locally that may help resolve this; #1094 and #1006
I apologize that this post is a bit long and goes slightly off topic / in depth; but figured this may end up being a great solution for others beyond just myself.
But thought I'd share what I'm thinking of trying in the event anyone has any improvements or recommendations on how to make this solution better. (or even if there are any issues I'm not seeing)
(Devs/Mods if I should create a separate issue please let me know, but thought it would be best to post this here instead of creating a new issue and linking it)
I was originally using two guards, and setting a cookie on login for my "admin" guard, and setting a header value for my "api-admin" guard. After talking to one of the developers at T/T they recommended extending the EloquentUserProvider to try my different models allowing me to get rid of duplicated guards.
In my models I've overrode the getAuthIdentifier function
public function getAuthIdentifier()
{
return get_class($this)."_".$this->{$this->getAuthIdentifierName()};
}
And the "driver" I'm using for the provider looks like this:
<?php
namespace App\Providers;
use Illuminate\Auth\EloquentUserProvider;
use Illuminate\Contracts\Hashing\Hasher as HasherContract;
class UserProvider extends EloquentUserProvider
{
/**
* The array of models we want to be able to authenticate against
*
* @var array
*/
protected $models = [
'App\Models\Admin',
'App\Models\User'
];
/**
* Create a new database user provider.
*
* @param \Illuminate\Contracts\Hashing\Hasher $hasher
* @return void
*/
public function __construct(HasherContract $hasher)
{
$this->hasher = $hasher;
}
/**
* Retrieve a user by their unique identifier.
*
* @param mixed $identifier
* @return \Illuminate\Contracts\Auth\Authenticatable|null
*/
public function retrieveById($identifier)
{
$split = explode('_',$identifier);
$this->model = $split[0];
$identifier = $split[1];
return parent::retrieveById($identifier);
}
/**
* Retrieve a user by their unique identifier and "remember me" token.
*
* @param mixed $identifier
* @param string $token
* @return \Illuminate\Contracts\Auth\Authenticatable|null
*/
public function retrieveByToken($identifier, $token)
{
$split = explode('_',$identifier);
$this->model = $split[0];
$identifier = $split[1];
return parent::retrieveByToken($identifier, $token);
}
/**
* Retrieve a user by the given credentials.
*
* @param array $credentials
* @return \Illuminate\Contracts\Auth\Authenticatable|null
*/
public function retrieveByCredentials(array $credentials)
{
foreach($this->models as $model)
{
$this->model = $model;
$user = parent::retrieveByCredentials($credentials);
if($user != null)
return $user;
}
return null;
}
}
Registered in the AuthServiceProvider like this:
Auth::provider('tenantAware', function ($app, array $config) {
return new UserProvider($app['hash']);
});
And finally the config looks like this:
'guards' => [
'web' => [
'driver' => 'session',
'provider' => 'tenantAware',
],
'api' => [
'driver' => 'passport',
'provider' => 'tenantAware',
],
],
'providers' => [
'tenantAware' => [
'driver' => 'tenantAware'
],
],
With these changes normal non-api related login and viewing work perfect. As soon as I use passport users aren't selected properly.
Instead of passport storing everything as a "HasMany" or "BelongsTo" relationship, I'm going to change these to polymorphic relationships. This will allow the different models being used for authentication to be stored in the same database and not risk a user getting inappropriate access due to a guard not being set.
I.E. if the default guard is "api-admin" and a user authenticates without having had the guard changed; they won't be assumed to be an admin with the same primary key as them.
I have to look further into the code (Hopefully someone else can provide better guidance here); but then the model wouldn't need to be pulled from the config as it would be stored in the record relating to the token being used. With knowing the model, and the ID; the getAuthIdentifier function can easily be used if needed.
Using this type of solution, there would be no need to pass anything additional with the request in order to specify which guard or provider to use as all models would be using the same guard and provider.
This also decreases the amount of logic needed on the frontend dividing out and creating additional login forms for different providers.
This solution assumes that all providers are using eloquent models for authentication, (can't say it would work with multiple different provider types)
I haven't tested password resets or anything beyond basic login/logout.
Because I'm not actually storing the getAuthIdentifier value in a database field relating to the value returned by getAuthIdentifierName I foresee the possibility of issues arising, especially if there is any foreign keys using getAuthIdentifierName to specify the field for the relation
I was working on yet another multi-auth PR for passport and some considerations came to my mind:
How can we specify the auth provider to use?
...
grant_type=password&
[email protected]&
provider=admins
...
The problem with this approach (at least for the password grant type) is that the grant type provided by league does not accept any additional parameter outside username, password, grantType and clientEntity. Furthermore this would allow any client to use any auth provider.
...
grant_type=password&
[email protected]
...
The problem with this approach is the same as before, any client could use any auth provider. Plus it's important to figure out which character is safe to use as a separator between the provider and the username. This would only work for the password grant type, because it's the only one having the username parameter.
Another problem to tackle is that currently only one auth provider can be defined for passport and that's injected in the service provider, while instead it should be resolved by analyzing what's passed from one of the 3 options above.
So far I've tried only the second approach and only for the password grant type. You can find my progress here: https://github.com/lucadegasperi/passport/tree/master.
Very simple solution:
https://github.com/hamedov93/passport-multiauth
Base on @hamedov93 's solutions, I was able to create a simpler package with lesser configuration/settings.
https://github.com/binarygeotech/passport-booster
Please check it out, comments are welcome.
While I like the efforts made by both @hamedov93 and @binarygeotech I think that leaking the provider to use into the request to get an access token is not safe unless there's a check that a certain client can use that provider. The solution I use internally is to instead bind a client to a provider. You want a different provider, you use a different client. This way the request for an access token remains 100% oauth standard. You can see how I achieved that as well as a device authentication flow here: https://github.com/lucadegasperi/passport/tree/ultrapass
While I like the efforts made by both @hamedov93 and @binarygeotech I think that leaking the provider to use into the request to get an access token is not safe unless there's a check that a certain client can use that provider. The solution I use internally is to instead bind a client to a provider. You want a different provider, you use a different client. This way the request for an access token remains 100% oauth standard. You can see how I achieved that as well as a device authentication flow here: https://github.com/lucadegasperi/passport/tree/ultrapass
100% this. I utilize a similar pattern in my project. I don't think we should be opening the oauth request to determining the provider. The oauth client should be bound to the provider. As @lucadegasperi said, if you want a different provider then you use a different oauth client.
@taylorotwell @driesvints it would be nice to get some feedback on this as this problem isn't going away.
I think we can agree that @lucadegasperi is on the right track. Feel free to send in a PR that addresses that use case.
We will probably never accept a client that's in the request since that's not secure.
I didn't used any other package for this issue. First when i came here in this discussion i got a solution in @hamedov93 and @binarygeotech comments. But i thought why should i install any extra dependency if there is some logical implementations which can do something for me and it works. i got a solution after identifying the behaviour of passport driver is just about to authenticate token but how i get token for any other Model if i already declared "driver"=> "passport" and i did same for every Model
In my config/auth.php i couldn't changed any thing
"guards"=>[
"employees"=>[
"driver"=> "passport",
"provider"=> "employees"
],
"admins"=>[
"driver"=> "passport",
"provider"=> "admins"
],
"companies"=>[
"driver"=> "passport",
"provider"=> "companies"
]
]
and in my login api i just used super telented config() function to set value of provider of my desired guard just like below given snippet
Assume that you are in method for admin_login here what i did is
config(["auth.guards.admins.driver"=>"session"]);
And then Auth::attempt($credentials) didn't gave any error which i faced earlier.
And When i generated the token using $request->user()->createToken("access token") which i send to frontend for further api calls
I didn't used any other package for this issue. First when i came here in this discussion i got a solution in @hamedov93 and @binarygeotech comments. But i thought why should i install any extra dependency if there is some logical implementations which can do something for me and it works. i got a solution after identifying the behaviour of passport driver is just about to authenticate token but how i get token for any other Model if i already declared
"driver"=> "passport"and i did same for every Model
In myconfig/auth.phpi couldn't changed any thing
"guards"=>[ "employees"=>[ "driver"=> "passport", "provider"=> "employees" ], "admins"=>[ "driver"=> "passport", "provider"=> "admins" ], "companies"=>[ "driver"=> "passport", "provider"=> "companies" ] ]
and in my login api i just used super telentedconfig()function to set value of provider of my desired guard just like below given snippet
Assume that you are in method foradmin_loginhere what i did is
config(["auth.guards.admins.driver"=>"session"]);
And thenAuth::attempt($credentials)didn't gave any error which i faced earlier.
And When i generated the token using$request->user()->createToken("access token")which i send to frontend for further api calls
Can you give a detailed description of how you did it step by step? Thanks.
While I like the efforts made by both @hamedov93 and @binarygeotech I think that leaking the provider to use into the request to get an access token is not safe unless there's a check that a certain client can use that provider. The solution I use internally is to instead bind a client to a provider. You want a different provider, you use a different client. This way the request for an access token remains 100% oauth standard. You can see how I achieved that as well as a device authentication flow here: https://github.com/lucadegasperi/passport/tree/ultrapass
@lucadegasperi I read the changes made in your fork.
Looks great and complete.
@PeterPan73 sure let me share some steps with you
Step 1:
in my config/auth.php i have following guards.
``` 'guards' => [
'web' => [
'driver' => 'session',
'provider' => 'users',
],
'api' => [
'driver' => 'passport',
'provider' => 'users',
'hash' => false,
],
'contractor' => [
'driver' => 'passport',
'provider' => 'contractors',
'hash' => false,
],
'crew' => [
'driver' => 'passport',
'provider' => 'crews',
'hash' => false,
],
],
Step 2:
in my `routes/api.php` you can see my routes.
Route::group(['prefix' => 'v1'], function() {
Route::post('register-user','ApiController@register_user'); //user register
Route::post('user-login','ApiController@user_login'); //user login
Route::post('contractor-login','ContractorController@contractor_login'); //Contractor login
Route::post('crew-login','ContractorController@crew_login');//crew login
Route::group(['middleware'=>'auth:api'], function() {
// apis for users
Route::put('edit-profile','ApiController@edit_profile');
Route::post('create-job','ApiController@create_job');
});
Route::group(['middleware'=>'auth:contractor'], function() {
// apis for contractors
Route::post('create-crew','ContractorController@create_crew');
Route::post('create-job-proposal','ContractorController@create_job_proposal');
});
Route::group(['middleware'=>'auth:crew'], function() {
// apis for crew
// Route::post('edit-profile','CrewController@edit_profile');
});
});
Step 3:
i have updated my models file to implement `HasApiTokens` Trait and extends Model with `Authenticatable` class intead of `Model`
// App\User.php
use Illuminate\Foundation\Auth\User as Authenticatable;
use Laravel\Passport\HasApiTokens;
class User extends Authenticatable
{
use HasApiTokens;
}
// App\Contractor.php
use Illuminate\Foundation\Auth\User as Authenticatable;
use Laravel\Passport\HasApiTokens;
class Contractor extends Authenticatable
{
use HasApiTokens;
}
// App\Crew.php
use Illuminate\Foundation\Auth\User as Authenticatable;
use Laravel\Passport\HasApiTokens;
class User extends Authenticatable
{
use HasApiTokens;
}
Step 4:
in my ApiController which i used for users authentication and his/her authenticated routes
use Illuminate\Support\Str;
use Laravel\Passport\Token;
use Validator;
class ApiController extends Controller
{
public function user_login(Request $request){
$validation = Validator::make($request->all(),[
'email' => 'required',
'password' => 'required',
]);
if($validation->fails()){
$data = api_validation_error('Validation Errors',$validation->messages());
return response()->json($data,422);
}
if(Auth::attempt($request->only('email','password'))){
$user = $request->user();
Token::where('user_id',$user->id)->update(['revoked'=>1]);
$tokenObj = $user->createToken('user access token');
$token = $tokenObj->token;
$token->expires_at = Carbon::now()->addWeeks(2);
$token->save();
$token->accessToken;
$token = $tokenObj->accessToken;
$data = Arr::add($user->toArray(),'token_detail',['access_token' => $token,'token_type' => 'Bearer']);
$data = api_success('Login Successfully',$data);
return response()->json($data);
}else{
$data = api_error('Invalid Credentials');
return response()->json($data);
}
}
public function edit_profile(Request $request){
//this api will run if user pass then token
$validation = Validator::make($request->all(),[
'first_name' => 'required',
'last_name' => 'required',
'phone' => 'required',
'email' => 'required',
'address' => 'required',
'city' => 'required',
'state' => 'required',
'postal_code' => 'required',
]);
if($validation->fails()){
$data = api_validation_error('Validation Errors',$validation->messages());
return response()->json($data,422);
}
$user = $request->user();
$user->first_name = $request->first_name;
$user->last_name = $request->last_name;
$user->phone = $request->phone;
$user->email = $request->email;
$user->address = $request->address;
$user->city = $request->city;
$user->state = $request->state;
$user->postal_code = $request->postal_code;
$user->save();
if($user){
$data = api_success('Profile updated Successfully',$user);
return response()->json($data);
}
}
}
In `ContractorController.php`
class ContractorController extends Controller
{
//
public function contractor_crew_login(Request $request){
$validation = Validator::make($request->all(),[
'email' => 'required',
'password' => 'required',
]);
if($validation->fails()){
$data = api_validation_error('Validation Errors',$validation->messages());
return response()->json($data,422);
}
config(['auth.guards.contractors.driver'=>'session']);
if(Auth::guard('contractor')->attempt($request->only('email','password'))){
$user = auth()->guard('contractor')->user();
Token::where('user_id',$user->id)->update(['revoked'=>1]);
$tokenObj = $user->createToken('contractor access token');
$token = $tokenObj->token;
$token->expires_at = Carbon::now()->addWeeks(2);
$token->save();
$token->accessToken;
$token = $tokenObj->accessToken;
$data = Arr::add($user->toArray(),'token_detail',['access_token' => $token,'token_type' => 'Bearer']);
$data = api_success('Login Successfully',$data);
return response()->json($data);
}else{
$data = api_error('Invalid Credentials');
return response()->json($data);
}
}
public function create_crew(Request $request){
// this api will be accessible when contractor api pass token.
$validation = Validator::make($request->all(),[
'first_name' => 'required',
'last_name' => 'required',
'phone' => 'required',
'email' => 'required',
'password' => 'required|confirmed',
'password_confirmation' => 'required',
'address' => 'required',
'city' => 'required',
'state' => 'required',
'services' => 'required|array',
]);
if($validation->fails()){
$data = api_validation_error('Validation Errors',$validation->messages());
return response()->json($data,422);
}
if(Crew::where('email',$request->email)->exists()){
$data = api_error('crew with this email already exists');
return response()->json($data,422);
}
$crew = new Crew;
$crew->crew_id = (String) Str::uuid();
$crew->contractor_id = $request->user()->contractor_id;
$crew->first_name = $request->first_name;
$crew->last_name = $request->last_name;
$crew->phone = $request->phone;
$crew->email = $request->email;
$crew->password = bcrypt($request->password);
$crew->address = $request->address;
$crew->city = $request->city;
$crew->state = $request->state;
$crew->save();
$data = api_success('Crew added successfully',$crew);
return response()->json($data);
}
}
}
```
for crew apis i did same as i did for contractor apis. if it authenticate the crew he will able to access that particular api.
i hope i cleared everything and i think these code snippets are clearly understood
@billriess @lucadegasperi
It is worth noting that the token guard already picks the right provider based on the guard used for authentication.
$this->retrieveById will return a user for auth:api and an admin for auth:admin by default.
The next step is to check that the token was really issued for the provider of this user.
This can be accomplished by fetching the client provider from database and compare against it. Or we can add a custom claim to the token something like oauth_provider Therefore we will not need another database query to validate the provider.
But this requires passing a modified BearerTokenValidator to the resource server to return our custom claim to the token guard.
https://github.com/laravel/passport/pull/1220
I've taken another stab at implementing this, it seems like some of the tests failed although locally it seemed okay - I'll take a look at those tomorrow.
For anyone still interested in this feature please take a look at the pr above and provide feedback on it. Thanks.
I updated the PR, all tests are passing now.
Closing this issue now that this has been merged into master.
You all can thank @billriess for getting this in.
Thanks @billriess amazing job!
Great work @billriess
So What should I actually do then.
Is it only supported for password grant token ?
because provider is null for personal access token
Can you please also update the docs? I've created 2 guards:
'api' => [
'driver' => 'passport',
'provider' => 'users',
],
'admin' => [
'driver' => 'passport',
'provider' => 'admins',
],
with the following providers:
'providers' => [
'users' => [
'driver' => 'eloquent',
'model' => App\User::class,
],
'admins' => [
'driver' => 'eloquent',
'model' => App\Admin::class,
],
],
now when I try to use it:
Route::group(['middleware' => ['auth:api', 'auth:admin']], function () {
dd(Auth::guard('admin')->check());
});
returns true even if I pass the user token. I've tried to create a new client for Admin model with the artisan command:
php artisan passport:client --provider=admins
but it also seems to not work as expected. It created a new client but provider field is null.
Route::group(['middleware' => ['auth:api,admin']], function () {
dd(Auth::guard('admin')->check());
});
@driesvints is it only creates password grant token. cuz in the personal access token provider is still null
You set the provider when you create the oauth_client, it doesn't set the provider afterward. The purpose of this is to make sure only 1 provider can use the oauth_client. If you don't need this functionality you can leave it null. This is for password grants.
@billriess is it possible to have this functionality for personal access token
That doesn't work for me. I'm logged in as user but Auth::guard() tells me that I'm an admin...maybe some docs on this. How can we use this to check which guard is logged in on same route handler?
This is for password grants not personal access tokens.
@billriess what's the use case for this?
@billriess will it work with CreateFreshApiToken middleware
Most helpful comment
I updated the PR, all tests are passing now.