[Bug] password not encoded if using custom model

[vatsake]

Environment:

• LDAP Server Type: ActiveDirectory
• PHP Version:8.0

Describe the bug:

For some reason $user->unicodepwd is not automatically encoded IF I'm using custom model.

use LdapRecord\Models\Model;

class User extends Model
{
    /**
     * The object classes of the LDAP model.
     *
     * @var array
     */
    public static $objectClasses = [
        'top',
        'person',
        'organizationalperson',
        'user',
    ];

    protected $casts = [
        'accountexpires' => 'datetime:windows-int',
    ];
}

 $user = (new User());
$user->unicodepwd = $request->input('password');
dd($user->getModifications());

array:3 [
    "attrib" => "unicodepwd"
    "modtype" => 1
    "values" => array:1 [
      0 => "testpassword"
    ]

[stevebauman]

Hi @vatsake,

If you create your own custom User Active Directory model, you should either extend the built-in ActiveDirectory\User model, or implement the interface and traits that currently exist on that model, namely the HasPassword trait:

https://github.com/DirectoryTree/LdapRecord/blob/8ad10c26a992b46a265ff84b2d1845003c910a6e/src/Models/ActiveDirectory/User.php#L14-L32

Custom User Model:

use LdapRecord\Models\Model;
use LdapRecord\Models\Concerns\HasPassword;
use LdapRecord\Models\Types\ActiveDirectory;

class User extends Model implements ActiveDirectory
{
    use HasPassword;

    protected $passwordHashMethod = 'encode';

    protected $passwordAttribute = 'unicodepwd';

    // ...
}

I'd recommend extending the default User model if you can, as it provides a lot of scaffolding out-of-the-box:

Extending Default User Model:

use LdapRecord\Models\ActiveDirectory\User as BaseModel;

class User extends BaseModel
{
    // ...
}

This will resolve your password encoding issue 👍

[vatsake]

Extending from LdapRecord\User encodes password, but for some reason I get; ldap_modify_batch(): Batch Modify: Server is unwilling to perform

If i encode manually and extend just 'model', it works...

Code:

$user = new User();

        $user->givenName = $request->input('givenName');
        $user->sn = $request->input('sn');
        $user->displayName = $request->input('givenName') . ' ' . $request->input('sn');
        $user->cn = $request->input('givenName') . ' ' . $request->input('sn');
        $user->samaccountname = $request->input('samAccountName');
        $user->userPrincipalName = $request->input('samAccountName') . '@domain.com';
        $user->unicodepwd = $request->input('password');
        $user->pwdlastset = -1;
        .
        .
        .
        try {
            $user->save();
            $user->refresh();
        } catch (AlreadyExistsException $e) {
            // RIP
        } catch (ConstraintViolationException $e) {
           // RIP
        } catch (Exception $e) {
           // RIP
        }

        $user->update(['userAccountControl' => 512, 'pwdlastset' => 0]); 

[stevebauman]

Hi @vatsake,

I don't believe you can set a password during creation of an ActiveDirectory account -- it must be set after the account exists. Can you confirm if setting the password using your own model actually creates the user with the password you've provided?

Also, can you try re-extending the base User model and attempt setting the password after creation?

I.e.

$user = new User();

// ...

$user->save();

$user->update([
    'unicodepwd' => $request->input('password'),
    'userAccountControl' => 512,
    'pwdlastset' => 0,
]); 

[vatsake]

Hey

Yes, the password was set during account creation (just tried, logged in and such).

Yeah I also remember reading that password has to be set AFTER account creation. Okay, with extending LdapRecord User model, the password was set correctly after account creation...

[stevebauman]

Thanks for confirming that @vatsake,

I think this is a bug. Here in LdapRecord we're making the assumption that an existing password will be overwritten via a batch REPLACE modification:

https://github.com/DirectoryTree/LdapRecord/blob/8ad10c26a992b46a265ff84b2d1845003c910a6e/src/Models/Concerns/HasPassword.php#L136-L153

We can update this to check if the user exists to properly set the batch ADD modification so that the user can be created with a password:

$batch = $this->exists
    ? LDAP_MODIFY_BATCH_REPLACE
    : LDAP_MODIFY_BATCH_ADD;

$this->addModification(
    $this->newBatchModification(
        $attribute,
        $batch,
        [$password]
    )
);

Going to patch this, thanks for the report 🙏

[stevebauman]

Hi @vatsake, I've just released a new version of LdapRecord (v2.17.2). Would you be able to run composer update and attempt your script again to make sure it's working for you?

[vatsake]

Hey

Something is still missing. The password is not getting SET during account creation.

[stevebauman]

Can you confirm you're on the new version (v2.17.2)? You can find out by searching your composer.lock file for directorytree/ldaprecord

[vatsake]

Yeah, I am on the new version. I can even see unicodepwd "modtype" => 1

[stevebauman]

Hmm that's strange -- as that would be the same batch modification generated on the root Model without extending the User model. I've just added a test case to ensure this is the case: https://github.com/DirectoryTree/LdapRecord/commit/3e5af5583a4d7f68f54d90d97183fd32ae1bc7c7

Can you revert to using your own custom model (without extending ActiveDirectory\User) and then dump the batch modifications prior to $user->save() and post them here?

Ex:

$user = new User();

$user->unicodepwd = $request->input('password');
$user->pwdlastset = -1;

var_dump($this->getModifications());

// $user->save();

[vatsake]

Extended LdapRecord Model, password encoded manually

array:15 [
  0 => array:3 [
    "attrib" => "givenname"
    "modtype" => 1
    "values" => array:1 [
      0 => "12312"
    ]
  ]
  1 => array:3 [
    "attrib" => "sn"
    "modtype" => 1
    "values" => array:1 [
      0 => "123"
    ]
  ]
  2 => array:3 [
    "attrib" => "displayname"
    "modtype" => 1
    "values" => array:1 [
      0 => "12312 123"
    ]
  ]
  3 => array:3 [
    "attrib" => "cn"
    "modtype" => 1
    "values" => array:1 [
      0 => "12312 123"
    ]
  ]
  4 => array:3 [
    "attrib" => "samaccountname"
    "modtype" => 1
    "values" => array:1 [
      0 => "testuser"
    ]
  ]
  5 => array:3 [
    "attrib" => "userprincipalname"
    "modtype" => 1
    "values" => array:1 [
      0 => "[email protected]"
    ]
  ]
  6 => array:3 [
    "attrib" => "unicodepwd"
    "modtype" => 1
    "values" => array:1 [
      0 => ""\x00d\x001\x00I\x00F\x00]\x00!\x00m\x00I\x00d\x00Z\x00$\x004\x007\x00?\x00"\x00"
    ]
  ]
  7 => array:3 [
    "attrib" => "pwdlastset"
    "modtype" => 1
    "values" => array:1 [
      0 => "-1"
    ]
  ]
  8 => array:3 [
    "attrib" => "employeenumber"
    "modtype" => 1
    "values" => array:1 [
      0 => "ter1"
    ]
  ]
  9 => array:3 [
    "attrib" => "department"
    "modtype" => 1
    "values" => array:1 [
      0 => "123"
    ]
  ]
  10 => array:3 [
    "attrib" => "company"
    "modtype" => 1
    "values" => array:1 [
      0 => "123"
    ]
  ]
  11 => array:3 [
    "attrib" => "title"
    "modtype" => 1
    "values" => array:1 [
      0 => "123"
    ]
  ]
  12 => array:3 [
    "attrib" => "streetaddress"
    "modtype" => 1
    "values" => array:1 [
      0 => "23"
    ]
  ]
  13 => array:3 [
    "attrib" => "mail"
    "modtype" => 1
    "values" => array:1 [
      0 => "[email protected]"
    ]
  ]
  14 => array:3 [
    "attrib" => "mailnickname"
    "modtype" => 1
    "values" => array:1 [
      0 => "testuser"
    ]
  ]
]

[stevebauman]

@vatsake Can you post how you're encoding the password manually? The password is properly encoded on my end using the code below:

use LdapRecord\Models\ActiveDirectory\User;

$user = new User;

$user->password = 'foobar';

dd($user->getModifications());

// array:1 [
//   0 => array:3 [
//     "attrib" => "unicodepwd"
//     "modtype" => 1
//     "values" => array:1 [
//       0 => ""\x00f\x00o\x00o\x00b\x00a\x00r\x00"\x00"
//     ]
//   ]
// ]

[vatsake]

The password is correctly encoded with both custom and ActiveDirectory\User classes.

$user = (new User())->inside('...');
....
$user->unicodepwd = $request->input('password');
$user->pwdlastset = -1;
...
$user->save();
$user->refresh();
...
$user->update(['userAccountControl' => 512, 'pwdlastset' => 0]); // this throws ldap_modify_batch(): Batch Modify: Server is unwilling to perform
// This fails because the password is not set?

 0 => array:3 [
    "attrib" => "unicodepwd"
    "modtype" => 1
    "values" => array:1 [
      0 => ""\x00y\x00h\x009\x00#\x00b\x00V\x00f\x00E\x00m\x00a\x00n\x00h\x00"\x00"
    ]
  ]

However if I set password like this:

$user = (new User())->inside('...');
...
$user->save();
$user->refresh();
...
$user->update(['userAccountControl' => 512, 'pwdlastset' => 0, 'unicodepwd' => $request->input('password')]);

it works.

[stevebauman]

The password is correctly encoded with both custom and ActiveDirectory\User classes.

Thanks for confirming that @vatsake 🙏

// This fails because the password is not set?

Can you explain this line? The password won't be sent (or set) in the subsequent update() call because the model was previously saved above it, clearing any modifications to the user.

However if I set password like this:

Thanks for confirming that this approach works 🙏

[vatsake]

I'm not sure either. If i use the custom 'Model' class and encode my password with iconv like so

$user = (new User())->inside('...');
....
$user->unicodepwd = iconv('UTF-8', 'UTF-16LE', '"' . $request->input('password') . '"');
$user->pwdlastset = -1;
...
$user->save();
$user->refresh();
...
$user->update(['userAccountControl' => 512, 'pwdlastset' => 0]);

it works

***EDIT You can close this issue, it's not a big deal. I can use this $user->update(['userAccountControl' => 512, 'pwdlastset' => 0, 'unicodepwd' => $request->input('password')]);

[stevebauman]

That’s so bizarre, because that’s the exact method that’s being used in LdapRecord as well 🤔

https://github.com/DirectoryTree/LdapRecord/blob/f93da388263689e48e92bcc2a2f11a5ddf655f99/src/Models/Attributes/Password.php#L24