feat: Passkeys (WebAuthn) for feature/pre-new-admin - #542
Merged
hellopablo merged 18 commits intoSep 11, 2026
Merged
Conversation
Fixes the thirteen errors reported at level 1 and raises `.phpstan/config.neon` from level 0 to level 1 so they stay fixed. Three of them were real: - `User::create()` passed an undefined `$data` to `autoSaveExpandableFieldsExtract()`, which takes `array &$aData` by reference; passing an undefined variable to a typed by-reference parameter is a TypeError, so the method fatalled. It should have been `$aData`, as in `Common\Model\Base::create()`. - `User::update()` tested `$bPasswordUpdated`, which was never assigned, so the remember-me cookie was never refreshed after a password change despite the comment above it saying that is the intent. The variable is now set where the password is changed. - `mergeUpdateColumns()` read `$sColumn` after the loop it was assigned in, which is undefined when a table maps no columns. Made explicit with `end()`, preserving the existing behaviour, with a `@todo` recording that filtering on only the last mapped column is wrong for a table which references the user more than once. The rest are docblocks which claimed a non-nullable type from a `getById()` that can return null, immediately above the `empty()` check which proves otherwise, plus a redundant nested `empty()` and a `??` on a variable that cannot be defined. `composer analyse` gets `--memory-limit=1G`; 256M is not enough to reach level 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the WebAuthn library which the passkey ceremonies are built on, along with the extensions it needs: ext-openssl for key handling and signature verification, ext-mbstring for its string handling. ext-sodium is only suggested, as it is needed solely for authenticators which use Ed25519 keys. Also registers the `passkey` helper so `passkeysEnabled()`, `loadPasskeyAssets()` and the button helpers are available to app-overridden views. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stores one WebAuthn credential per row. `credential_id` holds the base64url encoded raw ID and is unique, so a credential cannot be registered twice. It is ascii rather than utf8mb4 because the spec allows raw IDs up to 1023 bytes; at 1400 characters a utf8mb4 unique index would exceed InnoDB's 3072 byte key limit. `user_handle` stores the opaque handle presented to the authenticator at registration, so verification can compare what the client sends against the value that credential was actually created with. Keeping it per row means rotating PRIVATE_KEY does not invalidate credentials which already exist. `public_key` is plaintext PEM: a public key is public by definition, and encrypting it would tie every passkey to PRIVATE_KEY rotation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The model provides the lookups the ceremonies need: by user, by credential ID, a count for the adoption nudge, and recording use after a successful assertion. The resource types the row and decodes the transports JSON. Every exception extends PasskeyException, so a caller can catch one type to handle any failure in a ceremony, or catch the specific subclasses when it needs to tell a stale challenge from a bad signature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Holds the WebAuthn logic: configuration, the two ceremonies, and the session challenge store. Every call into lbuchs is made from getWebAuthn() and the build/verify methods, so a change of library is confined to this file. getWebAuthn() deliberately returns a new instance each time, because the library mints and caches one challenge per instance and sharing one would re-issue a spent challenge. The build/verify methods touch neither the database nor the session, so they can be tested directly; the orchestration methods above them add storage and the challenge round trip. Two checks are ours rather than the library's. assertOriginAllowed() matches the whole origin exactly, where the library matches only a suffix of the host, which a lookalike domain would satisfy. And verifyAuthentication() compares the client's user handle against the stored one, because the library does not read it back. The user handle is an HMAC of the user's ID rather than a stored column, so no change to the `user` table is needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`loginWithPasskey()` mirrors `loginWithCredentials()`: the same brute force delay, lockout and suspension checks, the same generic failure message, and the same remember-me handling. It bypasses only the temporary and expired password checks, because no password took part in the login. An unrecognised credential cannot be attributed to a user, so it gets the delay and the generic message and nothing else. `recordLoginMethod()` notes how the session authenticated. It is called immediately before `setLoginData()` because that fires USER_LOG_IN synchronously and listeners need to be able to read the signal; the multi-factor module will use it to skip the challenge for a user-verified passkey login. `clearLoginData()` unsets the signal, covering the fail-closed path where the MFA module clears a half-finished login. `logout()` already destroys the session. No existing signature changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seven endpoints covering both ceremonies plus management. Only `challenge` and `assert` are reachable logged out; everything else manages an existing account. All of them 404 when passkeys are disabled, so the feature simply does not exist until it is switched on. There is no CSRF token on API routes, so the write endpoints lean on two things instead. Every POST must be `application/json`, which a cross-site form cannot send without a preflight; and the request must look same-origin, using the browser's own fetch metadata where it is sent and falling back to Origin/Referer where it is not. `attest` and `assert` are additionally bound by the session challenge and by the library's own origin check. `return_to` is restricted to this site: relative paths are resolved against it, and an absolute URL is only honoured when its host matches BASE_URL. Failures map to status codes that do not leak whether a credential exists; an unrecognised passkey and a bad signature both return the same generic 401. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds two ways in. A "Sign in with a passkey" button sits below the password controls behind a rule, because a passkey is a different way in rather than a variant of the password; and the identifier field gets the `webauthn` autocomplete token, which is what lets the browser offer a saved passkey in the field's own dropdown. The whole block stays hidden until the JavaScript confirms the browser supports WebAuthn, so an unsupported browser is never left with a rule and nothing beneath it. `isViewOverridden()` is added to the auth base controller and reused by `loadStyles()`, which was already making the same test inline. An app which has taken the login view over owns its own assets, so the module does not inject its JavaScript there; the `passkey` helper lets such an app opt back in with `loadPasskeyAssets()` and `passkeyLoginButton()`. `loadStyles()` keeps its signature; `isViewOverridden()` is additive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`auth/passkeys` lists a user's passkeys and lets them add, rename and remove one. Rename and remove are plain form posts so the page keeps working without JavaScript; only adding a passkey needs the browser API. Nothing is rendered when there are none, since the panel above already invites the user to add one. Below 768px the table stacks into a block per passkey, each cell carrying the heading it lost, because four columns and a text input cannot fit a phone and a table which scrolls sideways hides the controls people came for. After a password login a user with no passkeys is offered one, once per browser. Declining sets a cookie rather than user meta, because the capability being nudged towards belongs to the browser and not to the account. A browser with no platform authenticator answers on the user's behalf, so nobody is asked for something their device cannot provide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Passkeys tab when editing a user lists their credentials and offers a checkbox to revoke. Revocation happens in getPostData() rather than through the returned array, because a passkey is a row of its own rather than a column on the user, and each ID is checked against the user being edited so a stray value cannot revoke somebody else's. Removals are logged with the admin who made them. Settings > Authentication > Login gains the switch which turns passkeys on, plus the effective Relying Party ID and permitted origins shown read only. Those are derived from BASE_URL and can only be overridden in config, because changing the Relying Party ID invalidates every passkey already registered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Exposes window.NAILS.PASSKEY so other components can run a ceremony without repeating the encoding work, and wires up the declarative controls the views render. The MFA driver will use the same entry point, which is why the data attribute handlers are here rather than in a view. Three things are less obvious than they look: Controls are bound on a readyState check rather than DOMContentLoaded alone. The asset is deferred and a page may inject it later still, in which case waiting for the event would leave every control hidden. The browser reports "the user declined" and "the browser refused to ask" as the same NotAllowedError, so the two are told apart by whether the page had focus and how long the ceremony ran; nobody declines a prompt in a quarter of a second. And a hooked navigator.credentials can swallow the call entirely, leaving a promise which never settles, so ceremonies carry a watchdog which says so in the console after ten seconds and gives up at the authenticator's own timeout. Without these an environment problem is indistinguishable from nothing happening at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tests drive the real verification paths in lbuchs rather than mocking them. tests/Stub/WebAuthnFixture.php synthesises what an authenticator produces: a P-256 keypair from openssl, the COSE key, authenticator data, an fmt:none attestation object and client data, with assertions signed by the generated key. A minimal CBOR encoder sits alongside it, since the library only ships a decoder. That means a signature which verifies here verifies for the same reason a genuine one does, and the negative cases are genuine too: a forged signature, another key's assertion, a replayed counter, a missing user verification flag, a foreign user handle and a wrong origin are all rejected by the library's own code. No database and no network. Also covers the authenticator name lookup, the login-method signal round trip, the API's authentication gating, and that Routes::generate() is unchanged, since `auth/passkeys` relies on CI's controller mapping rather than a route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Written by PhpStorm after the composer update: the new vendor exclusions and include paths, plus a package prefix on the test source folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Relying Party panel used `alert--info`, which is the frontend convention. Admin's stylesheet only defines `alert-info`, so the panel rendered with no background, border or colour at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The model set no default sort column, so `getByUserId()` emitted no ORDER BY and row order was whatever the storage engine happened to return. Sorting by `id` keeps the list in the order the passkeys were added, which is what the view implies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pre-existing drift on this branch (unrelated to the passkeys work): Nails\Common\Interfaces\RouteGenerator::generate() declares an `array` return type, but this method didn't, which PHP only flags once the class is actually loaded. It stayed dormant until tests/RoutesTest.php exercised Routes::generate() directly, which crashed PHPUnit outright. develop already declares the return type correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
develop(17 commits,a37c309..cd620bf) ontofeature/pre-new-admin, adapted for this branch's older folder structure and string-based admin permissions: arrayreturn type onRoutes::generate(), a pre-existing gap on this branch that only surfaced once the new route test exercised the classNotes for review
Migration21on develop becameMigration19here, since this branch is missing two unrelated migrations (19/20) that exist on develop — numbers were kept sequential for this branch rather than matching develop's literal numbersPermission\Users\Edit::class,Permission\Settings\Login::class) to this branch's string permissions ('admin:auth:accounts:editOthers','admin:auth:settings:update:login'), matching the existing semantics on each branchassets/js/passkey.min.jsandassets/css/styles.min.cssvia webpack rather than trusting the cherry-picked minified output; both matched exactlyTest plan
./vendor/bin/phpunit— 218 tests, 413 assertions, all passingcomposer analyse(PHPStan level 1) — no new errors; the one remaining error (src/Admin/QuickAction/LoginAs.php) is a pre-existing, unrelated baseline issue on this branch🤖 Generated with Claude Code