DirectoryTree / LdapRecord-Laravel

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

Call to undefined method LdapRecord\Query\Model\ActiveDirectoryBuilder::getKeyName() #619

Closed DevHoracioRodriguez closed 6 months ago

DevHoracioRodriguez commented 6 months ago

Environment:

Description Error right after login.

Error Call to undefined method LdapRecord\Query\Model\ActiveDirectoryBuilder::getKeyName()

Bad Method Call Did you mean LdapRecord\Query\Model\ActiveDirectoryBuilder::getCacheKey() ?

The system was configured following Database Auth - Laravel Breeze

Observation Database user record was updated - only have one record for now. After login, 'updated_at' now is different from 'created_at'. Also, 'guid' & 'domain' fields now have data - not null anymore.

Flareapp log - summary

Composer

    "require": {
        "php": "^8.1",
        "bilfeldt/laravel-route-statistics": "^2.0",
        "directorytree/ldaprecord-laravel": "^3.1",
        "guzzlehttp/guzzle": "^7.2",
        "laravel/framework": "^10.8",
        "laravel/sanctum": "^3.2",
        "laravel/scout": "^10.6",
        "laravel/tinker": "^2.8",
        "livewire/livewire": "^2.12",
        "spatie/laravel-activitylog": "^4.7",
        "spatie/laravel-deleted-models": "^1.0",
        "spatie/laravel-schedule-monitor": "^3.4",
        "spatie/laravel-sluggable": "^3.5"
    },
    "require-dev": {
        "fakerphp/faker": "^1.9.1",
        "laravel/breeze": "^1.21",
        "laravel/pint": "*",
        "laravel/sail": "^1.18",
        "mockery/mockery": "^1.4.4",
        "nunomaduro/collision": "^7.0",
        "phpunit/phpunit": "^10.1",
        "spatie/laravel-ignition": "^2.0"
    },

Any ideas?

stevebauman commented 6 months ago

Hi @CyberEkklesiaOwner,

Can you post your config/auth.php? This looks like you're calling (or have placed) an LdapRecord model where an Eloquent model should be, as getKeyName() is a method on Eloquent models, but not on LdapRecord models.

DevHoracioRodriguez commented 6 months ago

Hello @stevebauman

Thank you for your quick reply. Please, see the code below:

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | Authentication Defaults
    |--------------------------------------------------------------------------
    |
    | This option controls the default authentication "guard" and password
    | reset options for your application. You may change these defaults
    | as required, but they're a perfect start for most applications.
    |
    */

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

    /*
    |--------------------------------------------------------------------------
    | Authentication Guards
    |--------------------------------------------------------------------------
    |
    | Next, you may define every authentication guard for your application.
    | Of course, a great default configuration has been defined for you
    | here which uses session storage and the Eloquent user provider.
    |
    | 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.
    |
    | Supported: "session"
    |
    */

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

    /*
    |--------------------------------------------------------------------------
    | 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"
    |
    | Added support: "ldap"
    |
    */

    'providers' => [
        'users' => [
            'driver' => 'ldap',
            'model' => LdapRecord\Models\ActiveDirectory\User::class,
            'rules' => [],
            'scopes' => [
                //App\Ldap\Scopes\DepartmentsUsers::class,
                //App\Ldap\Scopes\SchoolsUsers::class,
            ],
            'database' => [
                'model' => App\Models\User::class,
                'sync_passwords' => true,
                'sync_attributes' => [
                    'name' => 'cn',
                    'email' => 'mail',
                    'sAMAccountName' => 'sAMAccountName',
                    'givenName' => 'givenName',
                    'initials' => 'initials',
                    'middleName' => 'middleName',
                    'sn' => 'sn',
                    'title' => 'title',
                    'employeeID' => 'employeeID',
                    'distinguishedName' => 'distinguishedName',
                    'department' => 'department',
                    'departmentNumber' => 'departmentNumber',
                    'physicalDeliveryOfficeName' => 'physicalDeliveryOfficeName',
                    'telephoneNumber' => 'telephoneNumber',
                    'otherIpPhone' => 'otherIpPhone',
                    'memberOf' => 'memberOf',
                ],
                'sync_existing' => [
                    'email' => 'mail',
                ],
            ],
        ],
        // 'users' => [
        //     'driver' => 'database',
        //     'table' => 'users',
        // ],
    ],

    /*
    |--------------------------------------------------------------------------
    | Resetting Passwords
    |--------------------------------------------------------------------------
    |
    | You may specify multiple password reset configurations if you have more
    | than one user table or model in the application and you want to have
    | separate password reset settings based on the specific user types.
    |
    | The expiry time is the number of minutes that each reset token will be
    | considered valid. This security feature keeps tokens short-lived so
    | they have less time to be guessed. You may change this as needed.
    |
    | The throttle setting is the number of seconds a user must wait before
    | generating more password reset tokens. This prevents the user from
    | quickly generating a very large amount of password reset tokens.
    |
    */

    'passwords' => [
        'users' => [
            'provider' => 'users',
            'table' => 'password_reset_tokens',
            'expire' => 60,
            'throttle' => 60,
        ],
    ],

    /*
    |--------------------------------------------------------------------------
    | Password Confirmation Timeout
    |--------------------------------------------------------------------------
    |
    | Here you may define the amount of seconds before a password confirmation
    | times out and the user is prompted to re-enter their password via the
    | confirmation screen. By default, the timeout lasts for three hours.
    |
    */

    'password_timeout' => 10800,

];

In case you wonder, here is some more code:

Scope:

<?php

namespace App\Ldap\Scopes;

use LdapRecord\Models\Model;
use LdapRecord\Models\Scope;
use LdapRecord\Query\Model\Builder;

class Users implements Scope
{
    /**
     * Apply the scope to the given query.
     */
    public function apply(Builder $query, Model $model): void
    {
        $query->in('OU=Users,OU=Information Technology Services,OU=Departments,{base}');
    }
}

User Model:

<?php

namespace App\Models;

// use Illuminate\Contracts\Auth\MustVerifyEmail;
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\LdapAuthenticatable;
use LdapRecord\Laravel\Auth\HasLdapUser;
use LdapRecord\Laravel\Auth\AuthenticatesWithLdap;

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

    /**
     * The attributes that are mass assignable.
     *
     * @var array<int, string>
     */
    protected $fillable = [
        'name',
        'email',
        'password',
    ];

    /**
     * The attributes that should be hidden for serialization.
     *
     * @var array<int, string>
     */
    protected $hidden = [
        'password',
        'remember_token',
    ];

    /**
     * The attributes that should be cast.
     *
     * @var array<string, string>
     */
    protected $casts = [
        'email_verified_at' => 'datetime',
    ];
}

Login Request:

<?php

namespace App\Http\Requests\Auth;

use Illuminate\Auth\Events\Lockout;
use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\RateLimiter;
use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;

class LoginRequest extends FormRequest
{
    /**
     * Determine if the user is authorized to make this request.
     */
    public function authorize(): bool
    {
        return true;
    }

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array<string, \Illuminate\Contracts\Validation\Rule|array|string>
     */
    public function rules(): array
    {
        return [
            'email' => ['required', 'string', 'email'],
            'password' => ['required', 'string'],
        ];
    }

    /**
     * Attempt to authenticate the request's credentials.
     *
     * @throws \Illuminate\Validation\ValidationException
     */
    public function authenticate(): void
    {
        $this->ensureIsNotRateLimited();

        $credentials = [
            'mail' => $this->email,
            'password' => $this->password,
            'fallback' => [
                'email' => $this->email,
                'password' => $this->password,
            ],
        ];

        if (! Auth::attempt($credentials, $this->filled('remember'))) {
            RateLimiter::hit($this->throttleKey());

            throw ValidationException::withMessages([
                'email' => __('auth.failed'),
            ]);
        }

        RateLimiter::clear($this->throttleKey());
    }

    /**
     * Ensure the login request is not rate limited.
     *
     * @throws \Illuminate\Validation\ValidationException
     */
    public function ensureIsNotRateLimited(): void
    {
        if (! RateLimiter::tooManyAttempts($this->throttleKey(), 5)) {
            return;
        }

        event(new Lockout($this));

        $seconds = RateLimiter::availableIn($this->throttleKey());

        throw ValidationException::withMessages([
            'email' => trans('auth.throttle', [
                'seconds' => $seconds,
                'minutes' => ceil($seconds / 60),
            ]),
        ]);
    }

    /**
     * Get the rate limiting throttle key for the request.
     */
    public function throttleKey(): string
    {
        return Str::transliterate(Str::lower($this->input('email')).'|'.$this->ip());
    }
}
DevHoracioRodriguez commented 6 months ago

Hello @stevebauman

Thank you for your previous reply. Based on the code you requested (see my previous post - full of code), Can you point out any direction to solve the error? So far, the import works as expected. Even two import processes are scheduled. Please look below the code and results.

Kernel:


        // Common options for both imports
        $options = [
            '--no-interaction',
            '--chunk' => '100 ',
            '--filter' =>'(&(objectClass=user)(objectCategory=person))',
            '--attributes' =>'cn,mail,sAMAccountName,givenName,initials,middlename,sn,title,employeeID,distinguishedName,department,departmentNumber,physicalDeliveryOfficeName,telephoneNumber,otherIpPhone,memberOf',
        ];

        // AD Import - Departments Users
        $schedule->command('ldap:import users', array_merge(['--scopes' => 'App\Ldap\Scopes\DepartmentsUsers::class'], $options))
            //->appendOutputTo(storage_path('logs/ldap_departments_users.log'))
            ->everyMinute()
            ->name("LDAP Departments Users") // Name the scheduled task.
            ->withoutOverlapping() // to prevent a scheduled task from running concurrently while a previous instance of the same task is still running.
            ->runInBackground() // scheduled task is forked into a separate process, allowing the main application or other scheduled tasks to continue running concurrently.
            ->timezone('America/New_York');

        // AD Import - Schools Users
        $schedule->command('ldap:import users', array_merge(['--scopes'=>'App\Ldap\Scopes\SchoolsUsers::class'], $options))
            //->appendOutputTo(storage_path('logs/ldap_schools_users.log'))
            ->everyMinute()
            ->name("LDAP Schools Users") // Name the scheduled task.
            ->withoutOverlapping() // to prevent a scheduled task from running concurrently while a previous instance of the same task is still running.
            ->runInBackground() // scheduled task is forked into a separate process, allowing the main application or other scheduled tasks to continue running concurrently.
            ->timezone('America/New_York');

Schedule:


  2024-01-16 13:21:01 Running ["artisan" ldap:import users --scopes="App\Ldap\Scopes\DepartmentsUsers::class" --no-interaction --chunk=100  --filter="(&(objectClass=user)(objectCategory=person))" --attributes="cn,mail,sAMAccountName,givenName,initials,middlename,sn,title,employeeID,distinguishedName,department,departmentNumber,physicalDeliveryOfficeName,telephoneNumber,otherIpPhone,memberOf"] in background  253ms DONE
  ⇂ LDAP Departments Users

  2024-01-16 13:21:01 Running ["artisan" ldap:import users --scopes="App\Ldap\Scopes\SchoolsUsers::class" --no-interaction --chunk=100  --filter="(&(objectClass=user)(objectCategory=person))" --attributes="cn,mail,sAMAccountName,givenName,initials,middlename,sn,title,employeeID,distinguishedName,department,departmentNumber,physicalDeliveryOfficeName,telephoneNumber,otherIpPhone,memberOf"] in background  225ms DONE
  ⇂ LDAP Schools Users

Results: Successful DB Update.

Again, let me know if there is a way to solve the problem.

Thank you for your help and valuable time.

DevHoracioRodriguez commented 6 months ago

Well, after all, I think that has to do something with a hard-coded value.

Similar to this issue and others.

I think you get it...

Update

Yeap! After I removed 'Laravel Route Statistics' the error went away. Now, from other posts of yours, your package seems to be in conflict with other solutions - or others with yours. So, What would be a recommend workaround to avoid this trouble? Note: I need to install back 'Laravel Route Statistics' but also, might need to install others with similar issues like spatie-permission, nova and others.

stevebauman commented 6 months ago

Hi @CyberEkklesiaOwner,

In the case of the Laravel Route Statistics package you mention, it looks like you can alter the configuration to utilize your own model, which you can then extend their built-in model, but override the user() relationship method:

https://github.com/bilfeldt/laravel-route-statistics/blob/534d1fb68ff178221efaedf62f9eabcecd551cbc/config/route-statistics.php#L37

namespace App\Models;

use Bilfeldt\LaravelRouteStatistics\Models\RouteStatistic as BaseModel;

class RouteStatistic extends BaseModel
{
    public function user(): BelongsTo
    {
        return $this->belongsTo(\App\Models\User::class);
    }

In regards to Spatie Permission, I'd imagine you'd need to do the same, but I can't assist with these packages further. It is out of scope of this package.

Some packages may be incompatible with LdapRecord-Laravel if they hard-code user model retrieval to the auth config and don't provide a way to alter this logic or behaviour.

Closing, as the issue is unrelated to LdapRecord-Laravel.

DevHoracioRodriguez commented 5 months ago

Hello @stevebauman

Thank you for your help. Indeed your recommendation helped me to solve the problem - and for sure to future packages. In my case I used Custom Models.

  1. Added CustomModel class on ..app/config/app.providers.
  2. Created a Service Provider to override the binding. Example: .app/Http/Providers/CustomModelServiceProvider.php
  3. Created a Custom Model ../app/Http/Models/CustomModel.php (a copy of the vendor's model with my changes - see below)
return $this->belongsTo(config('auth.providers.users.database.model')); <-- database added to point User::class

I hope this is useful to anybody.

DevHoracioRodriguez commented 5 months ago

In regards to Spatie Permission, I'd imagine you'd need to do the same, but I can't assist with these packages further. It is out of scope of this package.

Some packages may be incompatible with LdapRecord-Laravel if they hard-code user model retrieval to the auth config and don't provide a way to alter this logic or behaviour.

Hello @stevebauman

This might not be an issue anymore for Laravel-Permission. After getting LdapRecord nicely working importing from Active Directory and integrated with Laravel Breeze, an installation of laravel-permission didn't have any errors at all. Their code is a bit different now.

So, in my experience, both are fully integrated with no additional coding to make them work together.

P.S: This might apply to other issues previously reported like #101, #174, #480, #150, and probably many others... Maybe, you might want to publish compatibility issue solved with Laravel-Permission or perhaps ask the community for an updated integration test?

Enzo-PVsyst commented 3 months ago

Hello @CyberEkklesiaOwner

I'm working on a project that used to work on Laravel 10 with this package on version 2.X and laravel permissions on 5.X.

I'm facing the same issue than the title of this feed since I'm trying to upgrade to laravel 11.

I needed to update to latest version of those two packages in order to be compatible with laravel & php.

Now when I run my migrations (that previously worked), it fails on this line : Permission::findByName('ACCESS_PROJECTS_PAGE')->delete();

Do you have any idea of what could I do to have those packages workings together ?

I can still loging with LDAP with no issue and I have my permissions working well.