Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Auth password_sync not working if Eloquent user model has defined password method as Attribute with hashing #677

Open
Panoptik opened this issue Nov 15, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@Panoptik
Copy link

Environment:

  • LDAP Server Type: ActiveDirectory
  • LdapRecord-Laravel Major Version: v3 (v3.3.5)
  • PHP Version: 8.3
    Describe the bug:

I have configured auth with ldap using next config [auth.php]:

return [
    // ...
    'providers' => 
         // ...

         // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration
        'ldap' => [
            'driver' => 'ldap',
            'model' => App\Ldap\Models\User::class,
            // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration#rules
            // rules applied after successful login performed and allow to make additional post-login check
            'rules' => [],
            // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration#scopes
            // scopes applied during call as additional query conditions (used in import and authentication)
            'scopes' => [
                UserScope::class,
            ],
            'database' => [
                'model' => App\Models\User::class,
                // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration#sync-passwords
                'sync_passwords' => true,
                // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration#sync-attributes
                'sync_attributes' => [
                    'email' => 'mail',
                    AttributeHandler::class,
                ],
                // https://ldaprecord.com/docs/laravel/v3/auth/database/configuration#sync-existing-records
                'sync_existing' => [
                    'email' => [
                        'attribute' => 'mail',
                        'operator' => 'ilike',
                    ],
                ],
            ],
        ],
];

In my /app/Models/User is declared password attribute

class User extends Authenticatable implements LdapAuthenticatable
{
    use AuthenticatesWithLdap;

    // ...

    public function password(): \Illuminate\Database\Eloquent\Casts\Attribute
    {
        return \Illuminate\Database\Eloquent\Casts\Attribute::make(set: fn ($value) => bcrypt($value));
    }

Problem:
While I perform successfull login with LDAP: User successfully created and synced, however password rehashed twice:

  • first time: directlry within \LdapRecord\Laravel\Import\Hydrators\PasswordHydrator::hydrate
  • second time this hashed password comes into User::password() method call from next PasswordHydrator's code
class PasswordHydrator extends Hydrator
{
  // ...
  
    /**
     * Set the password on the users model.
     */
    protected function setPassword(EloquentModel $model, string $column, string $password): void
    {
        // If the model has a mutator for the password field, we
        // can assume hashing passwords is taken care of.
        // Otherwise, we will hash it normally.
        $password = $model->hasSetMutator($column)
            ? $password
            : Hash::make($password);

         // this part forced to call $user->password       <----
        $model->setAttribute($column, $password);
    }
}

After hard debugging during an hour I found this place in code and that 3 lines with comments.
Unfortunatelly this is not so obvious and not described in docs.

I had resolved my issue with updating mentioned password() Atrribute method, however I'd like to have some extended configuration to be able manage it without changing code.

Suggestion:
to add additional flag 'prevent_password_hash' => true and manage it with mentioned PasswordHydrator's method setPassword

class PasswordHydrator extends Hydrator
{
  // ...
  
    /**
     * Set the password on the users model.
     */
    protected function setPassword(EloquentModel $model, string $column, string $password): void
    {
        // If the model has a mutator for the password field, we
        // can assume hashing passwords is taken care of.
        // Otherwise, we will hash it normally.
        $password = ($model->hasSetMutator($column) || $this->getIsPreventPasswordHash())
            ? $password
            : Hash::make($password);

        $model->setAttribute($column, $password);
    }
}

This issue created with intention to improve documentation or user experience. It not fully belongs to library, but at the same time having such ability/documentation could help in some cases
Thanks in advance for answer

@Panoptik Panoptik added the bug Something isn't working label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant