diff --git a/src/Connection.php b/src/Connection.php index de111689..61b98d76 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -326,6 +326,28 @@ public function isolate(Closure $operation): mixed } } + /** + * Change a user's password using the RFC 3062 Password Modify extended operation. + * + * The change is performed on an isolated connection bound as the user itself, + * so the directory verifies the current password (a self-service change) + * without altering the bind state of the primary connection. + * + * @throws LdapRecordException + */ + public function changePassword(string $dn, string $oldPassword, string $newPassword): bool|string + { + return $this->isolate(function (Connection $connection) use ($dn, $oldPassword, $newPassword) { + // Connecting as the user proves knowledge of their current + // password, and upgrades the connection with StartTLS when + // configured, before any credentials are transmitted. A + // failed bind throws, leaving the change unapplied. + $connection->connect($dn, $oldPassword); + + return $connection->getLdapConnection()->exopPasswd($dn, $oldPassword, $newPassword); + }); + } + /** * Attempt to get an exception for the cause of failure. */ diff --git a/src/Ldap.php b/src/Ldap.php index 09350e75..31ac1adb 100644 --- a/src/Ldap.php +++ b/src/Ldap.php @@ -368,6 +368,22 @@ public function modifyBatch(string $dn, array $values): bool }); } + /** + * {@inheritdoc} + */ + public function exopPasswd(string $user = '', string $oldPassword = '', string $newPassword = '', ?array &$controls = null): bool|string + { + if (! function_exists('ldap_exop_passwd')) { + throw new LdapRecordException( + 'The function [ldap_exop_passwd] is unavailable. Ensure your PHP LDAP extension supports extended operations.' + ); + } + + return $this->executeFailableOperation(function () use ($user, $oldPassword, $newPassword, &$controls) { + return ldap_exop_passwd($this->connection, $user, $oldPassword, $newPassword, $controls); + }); + } + /** * {@inheritdoc} */ diff --git a/src/LdapInterface.php b/src/LdapInterface.php index 80b6d6ee..499aa8dc 100644 --- a/src/LdapInterface.php +++ b/src/LdapInterface.php @@ -564,6 +564,19 @@ public function modify(string $dn, array $entry): bool; */ public function modifyBatch(string $dn, array $values): bool; + /** + * Modify a password using the RFC 3062 Password Modify extended operation. + * + * Returns the server-generated password when no new password is supplied, + * true on success when a new password is given, or false on failure. + * + * @see https://www.php.net/manual/en/function.ldap-exop-passwd.php + * @see https://www.rfc-editor.org/rfc/rfc3062 + * + * @throws LdapRecordException + */ + public function exopPasswd(string $user = '', string $oldPassword = '', string $newPassword = '', ?array &$controls = null): bool|string; + /** * Add attribute values to current attributes. * diff --git a/src/Models/Attributes/Password.php b/src/Models/Attributes/Password.php index 71f4d06e..8d62063c 100644 --- a/src/Models/Attributes/Password.php +++ b/src/Models/Attributes/Password.php @@ -102,6 +102,40 @@ public static function md5(string $password): string return '{MD5}'.static::makeHash($password, 'md5'); } + /** + * Make an argon2i password. + * + * OpenLDAP's argon2 module registers the single scheme {ARGON2} — + * the variant is carried by the PHC string that follows it. + * + * @throws LdapRecordException + */ + public static function argon2i(string $password): string + { + if (! defined('PASSWORD_ARGON2I')) { + throw new LdapRecordException('Argon2i hashing is not supported by this PHP build.'); + } + + return '{ARGON2}'.password_hash($password, PASSWORD_ARGON2I); + } + + /** + * Make an argon2id password. + * + * OpenLDAP's argon2 module registers the single scheme {ARGON2} — + * the variant is carried by the PHC string that follows it. + * + * @throws LdapRecordException + */ + public static function argon2id(string $password): string + { + if (! defined('PASSWORD_ARGON2ID')) { + throw new LdapRecordException('Argon2id hashing is not supported by this PHP build.'); + } + + return '{ARGON2}'.password_hash($password, PASSWORD_ARGON2ID); + } + /** * Make a non-salted NThash password. */ @@ -228,6 +262,18 @@ public static function getSalt(string $encryptedPassword): string throw new LdapRecordException('Could not extract salt from encrypted password.'); } + /** + * Determine if passwords hashed with the given method can only be + * changed using the password modify extended operation (RFC 3062). + * + * These hashes embed a random salt that cannot be extracted, so the + * stored hash cannot be reproduced for a REMOVE/ADD batch modification. + */ + public static function hashMethodRequiresExop(string $method): bool + { + return in_array(strtolower($method), ['argon2', 'argon2i', 'argon2id'], true); + } + /** * Determine if the hash method requires a salt to be given. * diff --git a/src/Models/Concerns/HasPassword.php b/src/Models/Concerns/HasPassword.php index 8eb01ef9..8185f2e1 100644 --- a/src/Models/Concerns/HasPassword.php +++ b/src/Models/Concerns/HasPassword.php @@ -10,15 +10,24 @@ /** @mixin Model */ trait HasPassword { + /** + * The [old, new] password pair of a change deferred until the model is saved. + */ + protected ?array $pendingPasswordChange = null; + /** * Set the password on the user. * * @throws ConnectionException + * @throws LdapRecordException */ public function setPasswordAttribute(array|string $password): void { $this->assertSecureConnection(); + // Setting a password always supersedes a previously deferred change. + $this->pendingPasswordChange = null; + // Here we will attempt to determine the password hash method in use // by parsing the users hashed password (if it as available). If a // method is determined, we will override the default here. @@ -29,9 +38,27 @@ public function setPasswordAttribute(array|string $password): void // If the password given is an array, we can assume we // are changing the password for the current user. if (is_array($password)) { + [$oldPassword, $newPassword] = $password; + + // Argon2 hashes embed a random salt, so the currently stored hash + // cannot be reproduced to emit a REMOVE/ADD batch modification. + // Instead we defer a self-service RFC 3062 Password Modify + // extended operation until the model is saved. + if ($this->passwordChangeRequiresExop($method)) { + if (! $this->exists || ! $this->getDn()) { + throw new LdapRecordException( + 'A password change requires an existing model with a distinguished name.' + ); + } + + $this->pendingPasswordChange = [$oldPassword, $newPassword]; + + return; + } + $this->setChangedPassword( - $this->getHashedPassword($method, $password[0], $this->getPasswordSalt($method)), - $this->getHashedPassword($method, $password[1]), + $this->getHashedPassword($method, $oldPassword, $this->getPasswordSalt($method)), + $this->getHashedPassword($method, $newPassword), $this->getPasswordAttributeName() ); } @@ -119,6 +146,86 @@ protected function setChangedPassword(string $oldPassword, string $newPassword, ); } + /** + * Determine if changing a password hashed with the given method requires + * an extended operation rather than a batch modification. + * + * Overriding this to return true routes any password change through the + * RFC 3062 extended operation, letting the server hash the password. + */ + protected function passwordChangeRequiresExop(string $method): bool + { + return Password::hashMethodRequiresExop($method); + } + + /** + * Determine if the model has a password change deferred until save. + */ + public function hasPendingPasswordChange(): bool + { + return ! is_null($this->pendingPasswordChange); + } + + /** + * Flush any password change deferred until save. + * + * @throws LdapRecordException + */ + public function flushPendingPasswordChange(): void + { + if (! $change = $this->pendingPasswordChange) { + return; + } + + [$oldPassword, $newPassword] = $change; + + $this->getConnection()->changePassword( + $this->getDn(), $oldPassword, $newPassword + ); + + // Cleared only once the operation has succeeded, so + // a failed change is re-attempted on a retried save. + $this->pendingPasswordChange = null; + } + + /** + * Determine if the model has operations deferred until save. + */ + protected function hasDeferredOperations(): bool + { + return $this->hasPendingPasswordChange(); + } + + /** + * Perform any operations deferred until the model is saved. + * + * @throws LdapRecordException + */ + protected function performDeferredOperations(): void + { + $this->flushPendingPasswordChange(); + } + + /** + * Discard attribute changes and reset the attributes to their original state. + */ + public function discardChanges(): static + { + $this->pendingPasswordChange = null; + + return parent::discardChanges(); + } + + /** + * Set the model's attributes with raw LDAP result values. + */ + public function setRawAttributes(array $attributes = []): static + { + $this->pendingPasswordChange = null; + + return parent::setRawAttributes($attributes); + } + /** * Set the password on the model. */ @@ -210,6 +317,12 @@ public function determinePasswordHashMethod(): ?string return null; } + // The {ARGON2} scheme carries its variant inside the PHC + // string following the prefix, not in the scheme name. + if (strcasecmp($method, 'argon2') === 0) { + return str_contains($password, '$argon2i$') ? 'argon2i' : 'argon2id'; + } + if (! $hashAndAlgo = Password::getHashMethodAndAlgo($password)) { return $method; } diff --git a/src/Models/Model.php b/src/Models/Model.php index 24d42f80..45a008bc 100644 --- a/src/Models/Model.php +++ b/src/Models/Model.php @@ -962,6 +962,21 @@ public function save(array $attributes = []): void $this->in = null; } + /** + * Determine if the model has operations deferred until save. + */ + protected function hasDeferredOperations(): bool + { + return false; + } + + /** + * Perform any operations deferred until the model is saved. + * + * @throws LdapRecordException + */ + protected function performDeferredOperations(): void {} + /** * Inserts the model into the directory. * @@ -1016,13 +1031,24 @@ protected function performInsert(): void */ protected function performUpdate(): void { - if (! count($modifications = $this->getModifications())) { + $modifications = $this->getModifications(); + + // Deferred operations (such as argon2 password changes) cannot be + // expressed as batch modifications, but still constitute an update. + if (! count($modifications) && ! $this->hasDeferredOperations()) { return; } $this->dispatch('updating'); - $this->newQuery()->update($this->dn, $modifications); + // Deferred operations run first — they verify the user's current + // credentials, so they are more likely to be rejected than the + // batch modifications, and failing early avoids partial updates. + $this->performDeferredOperations(); + + if (count($modifications)) { + $this->newQuery()->update($this->dn, $modifications); + } $this->dispatch('updated'); diff --git a/src/Testing/ConnectionFake.php b/src/Testing/ConnectionFake.php index 2c76a06d..132a0692 100644 --- a/src/Testing/ConnectionFake.php +++ b/src/Testing/ConnectionFake.php @@ -21,6 +21,11 @@ class ConnectionFake extends Connection */ protected bool $connected = false; + /** + * Whether the fake is a replica sharing another connection's fake LDAP connection. + */ + protected bool $replicated = false; + /** * Make a new fake LDAP connection instance. */ @@ -33,6 +38,35 @@ public static function make(array $config = [], string $ldap = LdapFake::class): return $connection; } + /** + * Clone the connection, reusing the same fake LDAP connection. + * + * Sharing the underlying fake allows operations performed on an isolated + * connection (e.g. self-service password changes) to be asserted through + * the original fake's expectations. + */ + public function replicate(): static + { + $replica = new static($this->configuration, $this->ldap); + + $replica->replicated = true; + + return $replica; + } + + /** + * {@inheritdoc} + * + * Disconnecting a replica must not reset the state of + * the fake LDAP connection it shares with its parent. + */ + public function disconnect(): void + { + if (! $this->replicated) { + parent::disconnect(); + } + } + /** * Set the user to authenticate as. */ diff --git a/src/Testing/LdapFake.php b/src/Testing/LdapFake.php index a1608e1a..3d2dc6e6 100644 --- a/src/Testing/LdapFake.php +++ b/src/Testing/LdapFake.php @@ -458,6 +458,14 @@ public function modifyBatch(string $dn, array $values): bool return $this->resolveExpectation(__FUNCTION__, func_get_args()); } + /** + * {@inheritdoc} + */ + public function exopPasswd(string $user = '', string $oldPassword = '', string $newPassword = '', ?array &$controls = null): bool|string + { + return $this->resolveExpectation(__FUNCTION__, func_get_args()); + } + /** * {@inheritdoc} */ diff --git a/tests/Unit/Models/Attributes/PasswordTest.php b/tests/Unit/Models/Attributes/PasswordTest.php index 67a2c45c..d3727935 100644 --- a/tests/Unit/Models/Attributes/PasswordTest.php +++ b/tests/Unit/Models/Attributes/PasswordTest.php @@ -92,6 +92,32 @@ public function test_sha512crypt() $this->assertEquals($password, Password::sha512crypt('password', Password::getSalt($password))); } + public function test_argon2i() + { + $password = Password::argon2i('password'); + + $this->assertStringStartsWith('{ARGON2}$argon2i$', $password); + $this->assertNotEquals($password, Password::argon2i('password')); + } + + public function test_argon2id() + { + $password = Password::argon2id('password'); + + $this->assertStringStartsWith('{ARGON2}$argon2id$', $password); + $this->assertNotEquals($password, Password::argon2id('password')); + } + + public function test_hash_method_requires_exop() + { + $this->assertTrue(Password::hashMethodRequiresExop('argon2')); + $this->assertTrue(Password::hashMethodRequiresExop('argon2i')); + $this->assertTrue(Password::hashMethodRequiresExop('ARGON2ID')); + + $this->assertFalse(Password::hashMethodRequiresExop('ssha')); + $this->assertFalse(Password::hashMethodRequiresExop('md5')); + } + // Unsalted Hash Tests. // public function test_sha() diff --git a/tests/Unit/Models/OpenLDAP/UserTest.php b/tests/Unit/Models/OpenLDAP/UserTest.php index e03a4d11..26dee918 100644 --- a/tests/Unit/Models/OpenLDAP/UserTest.php +++ b/tests/Unit/Models/OpenLDAP/UserTest.php @@ -4,8 +4,13 @@ use LdapRecord\Connection; use LdapRecord\Container; +use LdapRecord\LdapRecordException; use LdapRecord\Models\Attributes\Password; +use LdapRecord\Models\Events\Updated; +use LdapRecord\Models\Events\Updating; use LdapRecord\Models\OpenLDAP\User; +use LdapRecord\Testing\DirectoryFake; +use LdapRecord\Testing\LdapFake; use LdapRecord\Tests\TestCase; class UserTest extends TestCase @@ -75,6 +80,240 @@ public function test_algo_and_salt_is_automatically_detected_when_changing_a_use $this->assertEquals(Password::CRYPT_SALT_TYPE_SHA512, $newAlgo); } + public function test_changing_argon2_password_defers_a_pending_change_instead_of_queuing_modifications() + { + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + // The change is performed via an extended operation on save, so no + // batch modifications are queued for it. + $this->assertEmpty($user->getModifications()); + $this->assertTrue($user->hasPendingPasswordChange()); + } + + public function test_resetting_argon2_password_still_queues_a_single_replace_modification() + { + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = 'new-secret'; + + $modifications = $user->getModifications(); + + $this->assertFalse($user->hasPendingPasswordChange()); + $this->assertCount(1, $modifications); + $this->assertEquals(LDAP_MODIFY_BATCH_REPLACE, $modifications[0]['modtype']); + $this->assertEquals('ARGON2', Password::getHashMethod($modifications[0]['values'][0])); + $this->assertStringContainsString('$argon2id$', $modifications[0]['values'][0]); + } + + public function test_reassigning_a_password_clears_a_pending_password_change() + { + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + $this->assertTrue($user->hasPendingPasswordChange()); + + $user->password = 'reset-secret'; + + // The reset supersedes the deferred change entirely — only + // the replace modification may reach the directory on save. + $this->assertFalse($user->hasPendingPasswordChange()); + $this->assertCount(1, $user->getModifications()); + } + + public function test_discarding_changes_clears_a_pending_password_change() + { + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + $user->discardChanges(); + + $this->assertFalse($user->hasPendingPasswordChange()); + } + + public function test_a_pending_password_change_survives_serialization() + { + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + $restored = unserialize(serialize($user)); + + $this->assertTrue($restored->hasPendingPasswordChange()); + } + + public function test_changing_argon2_password_throws_when_model_does_not_exist() + { + $user = new OpenLDAPUserTestStub([ + 'userpassword' => Password::argon2id('secret'), + ]); + + $this->expectException(LdapRecordException::class); + + $user->password = ['secret', 'new-secret']; + } + + public function test_changing_argon2_password_performs_a_self_service_exop_on_save() + { + $ldap = DirectoryFake::setup()->getLdapConnection(); + + $ldap->expect([ + LdapFake::operation('bind')->once() + ->with('cn=jdoe,dc=local,dc=com', 'secret') + ->andReturnResponse(), + + LdapFake::operation('exopPasswd')->once() + ->with('cn=jdoe,dc=local,dc=com', 'secret', 'new-secret') + ->andReturnTrue(), + ]); + + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + $user->save(); + + $this->assertFalse($user->hasPendingPasswordChange()); + + // Guards against the change being silently skipped because no batch + // modifications exist (the early return in Model::performUpdate). + $ldap->assertMinimumExpectationCounts(); + } + + public function test_changing_argon2_password_throws_when_current_password_is_rejected() + { + $ldap = DirectoryFake::setup()->getLdapConnection(); + + $ldap->expect([ + LdapFake::operation('bind')->once() + ->with('cn=jdoe,dc=local,dc=com', 'wrong-secret') + ->andReturnResponse(49), + ]); + + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['wrong-secret', 'new-secret']; + + $this->expectException(LdapRecordException::class); + + $user->save(); + } + + public function test_changing_argon2_password_fires_update_events_on_save() + { + $ldap = DirectoryFake::setup()->getLdapConnection(); + + $ldap->expect([ + LdapFake::operation('bind')->once()->andReturnResponse(), + LdapFake::operation('exopPasswd')->once()->andReturnTrue(), + ]); + + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + $fired = []; + + $dispatcher = Container::getInstance()->getDispatcher(); + + $dispatcher->listen(Updating::class, function () use (&$fired) { + $fired[] = 'updating'; + }); + + $dispatcher->listen(Updated::class, function () use (&$fired) { + $fired[] = 'updated'; + }); + + $user->save(); + + $this->assertEquals(['updating', 'updated'], $fired); + } + + public function test_failed_argon2_password_change_is_retained_and_can_be_retried() + { + $ldap = DirectoryFake::setup()->getLdapConnection(); + + $ldap->expect([ + LdapFake::operation('bind')->once() + ->with('cn=jdoe,dc=local,dc=com', 'secret') + ->andReturnResponse(49), + + LdapFake::operation('bind')->once() + ->with('cn=jdoe,dc=local,dc=com', 'secret') + ->andReturnResponse(), + + LdapFake::operation('exopPasswd')->once() + ->with('cn=jdoe,dc=local,dc=com', 'secret', 'new-secret') + ->andReturnTrue(), + ]); + + $user = (new OpenLDAPUserTestStub)->setRawAttributes([ + 'dn' => ['cn=jdoe,dc=local,dc=com'], + 'userpassword' => [ + Password::argon2id('secret'), + ], + ]); + + $user->password = ['secret', 'new-secret']; + + try { + $user->save(); + + $this->fail('Expected the initial save to throw.'); + } catch (LdapRecordException) { + // A transiently failed change is retained for retry. + } + + $this->assertTrue($user->hasPendingPasswordChange()); + + $user->save(); + + $this->assertFalse($user->hasPendingPasswordChange()); + } + public function test_correct_auth_identifier_is_returned() { $entryUuid = 'foo'; diff --git a/tests/Unit/Testing/ConnectionFakeTest.php b/tests/Unit/Testing/ConnectionFakeTest.php index 9f496960..8f04c527 100644 --- a/tests/Unit/Testing/ConnectionFakeTest.php +++ b/tests/Unit/Testing/ConnectionFakeTest.php @@ -61,6 +61,25 @@ public function test_acting_as_with_dn() $this->assertTrue($fake->auth()->attempt('cn=John Doe,dc=local,dc=com', 'secret', $stayBound = true)); } + + public function test_replicated_connection_shares_the_fake_without_resetting_it_on_disconnect() + { + $fake = ConnectionFake::make(); + + $ldap = $fake->getLdapConnection(); + + $ldap->connect('localhost', 389); + + $replica = $fake->replicate(); + + $this->assertSame($ldap, $replica->getLdapConnection()); + + // Disconnecting the replica (as Connection::isolate() does) must + // not reset the state of the fake shared with the parent. + $replica->disconnect(); + + $this->assertTrue($ldap->isConnected()); + } } class ExtendedLdapFake extends LdapFake {}