Adldap2 / Adldap2-Laravel

LDAP Authentication & Management for Laravel
MIT License
910 stars 184 forks source link

Passport bug after resetting DB #685

Closed sachaw closed 5 years ago

sachaw commented 5 years ago

Description:

Whilst this havily involves passport, I have not observed the same behavior when using the eloquent provider (although I could be wrong)

With a clean Laravel + Adldap2-Laravel + Passport install logging in with Auth::attempt() Works logging in with passport works

if the database is reset/remigrated etc, and the oauth keys are removed, and cache cleared, then new keys are generated, passport just returns an invalid credentials response, however calling Auth::attempt() both works and fixed following passport calls,

It appears that under whatever circumstances, using passport does not establish a new connection automatically and needs to be done manually.

bunday commented 5 years ago

Hi sachaw, this is exactly the problem I’m facing. I noticed that if I authenticate from my web platform, ldap works perfectly, they when I use postman to request a password token for that same user, passport works perfectly as well. But I want to ask where do you call the ‘Auth::attempt()’ maybe I can try that too for my project

bunday commented 5 years ago

@sachaw so I just used your comment to create a workaround. Here is how my process is

  1. I created an endpoint that received the Active Directory username and password of the user and run it on Auth:attempt(). What this does is it Sync the Active Directory Credential of the user to the Local DB.

  2. I created an endpoint that tries to consume the \Laravel\Passport\Http\Controllers\AccessTokenController@issueToken This then validates the same credentials entered in step 1 with the local DB and issues a Token.

Observation

For some funny reasons, I noticed that Laravel Passport wants to authenticate against just Local DB.

I'm open to a better way of implementing this situation if there is.

stevebauman commented 5 years ago

Hi @sachaw,

if the database is reset/remigrated etc, and the oauth keys are removed, and cache cleared, then new keys are generated, passport just returns an invalid credentials response, however calling Auth::attempt() both works and fixed following passport calls,

If you refresh your migrations, your users no longer exist in your database and have no oauth keys to use.

Calling Auth::attempt() works because it will import your LDAP user which would then allow passport to work properly since a local record of your user exists.

I'm not seeing the issue here, but if you insist, I'll need a clearer explanation of why it should work without having a local record of your database user.

viniciusraupp commented 5 years ago

@sachaw so I just used your comment to create a workaround. Here is how my process is

  1. I created an endpoint that received the Active Directory username and password of the user and run it on Auth:attempt(). What this does is it Sync the Active Directory Credential of the user to the Local DB.
  2. I created an endpoint that tries to consume the \Laravel\Passport\Http\Controllers\AccessTokenController@issueToken This then validates the same credentials entered in step 1 with the local DB and issues a Token.

Observation

For some funny reasons, I noticed that Laravel Passport wants to authenticate against just Local DB.

I'm open to a better way of implementing this situation if there is.

I'm in the same situation. Is your implementation working well? How did you do step 2?

Regards.

viniciusraupp commented 5 years ago

I am testing passport with password-grant-tokens and in this case apparently the passport only checks the credentials in the local DB because when I clean the DB with migrate: refresh and import users the password entered in the DB is ramdomic and is updated only after the first login via web and after that works normally.

Translated with google translate, sorry.

https://laravel.com/docs/5.8/passport#password-grant-tokens

sachaw commented 5 years ago

So basically, we need a way either in passport or adldap to get the users passwords and hash them

viniciusraupp commented 5 years ago

I am currently implementing this two methods described in the UserRepository class of the passport in the User model: findForPassport and validateForPassportPasswordGrant

https://github.com/laravel/passport/blob/c0c3fca80d8f5af90dcbf65e62bdd1abee9ac25d/src/Bridge/UserRepository.php

sachaw commented 5 years ago

thanks @viniciusraupp, here's my current implementation, which seems to work.

Just place this in your app\User.php

public function findForPassport($username)
{
    session(['username' => $username]);
    return $this->where('username', $username)->first();
}

public function validateForPassportPasswordGrant($password)
{
    $credentials = ['username' => session()->pull('username'), 'password' => $password];
    return Auth::attempt($credentials);
}
amnoth commented 5 years ago

@sachaw I'm not seeing where validateForPassportPasswordGrant() is being called when there is no user found. Am I missing something? I have tried your snippet but am not seeing the same results.

sachaw commented 5 years ago

@amnoth validateForPassportPasswordGrant() is called by passport if it exists in your used model, it passes the raw username and passport and lets you do whatever with it, returning true returns your access token returning false returns the normal invalid credentials response.

amnoth commented 5 years ago

@sachaw but that only seems to happen if there is a $user model found by that name. Where is the snippet finding the user model in the findForPassport()? In my testing I hit the:

48 if (! $users) {
49     return;
...

block of the UserRepository.php when attempting a login from the password client of passport with no users in the database already. Thanks!