Add support for argon2

[Ph0tonic]

Hey, here is a PR to support argon hashing algorithm 🎉

Add argon2i and argon2id password hashing support

Adds Password::argon2i() and Password::argon2id() methods to the Password class, enabling use of the {ARGON2I} and {ARGON2ID} userPassword schemes supported by OpenLDAP via the pw-argon2 module.

Changes

• Add Password::argon2i() and Password::argon2id() static methods using PHP's native password_hash()
• Throw a LdapRecordException when attempting to change an argon2 password using the array syntax ($user->password = ['old', 'new']), since argon2 hashes are non-deterministic and the old hash cannot be reproduced to satisfy the LDAP REMOVE operation

Notes

• Requires PHP 8.1+ (already the project minimum) with libargon2 support (standard on modern distributions)
• Requires the OpenLDAP pw-argon2 contrib module to be loaded server-side (moduleload pw-argon2) — without it, the server will reject the {ARGON2ID} scheme
• Password reset ($user->password = 'new') works as expected; only the password change (array) flow is unsupported — this can be addressed in a follow-up by implementing the LDAP Password Modify extended operation (ldap_exop_passwd)

[stevebauman]

Thanks for the PR @Ph0tonic!

In regards to this:

Password reset ($user->password = 'new') works as expected; only the password change (array) flow is unsupported — this can be addressed in a follow-up by implementing the LDAP Password Modify extended operation (ldap_exop_passwd)

According to the docs for this method, it supports being called with $old_password here:

https://www.php.net/manual/en/function.ldap-exop-passwd.php

function ldap_exop_passwd(
    LDAP\Connection $ldap,
    string $user = "",
    #[\SensitiveParameter] string $old_password = "",
    #[\SensitiveParameter] string $new_password = "",
    array &$controls = null
): string|bool

So it looks like we may be able to support the same flow without throwing the exception here.

We could extract the mechanism for changing passwords into some interface and classes depending on the method being used. Maybe something like (psuedo-code):

$changer = match (strtolower($method)) {
    'argon2i', 'argon2id' => new ArgonPasswordChanger(...),
    default => new DefaultPasswordChanger(...),
};

Thoughts?

[stevebauman]

In other words I think we should implement the full operation here instead of in a follow up, because if the exception is no longer thrown then that's a major breaking change.

[Ph0tonic]

I agree — supporting this directly is the right approach.

The challenge is that setChangedPassword works by queuing a BatchModification that gets executed asynchronously when save() is called. ldap_exop_passwd doesn't fit into that model — it's a separate LDAP protocol operation that can't be expressed as a batch modification, so it can't participate in the existing modification queue.

If we call exopPasswd directly in the setter it executes immediately, breaking the deferred-write contract that the rest of the password flow follows.

To defer it to save() time, we'd need an extension point in Model::performUpdate() between the "no modifications, bail early" guard and the event dispatching — something like hasPendingUpdates() / executePendingUpdates() hooks that traits could override. Without that hook, we're forced to either override performUpdate() entirely (which duplicates the dispatch/sync sequence) or execute eagerly in the setter (which breaks the save contract).

Would you be open to adding such hooks to Model, or is there a pattern in the codebase I'm missing that already handles this kind of deferred non-batch operation?

[stevebauman]

Yea you're right, will be tricky to maintain that queued behaviour. I think we could register a one-time event listener on the updating event to bail/throw an exception if the password change fails. Let me take a look into this and see what I can do 👍

[Ph0tonic]

@stevebauman — pushed an update that implements the full change flow (no more exception). Summary of the approach and the design decisions behind it:

Why the array syntax can't stay a batch modification for argon2

The ['old', 'new'] flow normally emits REMOVE(old-hash) + ADD(new-hash), reproducing the stored old hash by extracting its salt and re-hashing. Argon2 embeds a random salt and password_hash() won't accept a caller-supplied one, so the stored hash can't be reproduced — the REMOVE can never match. So argon2 changes have to go through the RFC 3062 Password Modify extended operation (ldap_exop_passwd), which is a separate protocol op, not a batch modification.

One correction on the updating-listener idea

A one-time listener on updating won't fire in the common case. An argon2 change touches neither $this->modifications nor any dirty attribute, so getModifications() is empty — and performUpdate() early-returns on empty modifications before dispatching updating (Model.php). So when the password change is the only change, updating/updated never fire. (listenForModelEvent also registers on the shared container dispatcher, which would leak across instances.)

What I did instead

• The setter no longer throws for argon2; it queues a deferred closure ($pendingPasswordChange).
• Added a generic Model::performDeferredOperations() hook (default no-op) called in save() right after performUpdate()/performInsert() and before dispatch('saved'). HasPassword overrides it to flush the pending change. This guarantees it runs even when there are no batch modifications, keeps the "nothing hits the wire until save()" contract, and aborts the save if the operation fails.
• Kept the strategy selection minimal — a passwordChangeRequiresExop() predicate (match on argon2i/argon2id) rather than a full PasswordChanger hierarchy. Easy to promote to a strategy object later if more exop-only schemes show up.
• Added exopPasswd() to LdapInterface + Ldap (guarded on function_exists, routed through executeFailableOperation so a failure throws) + LdapFake.

Self-service vs. admin reset

The actual change lives in a new Connection::changePassword($dn, $old, $new) that runs on an isolated connection bound as the user, then performs the exop. The bind is deliberate: the Password Modify exop runs as whoever is currently bound, and our primary connection is bound as the configured admin — so a bare exop there would be an admin reset that most servers authorize without verifying the old password (a wrong old password could silently succeed). Binding as the user with the current password is what actually enforces the "must know the old password" guarantee and runs the change in the user's own context, without disturbing the primary connection's bind state.

Tests

OpenLDAP\User tests now cover: the change is deferred (no batch mods queued), reset still emits a single REPLACE, the self-service exop fires on save() even with no other modifications (asserted via LdapFake + assertMinimumExpectationCounts), and a rejected current password throws. The existing SSHA/CRYPT change flow and AD's unicodePwd flow are untouched.

BC note

The only real surface change is adding exopPasswd() to LdapInterface — a third party implementing the interface directly (rather than extending Ldap) would need to add it. Both shipped implementations (Ldap, LdapFake) do.

Happy to adjust any of the naming or move changePassword somewhere you'd prefer.