Ldaprecord-laravel: [Bug] LdapUserModel is null when calling a user

[fluxX04]

Environment (please complete the following information):

• LDAP Server Type: ActiveDirectory
• LdapRecord-Laravel Major Version: v2 (latest)
• PHP Version: 7.3
• Laravel: 8

Describe the bug:
Hey,

i'm currently testing the ldap authentication over a laravel api (sanctum). The authentication over ldap, sanctum and the sync of the attributes works but when calling the ldap property on the user model the value is null.

My UserModel:

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
use Laravel\Sanctum\HasApiTokens;
use LdapRecord\Laravel\Auth\AuthenticatesWithLdap;
use LdapRecord\Laravel\Auth\HasLdapUser;
use LdapRecord\Laravel\Auth\LdapAuthenticatable;

class User extends Authenticatable implements LdapAuthenticatable
{
    use HasFactory, Notifiable, HasApiTokens, AuthenticatesWithLdap, HasLdapUser;

    ...
}

My auth.php

<?php

return [
    'defaults' => [
        'guard' => 'web',
        'passwords' => 'users',
    ],

    'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],
    ],

    'providers' => [
        // 'users' => [
        //     'driver' => 'eloquent',
        //     'model' => App\Models\User::class,
        // ],

        'users' => [
            'driver' => 'ldap',
            'model' => LdapRecord\Models\ActiveDirectory\User::class,
            'rules' => [
                App\Ldap\Rules\OnlyCompanyRule::class,
            ],
            'database' => [
                'model' => App\Models\User::class,
                'sync_passwords' => true,
                'sync_attributes' => [
                    'firstname' => 'givenname',
                    'lastname' => 'sn',
                    'email' => 'mail',
                    'username' => 'samaccountname'
                ],
                'password_column' => 'password',
            ],
        ],

        // 'users' => [
        //     'driver' => 'database',
        //     'table' => 'users',
        // ],
    ],
];

My test route:

Route::group(['middleware' => ['auth:sanctum']], function () {
    Route::get('/me', function (Request $request) {
        dd($request->user(), $request->user()->ldap);
    });
});

Did i miss something?
Thanks!

Greetz

[stevebauman]

Hi @fluxX04,

Are you able to do a deep debugging dive and see why one of these is returning null?

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/781c7854cf4dee3c2c86b6a0649694482c8f5cdd/src/Auth/HasLdapUser.php#L24-L47

[fluxX04]

Hi @stevebauman!

thanks for the replay - yes sure.

The $guard value is "sanctum" and when calling getCurrentAuthProvider it returns.

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/781c7854cf4dee3c2c86b6a0649694482c8f5cdd/src/Auth/CreatesUserProvider.php#L30-L35

Because the functions looks for "auth.guards.sanctum.provider" and this is not defined in the auth.php file.
When i overwrite the $guard to web i get the ldap model.

Greetz

[stevebauman]

Thanks for the debugging this @fluxX04! I understand the issue. I think we may have to leverage the sanctum config to retrieve the users guard, as sanctum does in its guard:

https://github.com/laravel/sanctum/blob/2.x/src/Guard.php#L55

Give me a couple days and I should have this patched. Though feel free to submit a PR if you require it patched immediately 👍

[manuprieto]

Hi!

I am trying to get connected with simple Laravel UI and I am getting same result as @fluxX04

Attempt returns true but when trying to get the ldap property of User it returns null.

I think it might be same bug... Hope this can help too.

My User Model:

class User extends Authenticatable implements JWTSubject, LdapAuthenticatable
{
    use HasApiTokens, Notifiable, AuthenticatesWithLdap, HasLdapUser;`

This is my auth config:

'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'users',
        ],

        'api' => [
            'driver' => 'jwt',
            'provider' => 'users',
        ],

        /*'api' => [
            'driver' => 'token',
            'provider' => 'users',
            'hash' => false,
        ], */
    ],

    /*
    |--------------------------------------------------------------------------
    | User Providers
    |--------------------------------------------------------------------------
    |
    | All authentication drivers have a user provider. This defines how the
    | users are actually retrieved out of your database or other storage
    | mechanisms used by this application to persist your user's data.
    |
    | If you have multiple user tables or models you may configure multiple
    | sources which represent each model / table. These sources may then
    | be assigned to any extra authentication guards you have defined.
    |
    | Supported: "database", "eloquent"
    |
    */

    'providers' => [
        'users' => [
            'driver' => 'ldap',
            'model' => LdapRecord\Models\OpenLDAP\User::class,
            'rules' => [],
            'database' => [
                'model' => App\User::class,
                'sync_passwords' => false,
                'sync_attributes' => [
                    'username' => 'uid',
                ],
                'sync_existing' => [
                    'username' => 'uid',
                ],
            ],
        ],

        // 'users' => [
        //     'driver' => 'database',
        //     'table' => 'users',
        // ],
    ],

Result of Auth::user():

App\User {#1584 ▼
  #fillable: array:10 [▶]
  #hidden: array:2 [▶]
  #casts: array:1 [▶]
  #connection: "inventario"
  #table: "users"
  #primaryKey: "id"
  #keyType: "int"
  +incrementing: true
  #with: []
  #withCount: []
  #perPage: 15
  +exists: true
  +wasRecentlyCreated: false
  #attributes: array:24 [▶]
  #original: array:24 [▶]
  #changes: []
  #classCastCache: []
  #dates: []
  #dateFormat: null
  #appends: []
  #dispatchesEvents: []
  #observables: []
  #relations: []
  #touches: []
  +timestamps: true
  #visible: []
  #guarded: array:1 [▶]
  #rememberTokenName: "remember_token"
  #accessToken: null
  #ldapUserModel: null

[CmdrSharp]

I should add that I believe to also be affected.

public function ldapGroups()
{
    return Cache::remember("user_{$this->attributes['id']}_groups", 3600, function () {
        return $this->ldap->groups()->get();
    });
}

I have this method in my User Model which caches the user groups from LDAP. This intermittently errors due to groups() being called on null. When it does, if I log onto the site manually so that it re-binds, it starts working again.

[stevebauman]

Hi @manuprieto, are you authenticating via your api guard, or your web guard?

[stevebauman]

Hi @CmdrSharp, are you using Laravel Sanctum as well?

[CmdrSharp]

Hi @CmdrSharp, are you using Laravel Sanctum as well?

HI @stevebauman! :) Yes, it's a Jetstream project using Sanctum.

[stevebauman]

Ah okay excellent, thanks for the super quick response @CmdrSharp! 😄

I'm going to push a fix that should resolve this right now. Whenever you're free, could you give it a shot by updating to the dev-master branch and ensure it's working for you?

[CmdrSharp]

Ah okay excellent, thanks for the super quick response @CmdrSharp! 😄

I'm going to push a fix that should resolve this right now. Whenever you're free, could you give it a shot by updating to the dev-master branch and ensure it's working for you?

Wilco! I've had some bad luck trying to reproduce the issue reliably but I think I can figure that out. Technically off work now but I'll hop on and give it a go shortly.
Thanks for your awesome work and inspiring attitude as always :) I'll report back.

[stevebauman]

Thanks so much for your kind words! I really appreciate that 😄 Sounds good!

In the meantime I'm going to try and see if I can write a test case for this 👍

[CmdrSharp]

Thanks so much for your kind words! I really appreciate that 😄 Sounds good!

In the meantime I'm going to try and see if I can write a test case for this 👍

@stevebauman I've done some testing and unfortunately the issue seems to persist. A simple route like this still shows that the ldap property is null:

Route::group(['middleware' => ['auth:sanctum']], function () {
    Route::get('/me', function (Request $request) {
        return is_null($request->user()->ldap);
    });
});

Now I did a tiny bit of debugging and for me, it doesn't seem to hit the new case.

        switch ($guard) {
            case 'sanctum':
                Log::info("Hit the {$guard} case");
                return $this->resolveAuthenticatedGuard(
                    Arr::wrap(config('sanctum.guard', 'web'))
                );
            default:
                Log::info("Hit the {$guard} case");
                return $guard;
        }

[2021-04-17 22:13:21] local.INFO: Hit the api case
[2021-04-17 22:13:21] local.ERROR: Call to a member function groups() on null {"userId":1,"exception":"[object] (Error(code: 0): Call to a member function groups() on null at /path/to/project/app/Models/User.php:97)
[stacktrace]
[2021-04-17 22:13:21] local.INFO: Hit the api case
[2021-04-17 22:13:21] local.INFO: Hit the api case
[2021-04-17 22:13:21] local.INFO: Hit the api case

Hopefully this could be a clue? If not, let me know if there's anything else I can do to help :)

[stevebauman]

Hmm -- this is going to be tricky. Since the authentication guard is going to be sanctum and not the ldap guard, we somehow have to resolve that ldap guard from the user instance itself, to be able to fetch the users matching LDAP object.

Due to that, I'm not entirely sure if this is going to be possible, since this feature directly relies on the user authenticating on the defined ldap guard itself. We could retrieve the configured sanctum.guards, but we will have to loop through each of them and attempt to find the users matching LDAP object. I'm not against implementing that, but I'm wondering if there's some other way I may not be seeing.

Do you have any ideas @CmdrSharp?

[CmdrSharp]

@stevebauman I have to admit my knowledge of this part of the framework isn't up to par anymore, so don't laugh at me if this is silly ;) My initial question is whether it's possible to do something in the HasLdapUser trait. It seems in my testing we get an instance of Illuminate\Auth\EloquentUserProvider when this issue occurs, Would it be possible to programmatically instantiate a UserProvider here?

[stevebauman]

Oh no worries! I'm working through this as well, so we're on the same page lol

It seems in my testing we get an instance of Illuminate\Auth\EloquentUserProvider when this issue occurs, Would it be possible to programmatically instantiate a UserProvider here?

I don't think we would want an instance of the EloquentUserProvider. I think we would need an instance of the LdapRecord DatabaseUserProvider to be able to retrieve the LdapUserRepository which contains the providers configured model to pull LDAP objects from:

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/927c61cd6c2c1116637610ca2366a40f429fa98e/src/Auth/HasLdapUser.php#L42-L44

I think the only way we can utilize this feature with Sanctum is to leverage its guard configuration option, manually instantiate the guard, create the guards user provider, then attempt fetching the user.

Though if multiple Sanctum guards are configured, we would need to loop through each Sanctum guard, instantiate it, create the guards user provider, attempt fetching the user, and then move onto the next if none is found.

This is potentially an "expensive" operation, but it's the only option I see for this...

[CmdrSharp]

@stevebauman Oh no mean right now we _do_ get a EloquentUserProvider when the issue is occurring. Thus the question if we could try to catch that and instead instantiate a UserProvider.

As for the option to loop over the guards - do you have any rough idea of how common it is to define multiple Sanctum guards? I'm trying to think of a reason for doing so.

[stevebauman]

Ohh okay I understand. Well -- what configuration would we supply to the UserProvider? How would we figure out the config path for the applications ldap guard? How would we resolve the LdapRecord model to use for locating the users LDAP object?

As for the option to loop over the guards - do you have any rough idea of how common it is to define multiple Sanctum guards? I'm trying to think of a reason for doing so.

This feature was PR'ed into Sanctum v2, so we'd have to support it, if we're going to support Sanctum:

https://github.com/laravel/sanctum/pull/246

[CmdrSharp]

Ohh okay I understand. Well -- what configuration would we supply to the UserProvider? How would we figure out the config path for the applications ldap guard? How would we resolve the LdapRecord model to use for locating the users LDAP object?

If I had the answers to that, I'd have a PR for you ;) That's the reason I formulated it more as a question than as an answer. I figured you might know better what data is available at that point in the execution.

This feature was PR'ed into Sanctum v2, so we'd have to support it, if we're going to support Sanctum:
laravel/sanctum#246

Ah, yes - sorry, I wasn't clear. Supporting it is a no-brainer, but I'm trying to think of how common the use case for adding multiple Sanctum guards in projects is to gauge how it'd affect users.

You said it's a potentially expensive operation - but it'd only be expensive while it still has to figure out which guard is relevant. Could caching somehow mitigate the "expensiveness" of the operation so that it at least doesn't have to continue being expensive once we already know?

[stevebauman]

If I had the answers to that, I'd have a PR for you ;) That's the reason I formulated it more as a question than as an answer. I figured you might know better what data is available at that point in the execution.

Haha fair enough!

You said it's a potentially expensive operation - but it'd only be expensive while it still has to figure out which guard is relevant. Could caching somehow mitigate the "expensiveness" of the operation so that it at least doesn't have to continue being expensive once we already know?

I think I'm being a little over-zealous with the "expensive" claim -- we'd really only be iterating through each guard, creating their user provider, and attempting an LDAP query if the user provider is an instance of the LdapRecord DatabaseUserProvider.

To know your use-case better, you mentioned in an earlier comment that the recent update wasn't hitting the new switch case for the $guard variable. When you visit that route that you created which is covered by the sanctum guard middleware, does this not return true?

Route::group(['middleware' => ['auth:sanctum']], function () {
    Route::get('/me', function (Request $request) {
        dd(auth('sanctum')->check());
    });
});

[CmdrSharp]

I think I'm being a little over-zealous with the "expensive" claim -- we'd really only be iterating through each guard, creating their user provider, and attempting an LDAP query if the user provider is an instance of the LdapRecord DatabaseUserProvider.

That's fair! Doesn't sound too bad to me. Though I completely understand that trial and error never feels like a proper solution :)

To know your use-case better, you mentioned in an earlier comment that the recent update wasn't hitting the new switch case for the $guard variable. When you visit that route that you created which is covered by the sanctum guard middleware, does this not return true?

Route::group(['middleware' => ['auth:sanctum']], function () {
    Route::get('/me', function (Request $request) {
        dd(auth('sanctum')->check());
    });
});

It does return true.

[manuprieto]

Hi @manuprieto, are you authenticating via your api guard, or your web guard?

Sorry being late. I am using web guard. Thank you for your help ;)

[stevebauman]

Happy to help @manuprieto!

@CmdrSharp

That's fair! Doesn't sound too bad to me. Though I completely understand that trial and error never feels like a proper solution :)

Yea exactly -- that's really my only concern.

I'm going to do some digging & testing later today (and this week) with Sanctum so I can have this resolved for you guys. Hoping that we can have this feature work without any configuration on your end 👍

[stevebauman]

Ok -- seemed to be an easy fix. I decided not to support an array of sanctum guards, as this added unnecessary complexity.

I tested this on my own Jetstream installation with LdapRecord-Laravel, and was able to hit the pre-defined API endpoint covered by the auth:sanctum middleware, and successfully return the users LDAP object:

// routes/api.php

Route::middleware('auth:sanctum')->get('/user', function (Request $request) {
    return $request->user()->ldap;
});

Can you do a pull on dev-master and give it a shot? Let me know if you run into any issues!

[CmdrSharp]

@stevebauman Unfortunately didn't solve it for me. I ran a short debug and the guard resolved in getCurrentAuthProvider seems to be api

It's worth keeping in mind that I have this method in my User model.

/**
 * @return mixed
 */
public function ldapGroups()
{
    return Cache::rememberForever("user_{$this->attributes['id']}_groups", function () {
        return $this->ldap->groups()->get();
    });
}

This works as long as I browse onto the site (sanctum covers our web guards too as recommended by Sanctum) first. Normally I'd have this cache invalidate after an hour, but I've temporarily set it to cache for longer to mitigate the issue.
Anyway - if the cache isn't present, and I hit a sanctum covered API route - $this->ldap is null. If I then browse onto the site (thus going via the web routes covered by sanctum), the cache gets created and it starts working.

Hopefully that sheds light on at least my case. If not, let me know and I'll try to explain it better! :)

[stevebauman]

Hmmm — how are you hitting your API endpoint? Through a web request, or via AJAX? Also, have you changed your configured sanctum guard?

I used a fresh Laravel Jetstream install.

[CmdrSharp]

@stevebauman Updated my previous reply, hopefully that sheds extra light! Just throwing an example GET request at the API endpoint via Postman :)

    'guards' => [
        'web' => [
            'driver' => 'session',
            'provider' => 'ldap',
        ],

        'api' => [
            'driver' => 'sanctum',
            'provider' => 'users',
            'hash' => false,
        ],
    ],

[stevebauman]

Ohhh I see — you’re using sanctum as the guard driver — not the guard itself.

Yeah I don’t think this will be possible to incorporate this specific use-case generally for everyone. Your API provider should be set to ldap to be able to resolve the user from the proper LdapRecord DatabaseUserProvider — otherwise there’s no way to retrieve the user at all. It can’t be resolved any other way.

Unless anyone else has any thoughts, I would probably override the HasLdapUser trait method and return the web guard manually for all users.

[CmdrSharp]

@stevebauman Gosh darnit. That of course solved it immediately. I never thought to change it initially, as I wasn't doing stuff like this on the user model to start with - and as such, I never needed to go beyond the users provider for API calls.

Hopefully the original issue here was resolved anyway. I'll just crawl away with my tail between my legs ;)

[CmdrSharp]

@stevebauman Ooh, actually - I might have tested that previously (I thought I might have but wasn't sure), but without your latest commit, it still won't work. So yes, problem solved for me with this latest commit! :)

[stevebauman]

Haha no worries @CmdrSharp! I'm so glad that you're up and running now! 🎉

Apologies for the late reply.

I've just released v2.3.0 that contains this patch. Run composer update and you're good to go!