Merge pull request #617 from DirectoryTree/bug-616

Bug 616 - Strange behavior unicodepwd and userAccountControl
DirectoryTree · Jul 7, 2023 · 3e69c09 · 3e69c09
2 parents 20b7fa4 + b8b0431
commit 3e69c09
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 62 deletions.
diff --git a/src/Models/Concerns/HasPassword.php b/src/Models/Concerns/HasPassword.php
@@ -47,7 +47,6 @@ public function setPasswordAttribute(array|string $password): void
     /**
      * Alias for setting the password on the user.
      *
-     *
      * @throws ConnectionException
      */
     public function setUnicodepwdAttribute(array|string $password): void
@@ -124,14 +123,16 @@ protected function setChangedPassword(string $oldPassword, string $newPassword,
      */
     protected function setPassword(string $password, string $attribute): void
     {
-        $modtype = $this->exists
-            ? LDAP_MODIFY_BATCH_REPLACE
-            : LDAP_MODIFY_BATCH_ADD;
+        if (! $this->exists) {
+            $this->setRawAttribute($attribute, $password);
+
+            return;
+        }
 
         $this->addModification(
             $this->newBatchModification(
                 $attribute,
-                $modtype,
+                LDAP_MODIFY_BATCH_REPLACE,
                 [$password]
             )
         );

diff --git a/tests/Unit/Models/ActiveDirectory/UserTest.php b/tests/Unit/Models/ActiveDirectory/UserTest.php
@@ -11,7 +11,10 @@
 use LdapRecord\Models\Attributes\AccountControl;
 use LdapRecord\Models\Attributes\Password;
 use LdapRecord\Models\Attributes\Timestamp;
+use LdapRecord\Models\Entry;
 use LdapRecord\Models\Model;
+use LdapRecord\Testing\DirectoryFake;
+use LdapRecord\Testing\LdapFake;
 use LdapRecord\Tests\TestCase;
 
 class UserTest extends TestCase
@@ -34,88 +37,91 @@ public function test_setting_password_requires_secure_connection()
     {
         $this->expectException(ConnectionException::class);
 
-        new User(['unicodepwd' => 'password']);
+        new User(['password' => 'password']);
     }
 
     public function test_changing_password_requires_secure_connection()
     {
-        $user = (new User())->setRawAttributes(['dn' => 'foo']);
+        $user = (new User)->setRawAttributes(['dn' => 'foo']);
 
         $this->expectException(ConnectionException::class);
 
-        $user->unicodepwd = ['old', 'new'];
-    }
-
-    public function test_set_password_on_new_user()
-    {
-        $user = new UserPasswordTestStub();
-
-        $user->unicodepwd = 'foo';
-
-        $this->assertCount(1, $mods = $user->getModifications());
-
-        $this->assertEquals([
-            'attrib' => 'unicodepwd',
-            'modtype' => LDAP_MODIFY_BATCH_ADD,
-            'values' => [Password::encode('foo')],
-        ], $mods[0]);
-    }
-
-    public function test_set_password_on_existing_user()
-    {
-        $user = new UserPasswordTestStub();
-
-        $user->exists = true;
-
-        $user->unicodepwd = 'foo';
-
-        $this->assertCount(1, $mods = $user->getModifications());
-
-        $this->assertEquals([
-            'attrib' => 'unicodepwd',
-            'modtype' => LDAP_MODIFY_BATCH_REPLACE,
-            'values' => [Password::encode('foo')],
-        ], $mods[0]);
+        $user->password = ['old', 'new'];
     }
 
     public function test_set_password_behaves_identically_on_non_user_models()
     {
-        $user = new UserPasswordTestStub();
+        DirectoryFake::setup()
+            ->getLdapConnection()
+            ->expect(LdapFake::operation('isUsingSSL')->andReturnTrue());
+
+        $user = new User();
 
         $user->unicodepwd = 'foo';
 
-        $nonUser = new NonUserPasswordTestStub();
+        $nonUser = new Entry();
 
         $nonUser->unicodepwd = Password::encode('foo');
 
         $this->assertEquals($user->getModifications(), $nonUser->getModifications());
     }
 
-    public function test_password_mutator_alias_works()
+    public function test_create_user_with_password()
     {
-        $user = new UserPasswordTestStub(['password' => 'secret']);
+        DirectoryFake::setup()
+            ->getLdapConnection()
+            ->expect([
+                LdapFake::operation('isUsingSSL')->once()->andReturnTrue(),
+                LdapFake::operation('add')->once()->with(fn ($dn) => true, fn ($attributes) => (
+                    $attributes['unicodepwd'] === [Password::encode('foobar')]
+                    && $attributes['useraccountcontrol'] = 512
+                ))->andReturnTrue(),
+            ]);
 
-        $this->assertEquals([Password::encode('secret')], $user->getModifications()[0]['values']);
-    }
+        $user = new User();
 
-    public function test_changing_passwords()
-    {
-        $user = (new UserPasswordTestStub())->setRawAttributes(['dn' => 'foo']);
+        $user->password = 'foobar';
+        $user->userAccountControl = 512;
+
+        $user->save();
+    }
+
+    public function test_update_user_with_password()
+    {
+        DirectoryFake::setup()
+            ->getLdapConnection()
+            ->expect([
+                LdapFake::operation('isUsingSSL')->once()->andReturnTrue(),
+                LdapFake::operation('modifyBatch')->once()->with(
+                    function ($dn) {
+                        return $dn === 'cn=john,dc=local,dc=com';
+                    },
+                    function ($mods) {
+                        return count($mods) === 2
+                            && $mods[0] === [
+                                'attrib' => 'unicodepwd',
+                                'modtype' => LDAP_MODIFY_BATCH_REMOVE,
+                                'values' => [Password::encode('old')],
+                            ]
+                            && $mods[1] === [
+                                'attrib' => 'unicodepwd',
+                                'modtype' => LDAP_MODIFY_BATCH_ADD,
+                                'values' => [Password::encode('new')],
+                            ];
+                    }
+                )->andReturnTrue(),
+            ]);
+
+        $user = new User();
+
+        $user->setRawAttributes([
+            'useraccountcontrol' => [512],
+            'dn' => ['cn=john,dc=local,dc=com'],
+        ]);
 
-        $user->unicodepwd = ['bar', 'baz'];
+        $user->password = ['old', 'new'];
 
-        $this->assertEquals([
-            [
-                'attrib' => 'unicodepwd',
-                'modtype' => 2,
-                'values' => [Password::encode('bar')],
-            ],
-            [
-                'attrib' => 'unicodepwd',
-                'modtype' => 1,
-                'values' => [Password::encode('baz')],
-            ],
-        ], $user->getModifications());
+        $user->save();
     }
 
     public function test_reject_computer_object_class_is_a_default_scope()