DirectoryTree / LdapRecord-Laravel

Multi-domain LDAP Authentication & Management for Laravel.
https://ldaprecord.com/docs/laravel/v3
MIT License
512 stars 54 forks source link

[Support] PHPStan reports Auth::getUser() to return the Ldap User instead of database model #659

Closed bertoost closed 6 months ago

bertoost commented 6 months ago

When I configure Laravel as per documentation, with database fallback support, like this;

'providers' => [
    'users' => [
        'driver' => 'ldap',
        'model' => LdapRecord\Models\ActiveDirectory\User::class,
        // ...
        'database' => [
            'model' => App\Models\User::class,
            // ...
        ],
    ],
],

PHPStan thinks that Auth::getUser() returns an instance of LdapRecord\Models\ActiveDirectory\User::class which is actually not the case and just our own model.

How can we solve this?

Regards, Bert

Environment:

stevebauman commented 6 months ago

Hey @bertoost,

I don't use PHPStan so I personally have no idea.

This should probably be reported on the PHPStan repo, as we can't control how it interprets the Laravel auth config files.

bertoost commented 5 months ago

Hi @stevebauman That's a bit to easy unfortunately.

Laravel (and thus phpstan, with larastan) expects configuration auth.providers.users.model to be the local model file. So, phpstan also expects that. Therefore, this package (which works great), should respect that also. When using the database fallback option, the model config key should be set to the local model and an additional configuration key should be added for the Ldap model to use.

Example;

'providers' => [
    'users' => [
        'driver' => 'ldap',
        'model' => App\Models\User::class,
        // ...
        'database' => [
            'ldapModel' => LdapRecord\Models\ActiveDirectory\User::class,
            // ...
        ],
    ],
],

Therefore the ldapModel could also live on a level above the database fallback configuration.

stevebauman commented 5 months ago

This has been suggested before, but it doesn't make sense to structure the configuration in this way. Why would configuration of the LdapRecord model reside in the "database" key? LdapRecord provides an LDAP authentication driver first, and a database layer second.

I understand this would make your specific use-case satisfy PHPStan, but I will personally be accountable for documenting this, as well as assisting other developers troubleshoot this in the future.

Auth driver configuration cannot be beholden to a single structure as they vary by implementation.

ruthgeridema commented 2 months ago

Please consider reopening this @stevebauman , encountering the same

stevebauman commented 2 months ago

Hi @ruthgeridema, please see past comments and suggest a fix or submit a PR.