Integrity constraint violation on initial login with existing user db record

[ckieffer]

Environment (please complete the following information):

• LDAP Server Type: ActiveDirectory
• PHP Version: 7.2
  • Laravel: 5.8
  • LdapRecord-Laravel: 1.4

This likely isn't a bug, but I haven't seen a similar issue or related documentation yet. Authentication is working but synchronization fails with existing user records.

Logging in for the first time as a user that was added to my users table prior to LdapRecord implementation fails with integrity constraint violations. Stack trace shows inserts instead of updates.

Logging in as a user that does not yet exist in the Users table works. User record is created and populated using the sync_attributes defined.

    'providers' => [
        // Commented out for ldaprecord.
        // 'users' => [
        //     'driver' => 'eloquent',
        //     'model' => App\User::class,
        // ],

        'ldap' => [
            'driver' => 'ldap',
            'model' => LdapRecord\Models\ActiveDirectory\User::class,
            'rules' => [],
            'database' => [
                'model' => App\User::class,
                'sync_passwords' => false,
                'sync_attributes' => [
                    // 'name' => 'cn', // Removed name (full name) from database.
                    'username' => 'samaccountname',
                    'last' => 'sn',
                    'first' => 'givenname',
                    'email' => 'userprincipalname',
                ],
            ],
        ],

Could this be due to an empty GUID value for the existing user record? If so, I can go ahead and add GUIDs to my user admin CRUD controller.

Also worth noting that I

• dropped the stock Laravel user.name field in favor of first and last.
• am testing with myself as the bind and authenticating user.
• can provide stack trace, if necessary.

[stevebauman]

Hi @ckieffer,

Could this be due to an empty GUID value for the existing user record? If so, I can go ahead and add GUIDs to my user admin CRUD controller.

Yes this is this case right now. Models are retrieved by their GUID in your applications database and a new record will be created if not found:

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/f2e2989d2f2cb2cb95e1ad7534f47cf661c3785d/src/LdapImporter.php#L118-L120

I do think this should be supported (syncing existing local database users with their LDAP model). Though I'm unsure how we could achieve it without adding an extra configuration parameter to specify the local database users username that should map to their LDAP model.

We can't really leverage the sync_attributes since it's simply an array mapping of all attributes that should sync when a model is located.

When I had to migrate my application from Adldap2-Laravel to LdapRecord-Laravel, I wrote a database migration that retrieved all database users and updated their GUID so they would be able to login. Basically:

foreach (Users::get() as $user) {
    $ldapUser = LdapUser::where('mail', '=', $user->email)->firstOrFail();

    $user->guid = $ldapUser->getConvertedGuid();

    $user->save();
}

I'm interested to hear your thoughts and how you implement this.

[ckieffer]

I see what you mean about the need for an additional config parameter. At first I thought a match against typical credential user name field names would cut it. After more thought this seems overly complicated and insecure. The only other thought is firing an event to allow developers to override and implement their own matching logic. In the end, its probably much easier to add an optional secondary user key config parameter.

I'm just beginning to implement LdapRecord with newish Laravel apps. I had evaluated AdLdap2 with Laravel but had not implemented it. I'm now working with a year-old demo app with the stock user auth and a few test accounts. None of the apps I'll develop using LdapRecord will create user if one doesn't exist already. I'd love to have a config switch to turn off this behavior but plan to add a rule as shown in your documentation (thank you).

At this point a one-time user GUID sync step seems like a reasonable migration requirement. Perhaps a migration guide for existing apps might be helpful documentation addition. Something that includes your example would go a long way :-)

[stevebauman]

In the end, its probably much easier to add an optional secondary user key config parameter.

Yea I agree, might be the easiest way. We could disable this by default so no breaking changes occur.

I'd love to have a config switch to turn off this behavior but plan to add a rule as shown in your documentation (thank you).

There's actually a built-in rule for this:

LdapRecord\Laravel\Auth\Rules\OnlyImported::class

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/master/src/Auth/Rules/OnlyImported.php

It isn't documented -- but I won't be removing it. I use it in my apps as well.

At this point a one-time user GUID sync step seems like a reasonable migration requirement. Perhaps a migration guide for existing apps might be helpful documentation addition. Something that includes your example would go a long way :-)

I agree! Documentation is everything, and I still have some gaps to fill for certain use-cases.

I'm going to keep this open while I play around with some solutions. I have no doubt that we can implement this, and I think it would be an excellent feature to have! 👍

[ckieffer]

wo0t! Thanks for pointing out your OnlyImported rule :-)

[stevebauman]

Hi @ckieffer! Besides this feature, are there any particular pain points in LdapRecord, LdapRecord-Laravel, or the documentation that you'd like to see improved? Don't be afraid to be harsh! I'm looking for any criticisms possible so it can keep improving.

Maybe there's another feature you'd like to see implemented or something that isn't explained clearly?

I'd love to know any feedback you may have! 👍

[stevebauman]

Ok we're good to go -- here's how it's configured.

We define a sync_existing array that contains key => value pairs that will be used to locate existing users in your database if they are not located by their GUID.

• The key is the database column to query
• The value is the LDAP objects attribute to search your database for

If the LDAP attribute returns null, then the actual attribute string will be used instead. This is useful for scoping the query by raw values (more on this below).

'providers' => [
    'ldap' => [
        // ...
        'database' => [
            'model' => App\User::class,
            'sync_passwords' => false,
            'sync_attributes' => [
                'cn' => 'name',
                'email' => 'mail',
            ],
            'sync_existing' => [
                'email' => 'mail',
            ],
        ],
    ]
],

In the above example, if there is an existing user in the local database with the email address sbauman@local.com with no GUID set, then the user will instead be located by their mail address in their LDAP object.

For another example, if usernames are instead used alongside multi-domain authentication, then we can add a constraint to scope the query by the auth providers LDAP domain name so the proper database record is located for that specific domain:

'sync_existing' => [
    'username' => 'samaccountname',
    'domain' => 'my-domain',
],

I'm going to write up some documentation and will close this out once complete. When that's done, I will create a new release 👍

[ckieffer]

That's awesome! Thanks. I've updated my app's user admin CRUD to include GUID assignment and it's working well. I think those migrating to LdapRecord with lots of users will really appreciate this enhancement.

As for other feedback, one item is the bindAsUser option. I've seen a few others post about this in the Laravel integration and it didn't seem possible, at least not now. Is that true? For those strictly using the integration for authentication, seems like a reasonable option to maintain.

I also think AD-specific examples along with the more general OpenLDAP options should be included in the docs. Took me a bit to adapt the provided example:

LDAP_USERNAME="cn=user,dc=local,dc=com"
LDAP_PASSWORD=secret

to the AD-specific format:

LDAP_USERNAME="samaccountname@local.com"
LDAP_PASSWORD=secret

A tabbed code panel to toggle between common examples, or side-by-side display, might be helpful for others.

Overall my experience with using LdapRecord and its documentation has been overwhelmingly positive. I can't thank you enough for your work and responsiveness.

[stevebauman]

Thanks for the kind words @ckieffer! That really means a lot to me ❤️

---

I think those migrating to LdapRecord with lots of users will really appreciate this enhancement.

Totally agree. v1.5.0 has just been released with this feature :tada:

https://ldaprecord.com/docs/laravel/auth/configuration/#database-sync-existing

---

As for other feedback, one item is the bindAsUser option. I've seen a few others post about this in the Laravel integration and it didn't seem possible, at least not now. Is that true? For those strictly using the integration for authentication, seems like a reasonable option to maintain.

This is still included as well -- it's kind of tucked away inside of the core:

https://ldaprecord.com/docs/connections/#binding

$user = 'cn=user,dc=local,dc=com';
$password = 'secret';

if ($connection->auth()->attempt($user, $password, $bindAsUser = true))
{
    echo "Username and password are correct!";

    // Run further LDAP operations under this user.
}

And you can use it inside of LdapRecord-Laravel like so:

Originally mentioned in issue: https://github.com/DirectoryTree/LdapRecord-Laravel/issues/153

// app/Http/Controllers/Auth/LoginController.php

protected function guard()
{
    $guard = Auth::guard();

    $authenticator = $guard->getProvider()->getLdapUserAuthenticator();

    $authenticator->authenticateUsing(function ($user, $password)) {
        return $user->getConnection()->auth()->attempt($user->getDn(), $password, $bindAsUser = true);
    });

    return $guard;
}

I hope that helps!

---

I also think AD-specific examples along with the more general OpenLDAP options should be included in the docs. Took me a bit to adapt the provided example:

This is actually included in the core configuration docs:

The username option must be a users Distinguished Name. If you are connecting to an Active Directory server, you may use a users userPrincipalName (username@domain.com) or Down-Level Logon Name (DOMAIN\username) instead.

https://ldaprecord.com/docs/configuration/#username-amp-password

---

A tabbed code panel to toggle between common examples, or side-by-side display, might be helpful for others.

I agree -- I'm going to see what I can do here for the future. Ease of use and thorough docs are my absolute top priority.

---

Thanks so much for your feedback!

[ckieffer]

Thanks for this. Thought about posting as a separate item but decided to post here. I've used the LdapRecord bindAsUser option on non-Laravel apps thanks to a previous pointer you provided. I'm struggling with the Laravel implementation.

And you can use it inside of LdapRecord-Laravel like so:

// app/Http/Controllers/Auth/LoginController.php

protected function guard()
{
    $guard = Auth::guard();

    $authenticator = $guard->getProvider()->getLdapUserAuthenticator();

    $authenticator->authenticateUsing(function ($user, $password)) {
        return $user->getConnection()->auth()->attempt($user->getDn(), $password, $bindAsUser = true);
    });

    return $guard;
}

Initial implementation threw:

Class 'App\Http\Controllers\Auth\Auth' not found

Per Laravel Guard Customization docs, added:

use Illuminate\Support\Facades\Auth;

After adding this I'm just getting bind errors and can't seem to figure out how to configure so that login form credentials get passed. Actually, overriding the guard may not be working.

ldap_bind(): Unable to bind to server: Invalid credentials

From my laravel.log:

[2020-06-02 14:29:06] dev.ERROR: ldap_bind(): Unable to bind to server: Invalid credentials {"exception":"[object] (LdapRecord\\Auth\\BindException(code: 2): ldap_bind(): Unable to bind to server: Invalid credentials at C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Auth\\Guard.php:126, ErrorException(code: 2): ldap_bind(): Unable to bind to server: Invalid credentials at C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Ldap.php:576)
[stacktrace]
#0 C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Auth\\Guard.php(142): LdapRecord\\Auth\\Guard->bind('cn=user,dc=loca...', 'secret')
#1 C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Connection.php(202): LdapRecord\\Auth\\Guard->bindAsConfiguredUser()
#2 C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Connection.php(259): LdapRecord\\Connection->connect()
#3 C:\\Web\\myapp.local.com\\vendor\\directorytree\\ldaprecord\\src\\Query\\Builder.php(533): LdapRecord\\Connection->run(Object(Closure))
...

I've tried hard-coding credentials into the authenticateUsing closure's attempt() and a few other items I won't bother mentioning. Obviously, I'm missing something. Here's my LoginController.php

namespace App\Http\Controllers\Auth;

use App\Http\Controllers\Controller;
use Illuminate\Foundation\Auth\AuthenticatesUsers;
use Illuminate\Support\Facades\Auth;  
use LdapRecord\Container;
use Illuminate\Http\Request;

class LoginController extends Controller
{
    use AuthenticatesUsers;

    public function __construct()
    {
        $this->connection = Container::getConnection('default');
        $this->middleware('guest')->except('logout');
    }

    protected function guard()
    {
        $guard = Auth::guard();
        $authenticator = $guard->getProvider()->getLdapUserAuthenticator();
        $authenticator->authenticateUsing(function ($user, $password) {
            return $user->getConnection()->auth()->attempt($user->getDn(), $password, $bindAsUser = true);
        });
        return $guard;
    }

    protected function credentials(Request $request)
    {
        return [
            //'samaccountname' => $request->get('username'),
            'userprincipalname' => $request->get('username') . '@local.com',
            'password' => $request->get('password'),
        ];
    }

    public function username()
    {
        return 'username';
    }
...

[stevebauman]

Hi @ckieffer,

What is your objective with this? Are you trying to bypass having an account that pre-binds to your directory to instead only bind as the user attempting to login?

I figure this, as I see this here in the exception logs:

LdapRecord\Auth\Guard->bind('cn=user,dc=loca...', 'secret')

The above username and password is the default distinguished name and password that is shipped in the published configuration file, so I'm assuming that is what is being attempted.

If that is the case -- this currently isn't supported and will only work effectively with Active Directory, since it doesn't require that you bind with a full distinguished name.

Even so, a pre-bind user is needed for a majority of LdapRecord-Laravel's features, such as importing users, eloquent model binding, single-sign-on integration and more.

If I somehow implement a way that does not require a pre-bind user, I would have to document every feature that will not work alongside it, which could cause a lot of confusion amongst other developers.

If there's a way you know of that I could integrate not requiring a pre-bind user cleanly, that'd be awesome!

[ckieffer]

I am trying to bypass having an account that pre-binds to our Active Directory. I would like to bind as the user attempting to login. On login, I do want to capture and sync a few user attributes. I am not setting up import of users or SSO. I may need to consider further the inability to do eloquent model binding.

It wasn't clear to me why the guard() override wasn't passing the login credentials for authentication binding.

Thanks for taking the time to go over this for me. I may still be able to get a devoted bind account to use but need to explore the bindAsUser method.

[stevebauman]

Ah okay I understand. For this to work, you will need to bind the connection prior to the Auth::attempt method inside of your LoginController.

Completely remove the override to the guard() method I previously mentioned, and then try overriding the attemptLogin() method instead:

// app/Http/Controllers/Auth/LoginController.php

/**
 * Attempt to log the user into the application.
 *
 * @param  \Illuminate\Http\Request  $request
 * @return bool
 */
protected function attemptLogin(Request $request)
{
    $guard = $this->guard();

    $connection = \LdapRecord\Container::getConnection('default');

    $result = $connection->auth()->attempt(
        $request->username . '@local.com', $request->password, $bindAsUser = true
    );

    if (! $result) {
        return false;
    }

    $authenticator = $guard->getProvider()->getLdapUserAuthenticator();

    $authenticator->authenticateUsing(function ($user, $password) {
        // The user is has already connected, we will simply return true here.
        return true;
    });

    return $guard->attempt(
        $this->credentials($request), $request->filled('remember')
    );
}

What we're doing is first binding to the connection as the attempting user, and then bypassing the authentication in the LdapUserAuthenticator since we already validated their credentials.

This should work since a connection is bound and their account should be located properly and be imported. If their credentials fail, we can simply return false, and they will be redirected to the login page with the 'Invalid credentials' error message.

Also I'm always happy to help! I think we can get this working the way you want it to 👍

Let me know if this works!

[ckieffer]

Thank you! Just implemented and it works beautifully. No other changes required!

[stevebauman]

Awesome @ckieffer! Glad you you’re up and running. 👍