Closed damib closed 1 year ago
stevebauman
commented
1 year ago Hi @damib,
In this issue you mention your "LDAP Server Type" is "Custom LDAP schema". What is that? Is the server an OpenLDAP server, an ActiveDirectory Server, or another distro? This information is crucial so I can thoroughly understand your environment and assist you appropriately.
damib
commented
1 year ago Hi The server is a Debian version of openldap, but the database is very old and recovered from a backup. So, because in the user entries I cannot find the classes you listed in the $objectClasses of the packaged OpenLDAP model class, I decided to build my own model.
Here is an example user entry returned by my server, as you can see classes don't match and the default guiKey is not present (entryuuid).
xxx@portatile:~/dev/example-app$ ldapsearch -x -b "uid=xxx,ou=People,dc=xxx,dc=xxx" -H ldap://xxx.xxx.xxx.xxx
# extended LDIF
#
# LDAPv3
# base <uid=xxx,ou=People,dc=xxx,dc=xxx> with scope subtree
# filter: (objectclass=*)
# requesting: ALL
#
# xxx, People, xxx.xxx
dn: uid=xxx,ou=People,dc=xxx,dc=xxx
cn: XXXX XXXX
givenName: XXX
uid: xxx
uidNumber: 1000
homeDirectory: /homes/xxx
loginShell: /bin/bash
mail: xxx@xxx.xx
objectClass: top
objectClass: inetOrgPerson
objectClass: posixAccount
objectClass: shadowAccount
sn: xxx
gidNumber: 100
# search result
search: 2
result: 0 Success
# numResponses: 2
# numEntries: 1Maybe I didn't catch the logic behind, anyway the model seems to work, as far as I can understand the method getConvertedGuid() was intended to convert to string binary guids, which would be fine even if the attribute is not binary, the problem is that the user provider is still trying to fetch the user using the not converted version of the guid.
Thanks for the reply.
damib
commented
1 year ago Update:
I've tried with the DB synchronized version and everything seems to work fine out of the box. This is why:
So it seems that the retrieveByCredentials() is working fine with the username in both classes, but a mismatch is happening on subsequent requests because the value stored in session differs from the one in the LDAP database, and this happens only for the class NoDatabaseUserProvider.
stevebauman
commented
1 year ago Hi @damib, thanks for providing that additional information.
Can you also post your config/auth.php file?
Here is an example user entry returned by my server, as you can see classes don't match and the default guiKey is not present (entryuuid).
Since your LDAP objects don't contain UUID's or GUID's then we will definitely have to implement some custom logic here.
Your implementation is totally fine for overriding the getConvertedGuid() method on your custom model.
What you will have to do next so that LDAP users are properly located on subsequent authenticated requests to your application, is override the built-in LdapUserRepository.
Create one inside an app/Ldap folder, and override the findByGuid() method. This is what Laravel will call, upon subsequent requests, to locate your LDAP user from your LDAP server:
// app/Ldap/LdapUserRepository.php
namespace App\Ldap;
use LdapRecord\Laravel\LdapUserRepository as BaseRepository;
class LdapUserRepository extends BaseRepository
{
public function findByGuid($guid)
{
$key = $this->createModel()->getObjectGuidKey();
return $this->query()->where($key, "=", $guid)->first();
}
}Once it's created, override the LdapRecord-Laravel it inside of your AppServiceProvider::boot():
// app/Providers/AppServiceProvider.php
use App\Ldap\LdapUserRepository;
use LdapRecord\Laravel\LdapUserRepository as LdapRecordUserRepository;
public function boot()
{
$this->app->bind(LdapRecordUserRepository::class, LdapUserRepository::class);
}Can you give that a shot and see if your sessions start persisting across requests?
damib
commented
1 year ago Hi Steve thanks for the reply, here is the relevant section of my auth.php (synch version):
'providers' => [
// 'users' => [
// 'driver' => 'eloquent',
// 'model' => App\Models\User::class,
// ],
'users' => [
'driver' => 'ldap',
'model' => App\Ldap\LdapUser::class,
'rules' => [],
'database' => [
'model' => App\Models\User::class,
'sync_passwords' => true,
'sync_attributes' => [
'name' => 'cn',
'email' => 'mail',
],
],
],
while this is the plain (not DB synchronized version):
'providers' => [
// 'users' => [
// 'driver' => 'eloquent',
// 'model' => App\Models\User::class,
// ],
'users' => [
'driver' => 'ldap',
'model' => App\Ldap\LdapUser::class,
'rules' => [],
],
],I've tried you solution without the override of getConvertedGuid(), and the problem is still there. The code of the overridden findByGuid() is called, and the query
$this->query()->where($key, "=", $guid)->first();
is trying to do something like:
$this->query()->where('uid', "=", "6d6d6d6d-6d6d-6d6d-6d61-6d6172697361")->first();Here the $key 'uid' is correct, the is the unique identifier within my LDAP database, the problem is that the long string "6d6d6d6d-6d6d-6d6d-6d61-6d6172697361" is the converted version of the username.
If I leave my override of the getConvertedGuid() everything works fine, now the $guid passed to the findByGuid() is the unmodified version of the user name, and the query succeed. But, as I said, it was working even without the overloading UserProvider class and the method findByGuid(). Just to be clear I've never had a problem of guid not persisted across the sessions, I always had problem of WRONG guid persisted, and this thing doesn't happen in the DB synchronized version because in that version the session contains just the numeric ID of the primary key of the user within the DB table.
so just to summarize:
I wonder how it is possible it can work with the standard openLdap model, since the main relevant difference here is the name of the guid field in the database (guidKey). Is it correct that the convertedGuid() is called whenever someone calls the LdapRecord Authenticatable:: getAuthIdentifier() implementation? If yes, ( this is the value persisted in the session), why it then expected to get any user via findByGuid()? (nobody is "unconverting" the value before the LDAP query...).
Thanks again. Damiano
stevebauman
commented
1 year ago Hi @damib,
I've tried you solution without the override of getConvertedGuid(), and the problem is still there.
Yes, you must keep your overridden getConvertedGuid() method on your custom model for my posted solution to work.
Just to be clear I've never had a problem of guid not persisted across the sessions, I always had problem of WRONG guid persisted.
This is because your $guidKey is not actually a valid GUID, but rather a username.
the value returned by convertedGuid() is saved in the session (I don't know if it is supposed to do so).
Yes, it will be saved inside of the session because this is what is needed to locate the logged in LDAP user. This is identical to how Laravel stores ID's inside of the session for locating the authenticated user from your MySQL database.
Further requests recover the covertedGuid from the session and try to findByGuid() as if it was never converted (I would need a method called findByConvertedGuid() in order to match the guid with ldap database).
I'm not sure what you mean by this. Can you elaborate?
Is it correct that the convertedGuid() is called whenever someone calls the LdapRecord Authenticatable:: getAuthIdentifier() implementation? If yes, ( this is the value persisted in the session), why it then expected to get any user via findByGuid()? (nobody is "unconverting" the value before the LDAP query...).
ActiveDirectory requires string GUID's to be converted to their hexadecimal value for users to be located. You can see this taking place here:
This is the purpose of the findByGuid() method.
damib
commented
1 year ago Hi Steve
Hi @damib,
I've tried you solution without the override of getConvertedGuid(), and the problem is still there.
Yes, you must keep your overridden
getConvertedGuid()method on your custom model for my posted solution to work.Just to be clear I've never had a problem of guid not persisted across the sessions, I always had problem of WRONG guid persisted.
This is because your
$guidKeyis not actually a valid GUID, but rather a username.
What do you mean with "valid GUID"? Couldn't it be any unique ID that can fetch users from LDAP database? It's not clear to me what you meant to do with the overloaded repository method findByGuid(), there the query would never match any DB entry because the $guid isn't anymore the plaintext username (or whatever other unique identifier string of the DB objects).
the value returned by convertedGuid() is saved in the session (I don't know if it is supposed to do so).
Yes, it will be saved inside of the session because this is what is needed to locate the logged in LDAP user. This is identical to how Laravel stores ID's inside of the session for locating the authenticated user from your MySQL database.
Does this mean it is supposed to be "unconverted" somewhere before the query to locate the user is executed? I mean, you get an unique ID for the user (username or whatever else unique), you convert it to a long string (hash?), it is then stored in the session. Then you take the long string from the session an try to match an entry in the DB as it was the original ID (before the conversion): it would never work. What I've found, instead, is that the original findByGuid() is indeed working provided it is called using the unconverted version of the guid.
Further requests recover the covertedGuid from the session and try to findByGuid() as if it was never converted (I would need a method called findByConvertedGuid() in order to match the guid with ldap database).
I'm not sure what you mean by this. Can you elaborate?
I see a converted value stored in the session and user to recover the user from LDAP server without any "unconversion" before the query is executed.
Is it correct that the convertedGuid() is called whenever someone calls the LdapRecord Authenticatable:: getAuthIdentifier() implementation? If yes, ( this is the value persisted in the session), why it then expected to get any user via findByGuid()? (nobody is "unconverting" the value before the LDAP query...).
ActiveDirectory requires string GUID's to be converted to their hexadecimal value for users to be located. You can see this taking place here:
This is the purpose of the
findByGuid()method.
I see, so in the case of ActiveDirectory a kind of mangling is done before the lookup query is executed. In all other cases (mine included) the "guid" is used plain, for the argument above it's not surprising to me it doesn't work.
Maybe I miss some basic information on how the whole mechanism is supposed to work, if it is the case I would like to know what it is. Because I don't want to waste your time, feel free to stop here, I've already found a workaround and I can live with it. If, instead, you suspect there is something strange to be further investigated I can provide more details.
Thanks Damiano
damib
commented
1 year ago Hi some more information using cli terminal (laravel tinker):
> $p
= LdapRecord\Laravel\Auth\NoDatabaseUserProvider {#3939}$p contains the same user provider used by laravel SessionGuard::class During the first authentication Laravel SessionGuard retrive the user by credentials:
> $u = $p->retrieveByCredentials(['uid' => 'marisa'])
= App\Ldap\User {#3984
+exists: true,
+wasRecentlyCreated: false,
+wasRecentlyRenamed: false,
}After succesful authentication the uset auth id is stored in the session by the SessionGuard
>
> $u->getAuthIdentifier()
= "6d6d6d6d-6d6d-6d6d-6d61-6d6172697361"On subsequent requests the SessionGuard tries to fetch the user from LdapRecord using:
> $u = $p->retrieveById('6d6d6d6d-6d6d-6d6d-6d61-6d6172697361')
= nullWRONG! Cannot retrieve the user from LDAP.
Instead if I try:
$p->retrieveById('marisa')
= App\Ldap\User {#3988
+exists: true,
+wasRecentlyCreated: false,
+wasRecentlyRenamed: false,
}FOUND!
Since this few method are the only one used by Laravel SessionGuard, probably a cleaner way to avoid the problem is just overload the my user model in this way:
public function getAuthIdentifier(){
return $this->getObjectGuid();
}
hurik
commented
1 year ago Thank you very much! I had the same problem and now it is fixed!
stevebauman
commented
1 year ago @damib
What do you mean with "valid GUID"? Couldn't it be any unique ID that can fetch users from LDAP database? It's not clear to me what you meant to do with the overloaded repository method findByGuid(), there the query would never match any DB entry because the $guid isn't anymore the plaintext username (or whatever other unique identifier string of the DB objects).
"Globally Unique Identifiers" come in several forms. The meaning is in the name -- a GUID must be "Globally Unique", i.e. not repeated anywhere else in the world. It is an ID that will be generated once, and never again. A GUID is not a username. A username is repeatable across domains.
I see, so in the case of ActiveDirectory a kind of mangling is done
ActiveDirectory requires GUIDs to be converted to a hexadecimal format for use in LDAP searches.
I'm glad you were able to resolve the authentication issues! I'll close this out as completed. I hope what I was able to add helped in some way.
Environment:
Describe the bug:
I followed the documentation on this page https://ldaprecord.com/docs/laravel/v2/auth/plain/configuration#introduction, so I've modified login controller, form, and Request in order to match my custom model:
namespace App\Ldap;
use LdapRecord\Models\Model; use Illuminate\Contracts\Auth\Authenticatable; use LdapRecord\Models\Concerns\CanAuthenticate; use LdapRecord\Models\Concerns\HasPassword;
class LdapUser extends Model implements Authenticatable { use CanAuthenticate; /**
@var array */ public static $objectClasses = [ "top", "inetOrgPerson", "posixAccount", "shadowAccount", ];
protected $guidKey = 'uid';
}
Things that are working:
PROBLEM: After the first successful authentication, all the protected routes redirect to the login page as if the user is not logged in. After deep investigation I've found that the session file contains the wrong uid for the user! Instead of the regular username, the session contains a the long string returned by the Model:: getConvertedGuid() method, so that whenever a subsequent request arrives, the Laravel tryes to fetch the user from the LdapRecord-laravel user provider (in my case NoDatabaseUserProvider) using such a long string as 'uid' search criteria. Obviously the user is not found and the request is considered not authenticated, redirecting (again) to the login page.
WORKAROUND
In my custom model I've overloaded the getConvertedGuid() with the following:
I'm not sure if this is a bug or not, anyway something on the chain is using the Model::getConvertedGuid() in place of the Model::getObjectGuid(). Or I'm doing something wrong.