test(mfa): end-to-end coverage, OpenAPI and documentation
This commit is contained in:
@@ -757,6 +757,93 @@ reference that lies. Scalar is vendored (`scalar.standalone.js`, served from
|
||||
reference page has to work on an air-gapped install with no outbound access at
|
||||
all - the same requirement licence verification already meets.
|
||||
|
||||
### Multi-factor authentication
|
||||
|
||||
Local and `hq`-sourced members can enrol TOTP and WebAuthn passkeys; OIDC users
|
||||
are exempt (`auth_source == "oidc"`), since their IdP owns authentication.
|
||||
|
||||
**A password that checks out mints a pending-login ticket, not a session with
|
||||
an `mfa_pending` flag.** The ticket is a Redis key (`km:mfa:<id>`, 5 min TTL)
|
||||
referenced by a separate `km_mfa_pending` cookie; only the `/auth/mfa/*` and
|
||||
`/auth/mfa/enrol/*` endpoints accept it, and it exchanges for a `km_session`
|
||||
only on a completed second factor or completed enrolment. A flag on `Session`
|
||||
would fail open - any route mounted under `auth.Middleware`, today's or a
|
||||
future one, could serve a half-authenticated user by forgetting to check it.
|
||||
A ticket fails closed: nothing under `auth.Middleware` recognises it at all,
|
||||
because it is never a `*Session`. `Session` itself gained `AMR []string`
|
||||
(`pwd`, `otp`, `webauthn`, `recovery`, `oidc`) and `StepUpAt *time.Time`; sign-in
|
||||
counts as a step-up, so `StepUpAt` is set at session creation.
|
||||
|
||||
**The pending ticket's attempt counter is an atomic Redis counter
|
||||
(`km:mfa:<id>:attempts`), not a field rewritten on the ticket document.** Two
|
||||
requests racing to fail a guess would otherwise both read the same `attempts`
|
||||
and both write it back incremented once, undercounting. `INCR` has no such
|
||||
race; the fifth failure deletes the ticket.
|
||||
|
||||
**The TOTP replay guard is keyed on the time step, not the code**:
|
||||
`km:totp:<user_id>:<step>`, `SET NX` with a 90s TTL. Keying on the code itself
|
||||
would let the same 6 digits be replayed across two different steps that
|
||||
happen to compute it (a 1-in-a-million collision, but a free one to close);
|
||||
keying on the step means a given 30-second window can be spent exactly once,
|
||||
which is what "single-use" actually means for a TOTP code.
|
||||
|
||||
**`user_mfa.totp_pending_enc` holds an unconfirmed TOTP secret and is
|
||||
deliberately not in `vantage-shared`'s `backup.ciphertextFields["user_mfa"]`**,
|
||||
which lists only `totp_secret_enc`. The confirmed secret is the one that
|
||||
authenticates anyone; an abandoned setup attempt (scanned once, never
|
||||
confirmed, replaced by the next `POST /me/mfa/totp/setup` call) is not worth
|
||||
widening the backup contract's surface for. `vantagectl verify`'s live probe
|
||||
therefore never touches it - this is intentional, not the same silent gap the
|
||||
ciphertext-field mirror otherwise guards against.
|
||||
|
||||
**Two new collections**, both in `ScopedCollections` so an instance purge
|
||||
removes them, both with a fatal index builder like `EnsureAuthIndexes`:
|
||||
|
||||
- `user_mfa` - one document per user who has started enrolment. Unique index
|
||||
`{instance_id, user_id}`. `totp_confirmed_at: nil` means setup started but
|
||||
TOTP is not active; "has MFA" means that field is set or the user owns a
|
||||
passkey.
|
||||
- `webauthn_credentials` - one document per passkey. Unique index
|
||||
`{instance_id, credential_id}`, plus `{instance_id, user_id}`. `sign_count`
|
||||
backs clone detection: a non-increasing non-zero count fails the assertion.
|
||||
|
||||
`require_mfa` (`models.Settings.RequireMFA bool`, `bson:"require_mfa"`)
|
||||
shipped in `vantage-shared` v0.7.0 - a plain bool because absent must mean off.
|
||||
Switching it on does not revoke existing sessions; they end at their normal
|
||||
24h TTL, and the next sign-in enforces enrolment for anyone with no factor yet.
|
||||
|
||||
**WebAuthn's RP ID is the request host with any port stripped**, resolved
|
||||
per-request rather than configured, the same way the org/host guard resolves
|
||||
an instance from `<slug>.vantage.<tld>`. A passkey is bound to the host it was
|
||||
registered on: moving a self-hosted instance to a new domain, or renaming a
|
||||
cloud instance (see "A rename moves the host" above), invalidates every
|
||||
passkey on it. TOTP and recovery codes are unaffected, since they carry no
|
||||
host binding. The docs say so; there is no migration path for a passkey
|
||||
across a host change.
|
||||
|
||||
**Step-up** (`auth.RequireStepUp()`) gates three existing sensitive routes -
|
||||
`POST /api/secrets/:group/reveal`, `GET /api/keys/:id/private-key`,
|
||||
`POST /api/console/connect` - plus the MFA-management endpoints that create or
|
||||
remove a factor. It passes when `StepUpAt` is within the last **ten minutes**,
|
||||
when the session's `AMR` contains `oidc` (the IdP's own session policy
|
||||
governs), or **when the request authenticated with an API token**
|
||||
(`TokenID != ""`). That last exemption is a known, accepted gap, not an
|
||||
oversight: a token has no human present to prompt for a second factor, so a
|
||||
token holding `secrets:read` or `keys:read` can reveal a secret or download a
|
||||
private key with no re-authentication at all. The mitigation is scoped,
|
||||
short-lived tokens, tracked separately in the gap review, not a code change
|
||||
here - a later reviewer should not "fix" this silently. `POST /api/me/step-up`
|
||||
takes `{totp}`, `{recovery}` or `{password}` (password only for a user with no
|
||||
MFA); `POST /api/me/step-up/webauthn/begin` and `/finish` do the same with a
|
||||
passkey. All three, like every unauthenticated MFA endpoint, sit behind
|
||||
`RateLimitAuth()` - a fixed Redis window, 20 requests/minute per
|
||||
`c.ClientIP()`, answering 429 with `Retry-After` - on the `RateLimitTokens`
|
||||
pattern but for sign-in and re-authentication rather than API tokens.
|
||||
|
||||
Library versions: `github.com/pquerna/otp` for TOTP, `github.com/go-webauthn/webauthn`
|
||||
**v0.18.1** for WebAuthn ceremonies, `qrcode` (npm) to draw the enrolment QR
|
||||
client-side so an air-gapped install needs nothing external.
|
||||
|
||||
### The public host
|
||||
|
||||
**vantage.hostxtra.co.uk is not served by this repository.** The marketing site
|
||||
@@ -967,6 +1054,11 @@ POST /auth/bootstrap /auth/login /auth/logout
|
||||
GET /auth/me
|
||||
GET /auth/providers # {local_enabled, providers:[{id,name,preset}]} - no issuer, client ID or secret
|
||||
GET /api/secrets/:group/values # bearer token (ESO)
|
||||
POST /auth/mfa/totp /auth/mfa/recovery # second factor against a pending-login ticket
|
||||
POST /auth/mfa/webauthn/begin /finish
|
||||
POST /auth/passkey/begin /auth/passkey/finish # passwordless sign-in
|
||||
POST /auth/mfa/enrol/totp/setup /confirm # ticket-scoped forced enrolment
|
||||
POST /auth/mfa/enrol/passkey/begin /finish
|
||||
```
|
||||
|
||||
Session-authed under `/api`:
|
||||
@@ -1012,6 +1104,12 @@ agent GET /agent/latest-version
|
||||
settings GET,PUT /settings · POST /settings/secrets-token (owner|admin)
|
||||
licence GET /license · POST /license (POST: self-hosted only)
|
||||
org GET,POST /org/users · PUT /org/users/:id/role · DELETE /org/users/:id
|
||||
DELETE /org/users/:id/mfa (owner|admin, step-up)
|
||||
mfa GET /me/mfa · POST /me/mfa/totp/setup (step-up)
|
||||
POST /me/mfa/totp/confirm · DELETE /me/mfa/totp (step-up)
|
||||
POST /me/mfa/recovery/regenerate (step-up)
|
||||
POST,PATCH,DELETE /me/passkeys[/begin,/finish,/:id] (step-up, except rename)
|
||||
POST /me/step-up · POST /me/step-up/webauthn/begin /finish
|
||||
providers GET,POST /auth/providers · PUT,DELETE /auth/providers/:id
|
||||
POST /auth/providers/:id/{test,ack-notice} · GET /auth/presets (owner|admin)
|
||||
tokens GET /tokens · GET /tokens/scopes · POST /tokens · DELETE /tokens/:id
|
||||
@@ -1067,7 +1165,7 @@ plane, each of which this codebase enforces:
|
||||
|
||||
## MongoDB Collections
|
||||
|
||||
`servers` · `keys` · `assignments` · `orgs` · `users` · `auth_providers` · `settings` · `secrets` · `workflows` · `workflow_steps` · `workflow_runs` · `workflow_log_lines` · `workflow_log_seq` · `monitors` · `incidents` · `monitor_rollups` · `notification_channels` · `console_sessions` · `audit_logs` · `server_packages` · `vuln_findings` · `vuln_alert_rules` · `vulndb_meta` · `server_workloads` · `api_tokens` · `status_pages` · `status_incidents` · `maintenance_windows` · `patch_policies` · `patch_runs` · `patch_run_outputs` · `migrations`
|
||||
`servers` · `keys` · `assignments` · `orgs` · `users` · `auth_providers` · `settings` · `secrets` · `workflows` · `workflow_steps` · `workflow_runs` · `workflow_log_lines` · `workflow_log_seq` · `monitors` · `incidents` · `monitor_rollups` · `notification_channels` · `console_sessions` · `audit_logs` · `server_packages` · `vuln_findings` · `vuln_alert_rules` · `vulndb_meta` · `server_workloads` · `api_tokens` · `status_pages` · `status_incidents` · `maintenance_windows` · `patch_policies` · `patch_runs` · `patch_run_outputs` · `user_mfa` · `webauthn_credentials` · `migrations`
|
||||
|
||||
Every document except `migrations` carries `org_id`. Struct definitions are the source of truth - see `server/internal/models/`.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user