topaLE/vaultwarden - vaultwarden - Gitea: Git with a cup of tea

Author	SHA1	Message	Date
Zaid Marji	247a8905d8	Fix correctness, test reliability, and observability issues Server hardening: - accounts.rs: reject duplicate ids in `passkey_unlock_data` before they collapse through the HashSet superset check and silently double-apply with the second write winning. - accounts.rs: re-check the PRF credential set immediately before `set_password` commits the new account key. A concurrent enrol between the validation snapshot and the rewrap loop would otherwise leave the new credential's `encrypted_user_key` wrapped under the OLD akey, permanently broken once the rotation commits. - mod.rs: drop the legacy bare-`PasskeyRegistration` fallback in `passkey_registration_challenge_state` — no writer produces that shape, and the fallback permitted token-less finishes against a hypothetical un-tokened row. - mod.rs: switch token compares in both challenge-state extractors to `crypto::ct_eq` (matches the `cred_id` pattern already used at mod.rs:1424). - mod.rs: normalise `passkey_assertion_challenge_state`'s deserialise error to the generic "Invalid assertion challenge" rather than leaking the underlying serde shape. - mod.rs: document why `post_api_webauthn_delete` is intentionally exempt from the SSO_ONLY gate (delete narrows capability, never grants it; session is still SSO-authed). - mod.rs: document that the post-`start_passkey_registration` mutation of `authenticator_selection.require_resident_key` is a client-side hint only (webauthn-rs destructures the field as unused), so readers don't mistake it for a server-enforced policy. Observability: - web_authn_credential.rs / two_factor.rs: log Diesel errors from the SELECT+DELETE transactions in `WebAuthnLoginChallenge::take` and `TwoFactor::take_by_user_and_type` before collapsing to `None`. The prior `.unwrap_or(None)` made degraded DB behaviour (deadlock, lock timeout, conn drop) indistinguishable from a normal stale-token rejection. - identity.rs: log the failure path of `passkey_transports`'s `serde_json::to_value(passkey)` instead of silently emitting `Transports: []`. Clients can't otherwise distinguish "authenticator reported no transports" from "we failed to serialise the passkey". UX: - vaultwarden.scss.hbs: re-add the `.vw-passkey-login` hide under `sso_enabled && sso_only`. Without it the login page renders the "Log in with passkey" button under SSO_ONLY, but the server gate at `identity.rs:1250` rejects the click — UX dead end. Test reliability: - passkey.spec.ts: the "Non-PRF passkey" test now logs out + logs back in via passkey between enrol and the lock-screen check. Without this refresh the lock-screen reads the pre-enrol cached `/api/sync` and the assertion passes for the wrong reason (cached webAuthnPrfOptions was empty before enrol regardless of what the server stored). - passkey.spec.ts: guard the three describe blocks that restart the vault container (`Passkey grant is rejected when SSO_ONLY is on`, `Passkey enrolment is rejected when SSO_ONLY is on`, `Passkey login rejects forged unverified-email handles`) on `useExternalVault` so `PW_USE_EXTERNAL_VAULT=1` (host-cargo iteration) cleanly skips them instead of colliding with the host process on port 8003. - passkey.spec.ts: the SSO_ONLY-enrolment `afterAll` now also blanks `sso_authority`, `sso_client_id`, `sso_client_secret` — the previous reset left those placeholders in `config.json` and contaminated any later test that toggled `sso_enabled=true`. - passkey.spec.ts: the bearer-sniffer in the SSO_ONLY-enrolment beforeAll now filters on `url.includes('/api/')` instead of capturing whatever token happened to fly last during createAccount; mirrors the disciplined sniffer in `account_lifecycle.spec.ts:178` and insulates the test from web-vault request-order churn. - 2fa.ts: export `resetTotpTimeStep()`; call it from the `test.beforeEach` in `account_lifecycle.spec.ts` and `passkey.spec.ts` so the module-scoped `lastSubmittedTotpTimeStep` doesn't leak across tests / projects under `workers: 1` (would otherwise force a 30s wait before the first TOTP of project N+1 based on project N's history). - global-utils.ts: document the `resetDB=false` contract honestly — docker recreates the Vaultwarden container on any env-var change in the compose `environment:` block, dropping the tmpfs sqlite DB along with the in-process RSA key. - login.smtp.spec.ts: drop the redundant "Dismiss extension prompts" step from the 2fa test. `logUser` already calls `utils.ignoreExtension` and lands on /vault, so the explicit "Add it later" click timed out on a button the prior step had already dismissed. Test coverage additions: - accounts.rs rotation validator: add a regression test for the duplicate-id rejection in `passkey_unlock_data`. With a HashSet-only superset check the rewrap loop in `post_rotatekey` could silently apply two updates to the same credential id (second blob wins); the test pins the new len-equality guard. - mod.rs registration-challenge state: replace `legacy_registration_challenge_rejects_finish_token` (which asserted a "legacy bare-`PasskeyRegistration` accepted without token" fallback no longer present) with `registration_challenge_rejects_unwrapped_legacy_state`, matching the assertion-side test's stricter contract: any blob that's not the `{token, state}` wrapper is rejected regardless of whether a token is sent. Pins the token-binding bypass closure.	2 weeks ago
Zaid Marji	58bbc2d533	playwright: passkey UI flow tests + SSO_ONLY denial coverage UI-driven Playwright coverage for the passkey login + management flows: enrolment via the Account page, PRF unlock toggle, login via the "Log in with passkey" affordance, removal, and post-removal fall-through to the master-password unlock path. The UI suite complements the request-level coverage in `passkey.spec.ts` by exercising the bundled web vault's connector iframe and the account-page CDP virtual-authenticator interaction. SSO_ONLY denial coverage extends the suite with two tests next to the existing webauthn-grant denial: - 'GET assertion-options (login challenge) denied with an SSO-mentioning message' — sibling to the existing webauthn-grant test, covers the unauthenticated entry point of the discoverable-login flow at `src/api/identity.rs:1250`. The SPA fetches this BEFORE invoking the WebAuthn ceremony, so the server-side gate here is what prevents an attacker from attempting passkey login even with a credential a victim has previously enrolled. - 'POST /api/webauthn/attestation-options denied with an SSO-mentioning message' — covers the deny-by-default gate on the enrolment endpoint at `src/api/core/mod.rs:308`. The handler is authenticated, so the test provisions a user via the UI under default config, sniffs the Bearer header from the SPA's post-login /api/sync, then flips `sso_enabled`/`sso_only` at runtime via `POST /admin/config` instead of restarting the vault container. The runtime toggle avoids the tmpfs wipe an env-change-driven container recreate would trigger, keeping the user, the RSA signing key, and the pre-issued token all valid for the test.	3 weeks ago
Zaid Marji	524194bbf4	config: advertise pm-2035-passkey-unlock feature flag The bundled web vault gates its lock-screen "Unlock with passkey" affordance on the `pm-2035-passkey-unlock` feature flag in `/api/config`'s `featureStates`. Without it, `WebAuthnPrfUnlockService.isPrfUnlockAvailable` short-circuits to `false` and the button never renders even for users with a PRF-enabled passkey enrolled. Vaultwarden supports PRF passkey unlock end-to-end (the `userDecryption.webAuthnPrfOptions` blob in `/api/sync` feeds the client-side unwrap), so the flag must be advertised as enabled. Extracts `build_feature_states` from the `/api/config` handler so the feature-state assembly is unit-testable without `CONFIG` initialisation, and pins both the new flag and the existing `pm-19148-innovation-archive` companion via tests in `src/api/core/mod.rs` + a `/api/config` Playwright probe in `passkey.spec.ts`.	3 weeks ago
Zaid Marji	6f2751ee79	Align UserDecryption response shapes with upstream Bitwarden Upstream's `IdentityTokenResponse.UserDecryptionOptions` model carries a singular `WebAuthnPrfOption`, populated solely from `UserDecryptionOptionsBuilder.WithWebAuthnLoginCredential` after a successful passkey assertion. Upstream's `SyncResponseModel.UserDecryption` model carries the plural `WebAuthnPrfOptions` array, populated for every PRF-enabled credential the user owns. The Bitwarden client reads each at a different point in the flow: - Singular drives the immediate post-passkey-login vault decryption (the client combines it with the PRF secret from the assertion it just performed). - Plural drives the lock-screen "Unlock with passkey" option (read from disk state populated at sync time). Vaultwarden previously emitted only the singular on the webauthn-grant response and nothing on /sync, so the lock-screen option never appeared even when the user had a PRF-enabled credential. This commit: - Adds `webAuthnPrfOptions` (plural array) to `/sync` via a new `build_webauthn_prf_options(&[WebAuthnCredential])` helper. - Extracts the singular emission into `build_webauthn_login_prf_option` and a wrapper `build_webauthn_login_response` that applies it to the output of `authenticated_response`. The wrapper makes the call site unit-testable: a regression that removes the call site trips the dead-code lint at build time (the helper's only caller). - Pins both helpers and the response-augmentation wrapper with unit tests covering shape (field names + values) and behaviour (idempotency, no-op for non-Enabled credentials, untouched payload for unsupported credentials). - Adds a playwright integration test that pins the wire-level response shapes for password-grant /connect/token and /sync against the upstream contract. The login responses for `password` and `client_credentials` grants no longer emit any PRF field (matching upstream — the singular is only populated on webauthn grant, and the plural doesn't exist on that response model at all).	2 weeks ago
Zaid Marji	c489186e4f	Add playwright tests for passkey login `passkey.spec.ts` exercises the unauthenticated and authorization-required surface that doesn't need a virtual authenticator: - `GET /identity/accounts/webauthn/assertion-options` returns the documented shape (`Content-Type: application/json`, `options` + `userVerification` + a non-empty `token`). The token format is intentionally not pinned: Vaultwarden mints a UUID, upstream Bitwarden mints a `DataProtectorTokenable`; both are opaque from the client's view. - Five back-to-back calls return five distinct tokens AND five distinct challenges — a refactor that re-used either would let an attacker replay. - The `grant_type=webauthn` token endpoint returns a generic auth-failed message for an unknown token, a malformed deviceresponse, and a structurally-valid but unsignable assertion. The regex accepts both Vaultwarden's "Passkey authentication failed." and upstream Bitwarden's "Invalid credential." — the security contract is the byte-equality between failure branches (oracle defense), not the surface text. - Every webauthn-management endpoint (`GET /api/webauthn`, attestation / assertion options, finish, update, delete) rejects anonymous callers AND callers with a garbage Bearer with 401. - Missing-required-field requests to the webauthn grant are rejected before the handler body runs (token / deviceresponse / client_id / scope). The specific rejection text differs between projects so we only assert that the response is an error. - The web vault renders the "Log in with passkey" entry point. Security-gate coverage covers forged user-handle attempts against disabled and unverified accounts plus the SSO_ONLY webauthn grant gate. The forged-handle cases create real target users but submit intentionally unsignable assertions, asserting the response stays byte-equal to the unknown-user baseline before WebAuthn verification succeeds. Adds the docker-compose environment passthrough needed for per-describe vault restarts with SIGNUPS_VERIFY and SSO_ONLY test configs. README: document the Playwright image's bake-in behavior — the Dockerfile copies `tests/` in at build time, so local edits to `*.spec.ts` are not picked up by `docker compose run Playwright` until the image is rebuilt. Verified empirically: an in-place rename of a `test('…')` title is invisible to `run` until `build Playwright` is invoked, and absolute paths through the mounted `..:/project` volume don't override Playwright's config-derived `testDir`. Add a short note next to the existing "force a rebuild" command. The spec is Firefox-compatible and runs unmodified under the existing playwright project matrix.	3 weeks ago