diff --git a/.env.example b/.env.example index 07a7fc6b6..9794cbc0a 100644 --- a/.env.example +++ b/.env.example @@ -60,6 +60,29 @@ ADMIN_PASSWORD=your_secure_admin_password USER_EMAIL=user@libredb.org USER_PASSWORD=your_secure_user_password +# TWO-FACTOR AUTHENTICATION (TOTP) — optional, local provider only +# ============================================ +# Base32 secret (RFC 4648: A-Z and 2-7). When set, that account must present a +# 6-digit code from an authenticator app after its password. Opt-in per account: +# set one, both, or neither. Under NEXT_PUBLIC_AUTH_PROVIDER=oidc the login page +# shows no password form, so MFA belongs at the identity provider (docs/OIDC.md) +# — but POST /api/auth/login stays reachable whenever a password is ALSO +# configured, and these secrets guard that route in every mode. +# +# Generate one, then enrol it in your app of choice: +# openssl rand 20 | base32 | tr -d '=' # 160-bit secret, per RFC 4226 +# Minimum 26 base32 characters, so the decoded secret carries the 128 bits +# RFC 4226 requires. A value that is not base32, or shorter than that, stops +# login with a clear 503 naming the variable rather than silently disabling or +# silently weakening the second factor. Blank the variable to turn MFA off. +# +# The placeholder below is deliberately not base32, like every other secret in +# this file. A published example secret is the one value that must never be left +# in place: the account would read as protected everywhere while anyone could +# compute its codes. Uncommented as-is, this earns the 503 instead. +# ADMIN_TOTP_SECRET=your_base32_secret_from_the_command_above +# USER_TOTP_SECRET= + # JWT Secret for session management (min 32 characters) # Generate with: openssl rand -base64 32 # A shorter value stops the server at startup (exit code 1) instead of booting diff --git a/README.md b/README.md index c36e3cb19..8d01f5a2b 100644 --- a/README.md +++ b/README.md @@ -918,6 +918,7 @@ extraEnvFrom: | [Local models](docs/llms/README.md) | Which local model can actually drive an agent run, measured across three workflows, one page per model | | [Agent Runtime](docs/AGENT.md) | Agent behaviour, bounds, deployment and known limitations | | [OIDC SSO](docs/OIDC.md) | SSO setup (Auth0, Keycloak, Okta, Azure AD, Zitadel, Google) + subsystem internals & security model | +| [Two-Factor Auth](docs/MFA.md) | TOTP on the local provider — generating a secret, enrolling an app, Docker/Helm wiring, and what it does not cover | | [Theming Guide](docs/ui/theming.md) | CSS theming, dark mode, and styling customization | | [Login Page](docs/ui/login-page.md) | Login page layout, OIDC/local modes, and design system | | [Editor Docs](docs/editor/) | SQL editor internals — completion, performance, query optimization | diff --git a/charts/libredb-studio/Chart.yaml b/charts/libredb-studio/Chart.yaml index c9121f2d9..0deb48228 100644 --- a/charts/libredb-studio/Chart.yaml +++ b/charts/libredb-studio/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: libredb-studio description: Web-based SQL IDE for cloud-native teams supporting sixteen engines - PostgreSQL, MySQL, SQLite, DuckDB, Oracle, SQL Server, MongoDB, Redis, Couchbase, ClickHouse, Apache Druid, Elasticsearch, OpenSearch, Apache Trino, Apache Cassandra and libSQL type: application -version: 0.1.62 +version: 0.1.63 appVersion: "0.15.0" kubeVersion: ">=1.26.0-0" home: https://github.com/libredb/libredb-studio diff --git a/charts/libredb-studio/README.md b/charts/libredb-studio/README.md index ae91bc6f9..a53b8b6e6 100644 --- a/charts/libredb-studio/README.md +++ b/charts/libredb-studio/README.md @@ -40,7 +40,7 @@ helm install libredb libredb/libredb-studio \ ```bash helm install libredb oci://ghcr.io/libredb/charts/libredb-studio \ - --version 0.1.62 \ + --version 0.1.63 \ --set secrets.jwtSecret=$(openssl rand -base64 32) \ --set secrets.adminPassword=MyAdmin123 ``` @@ -147,6 +147,36 @@ browser still speaks `https`, so it accepts the cookie. `false` is only for the the browser's own connection is cleartext - and it means session cookies travel in cleartext, so keep it to a trusted network. `true` forces the flag on. +## Two-Factor Authentication (TOTP) + +Optional, local-provider only, and opt-in per account. Set a base32 secret and that account must +present a 6-digit authenticator code after its password: + +```bash +# Generate it, enrol the printed value in your authenticator app, then install. +ADMIN_TOTP_SECRET="$(openssl rand 20 | base32 | tr -d '=')" +echo "$ADMIN_TOTP_SECRET" + +helm upgrade --install libredb libredb/libredb-studio \ + --set secrets.adminPassword=MyAdmin123 \ + --set secrets.adminTotpSecret="$ADMIN_TOTP_SECRET" +``` + +No example secret is printed here on purpose. One that looks real invites being copied and left in +place, and a second factor whose secret is published is worse than none. + +The value travels in the chart's Secret and is referenced from the pod, so it never appears in the +Deployment spec - which is why `extraEnv` is the wrong tool for it. Both `ADMIN_TOTP_SECRET` and +`USER_TOTP_SECRET` refs are always optional, including in strict mode, so a second factor nobody +asked for can never keep the pod from starting. `values.schema.json` applies the same test the app +does, base32 and at least the 128 bits RFC 4226 requires, so a bad secret fails at install time +rather than at the login screen. + +Under `authProvider=oidc` the login page shows no password form and MFA belongs to the identity +provider - but `POST /api/auth/login` stays reachable whenever `secrets.adminPassword` is also set, +and this guards that route in every mode. Full setup, enrolment and recovery: +[`docs/MFA.md`](../../docs/MFA.md). + ## OIDC SSO ```bash @@ -542,7 +572,7 @@ helm install libredb libredb/libredb-studio \ Your external secret is referenced with these keys (customizable via `secrets.existingSecretKeys`): - `jwt-secret`, `admin-password` — required in strict mode (the pod waits for them); in zero-config mode missing ones are generated at first start -- Optional: `admin-email`, `user-email`, `user-password` (the non-admin account exists only when `user-password` is set), `llm-api-key`, `oidc-client-id`, `oidc-client-secret`, `storage-postgres-url` +- Optional: `admin-email`, `user-email`, `user-password` (the non-admin account exists only when `user-password` is set), `admin-totp-secret`, `user-totp-secret`, `llm-api-key`, `oidc-client-id`, `oidc-client-secret`, `storage-postgres-url` ## Upgrading @@ -586,6 +616,8 @@ helm uninstall libredb | `secrets.adminPassword` | Admin password | `""` | | `secrets.userEmail` | User email | `user@libredb.org` | | `secrets.userPassword` | User password (optional; enables the non-admin account) | `""` | +| `secrets.adminTotpSecret` | Base32 TOTP secret for the admin account (optional second factor) | `""` | +| `secrets.userTotpSecret` | Base32 TOTP secret for the user account (optional second factor) | `""` | | `secrets.existingSecret` | Use existing Secret | `""` | | `config.bindAddress` | Container bind address (`HOSTNAME`): empty lets the image resolve one, preferring a verified dual-stack `::`; `::` forces it; `0.0.0.0` pins IPv4 | `""` | | `config.storageProvider` | Storage: local, sqlite, postgres | `local` | diff --git a/charts/libredb-studio/templates/deployment.yaml b/charts/libredb-studio/templates/deployment.yaml index 506227cdd..40ef3391c 100644 --- a/charts/libredb-studio/templates/deployment.yaml +++ b/charts/libredb-studio/templates/deployment.yaml @@ -181,6 +181,26 @@ spec: key: {{ .Values.secrets.existingSecretKeys.userPassword }} optional: true {{- end }} + {{- /* TOTP second factor. Always optional: MFA is opt-in per account, so a missing + key must never keep the pod from starting, and the app treats an absent value + as "no second factor". Keyed separately from the passwords rather than nested + under them so an operator can rotate a secret without touching a credential. */}} + {{- if or .Values.secrets.adminTotpSecret .Values.secrets.existingSecret }} + - name: ADMIN_TOTP_SECRET + valueFrom: + secretKeyRef: + name: {{ include "libredb-studio.secretName" . }} + key: {{ .Values.secrets.existingSecretKeys.adminTotpSecret }} + optional: true + {{- end }} + {{- if or .Values.secrets.userTotpSecret .Values.secrets.existingSecret }} + - name: USER_TOTP_SECRET + valueFrom: + secretKeyRef: + name: {{ include "libredb-studio.secretName" . }} + key: {{ .Values.secrets.existingSecretKeys.userTotpSecret }} + optional: true + {{- end }} {{- if or .Values.secrets.llmApiKey .Values.secrets.existingSecret }} - name: LLM_API_KEY valueFrom: diff --git a/charts/libredb-studio/templates/secret.yaml b/charts/libredb-studio/templates/secret.yaml index feaed82e8..d0b4dee51 100644 --- a/charts/libredb-studio/templates/secret.yaml +++ b/charts/libredb-studio/templates/secret.yaml @@ -26,6 +26,12 @@ data: {{ .Values.secrets.existingSecretKeys.userEmail }}: {{ .Values.secrets.userEmail | b64enc | quote }} {{ .Values.secrets.existingSecretKeys.userPassword }}: {{ .Values.secrets.userPassword | b64enc | quote }} {{- end }} + {{- if .Values.secrets.adminTotpSecret }} + {{ .Values.secrets.existingSecretKeys.adminTotpSecret }}: {{ .Values.secrets.adminTotpSecret | b64enc | quote }} + {{- end }} + {{- if .Values.secrets.userTotpSecret }} + {{ .Values.secrets.existingSecretKeys.userTotpSecret }}: {{ .Values.secrets.userTotpSecret | b64enc | quote }} + {{- end }} {{- if .Values.secrets.llmApiKey }} {{ .Values.secrets.existingSecretKeys.llmApiKey }}: {{ .Values.secrets.llmApiKey | b64enc | quote }} {{- end }} diff --git a/charts/libredb-studio/values.schema.json b/charts/libredb-studio/values.schema.json index 3c94e465c..534f4bb1a 100644 --- a/charts/libredb-studio/values.schema.json +++ b/charts/libredb-studio/values.schema.json @@ -103,6 +103,16 @@ "type": "string", "description": "User account password" }, + "adminTotpSecret": { + "type": "string", + "pattern": "^$|^[-\\s]*(?:[A-Za-z2-7][-\\s]*){26,}=*$", + "description": "Admin TOTP secret: empty (no second factor) or base32 (RFC 4648: A-Z and 2-7; spaces, hyphens and trailing = are stripped), at least 26 characters so the decoded secret carries the 128 bits RFC 4226 requires" + }, + "userTotpSecret": { + "type": "string", + "pattern": "^$|^[-\\s]*(?:[A-Za-z2-7][-\\s]*){26,}=*$", + "description": "User TOTP secret: empty (no second factor) or base32 (RFC 4648: A-Z and 2-7; spaces, hyphens and trailing = are stripped), at least 26 characters so the decoded secret carries the 128 bits RFC 4226 requires" + }, "llmApiKey": { "type": "string", "description": "LLM API key" diff --git a/charts/libredb-studio/values.yaml b/charts/libredb-studio/values.yaml index 472ecf7d4..16d74a79f 100644 --- a/charts/libredb-studio/values.yaml +++ b/charts/libredb-studio/values.yaml @@ -56,6 +56,8 @@ secrets: adminPassword: admin-password userEmail: user-email userPassword: user-password + adminTotpSecret: admin-totp-secret + userTotpSecret: user-totp-secret llmApiKey: llm-api-key oidcClientId: oidc-client-id oidcClientSecret: oidc-client-secret @@ -75,6 +77,17 @@ secrets: # -- Regular user account password. Optional: the non-admin account exists only when set; # it is never generated. userPassword: "" + # -- Base32 TOTP secret for the admin account (RFC 4648: A-Z and 2-7), at least 26 characters + # so the decoded secret carries the 128 bits RFC 4226 requires. Optional and + # local-provider only: when set, admin login requires a 6-digit authenticator code after the + # password. A value that is not base32 stops login with a clear 503 rather than silently + # dropping the second factor. Under config.authProvider=oidc the login page shows no password + # form and MFA belongs to the identity provider, but this still guards POST /api/auth/login + # whenever a password is also set. Generate one with: openssl rand 20 | base32 | tr -d '=' + adminTotpSecret: "" + # -- Base32 TOTP secret for the regular user account. Same rules as adminTotpSecret; inert + # unless userPassword is also set, since without it there is no user account to protect. + userTotpSecret: "" # -- LLM API key (optional, for AI features) llmApiKey: "" # -- OIDC client ID (required when authProvider=oidc) diff --git a/docs/MFA.md b/docs/MFA.md new file mode 100644 index 000000000..26335ff45 --- /dev/null +++ b/docs/MFA.md @@ -0,0 +1,256 @@ +# Two-Factor Authentication (TOTP) — LibreDB Studio + +LibreDB Studio can require a time-based one-time password (TOTP) after the password on the **local** +auth provider. It is opt-in per account, configured entirely through environment variables, and +verified against RFC 6238 — so any standard authenticator app works: Google Authenticator, Authy, +1Password, Bitwarden, Aegis, KeePassXC, and the rest. + +> **Using SSO instead?** For most teams that is the better answer: under +> `NEXT_PUBLIC_AUTH_PROVIDER=oidc` the login page shows no password form, and your identity +> provider already enforces MFA, passkeys, device trust and conditional access — Studio consumes +> the result. See [`docs/OIDC.md`](OIDC.md). Note that switching to OIDC does not *disable* +> `POST /api/auth/login`; see [Running both](#running-both) below. + +--- + +## Table of Contents + +- [Quick Start](#quick-start) +- [Configuration Reference](#configuration-reference) + - [Running both](#running-both) +- [Deploying with Docker](#deploying-with-docker) +- [Deploying with Helm](#deploying-with-helm) +- [How it works](#how-it-works) +- [Rate limiting and lockout](#rate-limiting-and-lockout) +- [Troubleshooting](#troubleshooting) +- [What this does and does not protect](#what-this-does-and-does-not-protect) + +--- + +## Quick Start + +### 1. Generate a secret + +A TOTP secret is base32 (RFC 4648: the letters `A`–`Z` and the digits `2`–`7`). 160 bits is what +RFC 4226 recommends: + +```bash +ADMIN_TOTP_SECRET="$(openssl rand 20 | base32 | tr -d '=')" +echo "$ADMIN_TOTP_SECRET" +``` + +Keep that shell open: every step below refers to `$ADMIN_TOTP_SECRET`, so the value never has to be +pasted anywhere it can be forgotten. This page prints no example secret on purpose. A real-looking +one invites being copied and left in place, and a second factor whose secret is published is worse +than none: every screen still says the account is protected while anyone can generate its codes. + +`base32` comes with GNU coreutils. On a machine without it, any authenticator app can generate a +secret for you — create a manual entry and copy the key it shows. + +### 2. Set it on the account + +```env +NEXT_PUBLIC_AUTH_PROVIDER=local +ADMIN_EMAIL=admin@libredb.org +ADMIN_PASSWORD=your_secure_admin_password +ADMIN_TOTP_SECRET=the_secret_you_generated_in_step_1 +``` + +Both values above are placeholders, and neither is valid for its variable: the TOTP one is not +base32, so a file left exactly like this stops login with a `503` naming the variable rather than +quietly installing a factor anyone can compute. + +Restart the server. The variable is read per login attempt, so nothing is cached across a restart. + +### 3. Enrol the secret in your authenticator + +Add a manual (key-based) entry with: + +| Field | Value | +|---|---| +| Account | your admin email | +| Key | the secret from step 1 | +| Type | Time-based | +| Digits | 6 | +| Period | 30 seconds | +| Algorithm | SHA-1 | + +Those are every app's defaults, so in practice you only paste the key. + +Prefer to scan a QR code? Build the standard URI yourself and render it with any offline QR tool — +Studio does not mint one, because doing so would mean the server handing the shared secret back +over HTTP after startup: + +``` +otpauth://totp/LibreDB%20Studio:admin@libredb.org?secret=YOUR_SECRET_HERE&issuer=LibreDB%20Studio +``` + +### 4. Sign in + +Enter your email and password as usual. The form then asks for the 6-digit code, and the session is +created only once that code verifies. + +--- + +## Configuration Reference + +| Variable | Required | Description | +|---|---|---| +| `ADMIN_TOTP_SECRET` | No | Base32 secret for the admin account. Absent or empty = no second factor. | +| `USER_TOTP_SECRET` | No | Base32 secret for the optional non-admin account. Inert unless `USER_PASSWORD` is also set — with no password there is no user account to protect. | + +**Formatting is forgiving, content is not.** Lowercase, spaces, hyphens and `=` padding are all +normalized away, so you can paste a secret exactly as your password manager displays it. Two things +are rejected outright, both as a **misconfiguration, not a disabled factor**: a value containing +anything outside the base32 alphabet, and a value that decodes to fewer than 16 bytes, which is the +128-bit minimum RFC 4226 requires (26 base32 characters; the generator in step 1 gives you 32). +Either one stops login with a `503` naming the offending variable, rather than silently letting the +password through, rejecting every correct code, or leaving a second factor too small to be worth +having. To turn MFA off, blank or unset the variable. + +Each account is independent — protect the admin and leave an automation-owned user account on a +password alone if that is what you need. + +### Running both + +`NEXT_PUBLIC_AUTH_PROVIDER=oidc` changes what the login page renders; it does not disable +`POST /api/auth/login`. If a deployment sets `ADMIN_PASSWORD` *and* runs OIDC, that route remains a +working way in — one that never touches your identity provider, and so never meets the MFA policy +you configured there. Two ways to close it, and they compose: + +- Leave `ADMIN_PASSWORD` unset under OIDC. Nothing generates it in OIDC mode, and with no password + the route can only answer `503`. +- Set `ADMIN_TOTP_SECRET` anyway. These variables are honoured in every mode, so the local route + keeps a second factor even when the intended path is SSO. + +--- + +## Deploying with Docker + +```bash +docker run -d -p 3000:3000 \ + -e JWT_SECRET="$(openssl rand -base64 32)" \ + -e ADMIN_PASSWORD=your_secure_admin_password \ + -e ADMIN_TOTP_SECRET="$ADMIN_TOTP_SECRET" \ + ghcr.io/libredb/libredb-studio:latest +``` + +Prefer a file or a secret store over an inline `-e` for the secret itself: anything on the command +line is visible to `docker inspect` and to the shell history of whoever ran it. + +--- + +## Deploying with Helm + +The chart carries the secret in its Kubernetes `Secret` and references it from the pod, so the value +never appears in the Deployment spec: + +```bash +helm install libredb-studio oci://ghcr.io/libredb/charts/libredb-studio \ + --set secrets.adminPassword=MyAdmin123 \ + --set secrets.adminTotpSecret="$ADMIN_TOTP_SECRET" +``` + +Bringing your own Secret works too — add the keys `admin-totp-secret` and `user-totp-secret` +(rename them through `secrets.existingSecretKeys`) to the Secret named by `secrets.existingSecret`. +Both references are optional, so an existing Secret that predates this feature keeps working +untouched. + +Do not use `extraEnv` for this. It writes the literal secret into the Deployment's pod spec, where +anyone with `get deployments` can read it — a wider audience than the password it protects. + +--- + +## How it works + +``` +Browser → POST /api/auth/login {email, password} +Server → password verified (constant time) → account has a TOTP secret +Server → 401 {mfaRequired: true} ← no session is created +Browser → POST /api/auth/login {email, password, totp} +Server → code verified → step marked spent → JWT session cookie +``` + +| Parameter | Value | +|---|---| +| Algorithm | HMAC-SHA-1 (RFC 6238 §1.2 default; the only algorithm apps interoperate on for a bare `otpauth` URI) | +| Digits | 6 | +| Period | 30 seconds | +| Accepted skew | ±1 step, so a code stays usable for up to 90 seconds | +| Replay | An accepted `(account, step)` pair is spent and cannot be reused (RFC 6238 §5.2) | + +Verification lives in [`src/lib/totp.ts`](../src/lib/totp.ts) with no third-party dependency: HOTP is +a truncated HMAC and base32 is a 32-character alphabet, and the one code path that exists to raise +the cost of a compromise is a poor place to add supply-chain surface. + +SHA-1 here is correct and must not be "upgraded". The construction's security rests on HMAC, which +does not depend on the collision resistance SHA-1 lost, and changing it would break every +authenticator app. + +--- + +## Rate limiting and lockout + +Being **asked** for a code costs nothing — it is the first half of a two-request flow, not a failed +attempt, so ordinary logins never eat into your budget. + +Submitting a **wrong** code does count, against both login buckets: `RATE_LIMIT_LOGIN_MAX` (5 per +5 minutes, per client address) and `RATE_LIMIT_LOGIN_ACCOUNT_MAX` (20 per 5 minutes, per account). +Guessing a 6-digit code is therefore bounded to a few dozen tries per window against roughly a +million values. + +The client bucket is keyed on the **address**, not the account, so wrong codes spend a budget the +whole address shares. Measured: five wrong codes for the admin from one address, then a correct +password for the *other* account from that same address, answers `429` for the rest of the window. +This matters for the common split of protecting the admin and leaving an automation-owned account +on a password alone: behind one NAT or one ingress they draw on the same bucket, so an admin whose +phone clock has drifted can stall the automation for five minutes. Give the automation its own +egress address, or raise `RATE_LIMIT_LOGIN_MAX`, if that coupling is unacceptable. + +Locked out of your own account? The secret is an environment variable, so recovery is the same as +for a lost password: blank `ADMIN_TOTP_SECRET` and restart. There are no recovery codes, and none +are needed — whoever can restart the server already holds the stronger credential. + +--- + +## Troubleshooting + +**"Invalid authentication code" for every code.** Almost always clock drift on the server: TOTP is a +function of the current time, and the accepted window is ±30 seconds. Check the host clock +(`timedatectl status`, or the node's NTP state) rather than re-enrolling. + +**"Invalid authentication code" for a code that just worked.** Each code is single-use. Wait for the +next one rather than resubmitting the same digits. + +**A 503 naming `ADMIN_TOTP_SECRET` or `USER_TOTP_SECRET`.** Two causes, and the message says which. +"is not a valid base32 secret" is a typo: `0`, `1`, `8` and `9` are not in the alphabet. "is too +short" means the value decodes to fewer than the 128 bits RFC 4226 requires; generate a fresh one +with the command in step 1 rather than padding the old one out. + +**The code field never appears.** The account has no secret configured, or the deployment runs +`NEXT_PUBLIC_AUTH_PROVIDER=oidc`, where the login page renders no password form for the field to +follow. The variables are still honoured in that mode: `POST /api/auth/login` stays guarded, there +is simply no page step to show. See [Running both](#running-both). + +**Nothing happens after a correct code.** Check for a `429`: the client bucket may have tripped from +earlier wrong codes. It clears on its own within the window. + +--- + +## What this does and does not protect + +It defeats a password that leaked on its own — reused from another breach, read out of a +`docker inspect`, or shoulder-surfed. That is the threat this control exists for, and it is the +common one. + +It is not a substitute for the identity provider. There is no passkey or WebAuthn support here, no +per-user enrolment, no recovery codes, and no device management — one shared secret per account, +provisioned by whoever runs the server. Teams that need more should run +[OIDC](OIDC.md) and enforce it upstream. + +One deployment note: the spent-code set lives in the application process, like the login rate-limit +counters. Above one replica each process enforces its own view, so a captured code can be replayed +once per replica inside its 90-second window. The chart defaults to `replicaCount: 1`. + +Stated as a control, with its verifying test, in +[`docs/SECURITY.md`](SECURITY.md#controls) (row 1.6). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 538442637..210af069d 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -36,6 +36,7 @@ Two consequences worth stating before the table: | 1.3 | State-changing requests are checked against the deployment's own origin | Implemented | [`src/lib/api/origin-check.ts`](../src/lib/api/origin-check.ts), [`src/proxy.ts`](../src/proxy.ts) | [`tests/security/csrf-origin.test.ts`](../tests/security/csrf-origin.test.ts) | | 1.4 | Authentication transitions and denials are audited | Partial | [`src/lib/audit.ts`](../src/lib/audit.ts), [`src/lib/api/require-session.ts`](../src/lib/api/require-session.ts) | [`tests/security/auth-audit.test.ts`](../tests/security/auth-audit.test.ts) | | 1.5 | The login comparison is constant time and its failure response is uniform | Implemented | [`src/lib/auth-compare.ts`](../src/lib/auth-compare.ts), [`src/app/api/auth/login/route.ts`](../src/app/api/auth/login/route.ts) | [`tests/security/login-enumeration.test.ts`](../tests/security/login-enumeration.test.ts) | +| 1.6 | A local account with a TOTP secret configured cannot be signed in with its password alone, and an accepted code cannot be used twice | Implemented | [`src/lib/totp.ts`](../src/lib/totp.ts), [`src/lib/local-auth.ts`](../src/lib/local-auth.ts), [`src/app/api/auth/login/route.ts`](../src/app/api/auth/login/route.ts) | [`tests/security/mfa-second-factor.test.ts`](../tests/security/mfa-second-factor.test.ts) | | 2.1 | Secrets, dependencies and the container image are scanned in CI | Implemented | [`.github/workflows/security-scan.yml`](../.github/workflows/security-scan.yml), [`.gitleaks.toml`](../.gitleaks.toml), [`.trivyignore.yaml`](../.trivyignore.yaml) | [`tests/unit/security-scan-workflow.test.ts`](../tests/unit/security-scan-workflow.test.ts), [`tests/unit/gitleaks-config.test.ts`](../tests/unit/gitleaks-config.test.ts), [`tests/unit/trivyignore-policy.test.ts`](../tests/unit/trivyignore-policy.test.ts) | | 2.2 | An SBOM is published with every release | Implemented | [`.github/workflows/release-artifacts.yml`](../.github/workflows/release-artifacts.yml) | [`tests/unit/release-sbom.test.ts`](../tests/unit/release-sbom.test.ts) | | 2.3 | No TypeScript error is suppressed at build time | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/unit/next-config-typecheck.test.ts`](../tests/unit/next-config-typecheck.test.ts) | @@ -142,6 +143,26 @@ per replica; multi-replica deployments should enforce the same budgets at the in in-handler admin checks and the middleware's `/admin` redirect return their denial with no audit line. Tracked in [`docs/BACKLOG.md`](./BACKLOG.md), entry H12. +**1.6.** Opt-in: a second factor exists for an account exactly when `ADMIN_TOTP_SECRET` / +`USER_TOTP_SECRET` is set, so the row claims nothing about a deployment that sets neither. +Verification is RFC 6238 with HMAC-SHA-1, six digits, a 30-second step and one step of skew either +side. Three things are worth stating precisely. + +It guards `POST /api/auth/login`, and that route is **not** disabled by +`NEXT_PUBLIC_AUTH_PROVIDER=oidc` — the provider setting changes what the login page renders, not +what the API accepts. A deployment that runs OIDC while still setting `ADMIN_PASSWORD` therefore +keeps a way in that never reaches the issuer, and so never meets the MFA policy configured there; +these variables are honoured in that mode too, which is how it is closed +([`docs/MFA.md`](./MFA.md), [`docs/OIDC.md`](./OIDC.md)). + +Replies distinguish "code required" from "invalid code", which is not the enumeration oracle 1.5 +removes: both are reachable only with a correct password, and without MFA that same request would +have returned a session. + +The spent-code set that makes an accepted code single-use lives in the application process, like +the counters in 1.2 — with more than one replica each process enforces its own view, so a captured +code can be replayed once per replica inside its 90-second window. + **3.1.** Applies to `STORAGE_PROVIDER=sqlite` and `postgres` only. Seven fields are encrypted; `host`, `port`, `user`, `agentUser`, `database`, `name` and the TLS certificates stay readable so a dump can still be identified. Rotating the key makes stored credentials unreadable — the connection survives, the diff --git a/docs/ui/login-page.md b/docs/ui/login-page.md index bce00f9d8..fff7ad39d 100644 --- a/docs/ui/login-page.md +++ b/docs/ui/login-page.md @@ -122,6 +122,11 @@ When local auth is active, the right panel shows: 1. **Email/password form** with icon-prefixed inputs 2. **"Sign In" button** — calls `POST /api/auth/login` with JSON body +3. **Authentication code field**, rendered only after the server answers `mfaRequired` for those + credentials, with the button relabelled "Verify code". The form never predicts this: whether an + account carries a second factor is server-side configuration, and deciding it client-side would + publish which accounts are protected. Editing either credential drops back to step 1, so a code + minted for one account is never submitted against another. See [MFA.md](../MFA.md). On successful login, the user is redirected based on their role: - `admin` → `/admin` @@ -130,6 +135,11 @@ On successful login, the user is redirected based on their role: On failure, the form surfaces the API's `message` via a toast instead of a generic error: - Wrong credentials → `"Invalid email or password"` (401). +- TOTP account, no code yet → `"Enter the 6-digit code from your authenticator app"` (401, + `mfaRequired: true`). Deliberately no toast: the field appearing is the message, and an error + toast would frame a normal step of the flow as a failure. +- TOTP account, wrong or replayed code → `"Invalid authentication code"` (401, `mfaRequired: true`), + toasted, and the field is cleared. - Server not configured (missing `ADMIN_PASSWORD`, or a missing/too-short `JWT_SECRET`) → the actionable `AuthConfigError` message (503), e.g. *"Login is unavailable: this server has no administrator password configured. Set @@ -175,7 +185,7 @@ The login page follows the app's premium dark aesthetic: | `src/lib/db-showcase.ts` | Showcase order and the derived engine list | | `src/lib/distribution/channels.generated.ts` | Generated live-channel list (`bun run channels:showcase`) | | `src/lib/agent/engine-support.ts` | The engines the agent card may claim execution on | -| `tests/components/LoginPage.test.tsx` | Component tests — rendering, form submission, OIDC mode, and the pinned agent claim | +| `tests/components/LoginPage.test.tsx` | Component tests — rendering, form submission, OIDC mode, the TOTP step, and the pinned agent claim | | `e2e/login.spec.ts` | Browser assertions that both showcase blocks render every derived entry, desktop and mobile | --- diff --git a/operator/helm-charts/libredb-studio/Chart.yaml b/operator/helm-charts/libredb-studio/Chart.yaml index c9121f2d9..0deb48228 100644 --- a/operator/helm-charts/libredb-studio/Chart.yaml +++ b/operator/helm-charts/libredb-studio/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: libredb-studio description: Web-based SQL IDE for cloud-native teams supporting sixteen engines - PostgreSQL, MySQL, SQLite, DuckDB, Oracle, SQL Server, MongoDB, Redis, Couchbase, ClickHouse, Apache Druid, Elasticsearch, OpenSearch, Apache Trino, Apache Cassandra and libSQL type: application -version: 0.1.62 +version: 0.1.63 appVersion: "0.15.0" kubeVersion: ">=1.26.0-0" home: https://github.com/libredb/libredb-studio diff --git a/operator/helm-charts/libredb-studio/README.md b/operator/helm-charts/libredb-studio/README.md index ae91bc6f9..a53b8b6e6 100644 --- a/operator/helm-charts/libredb-studio/README.md +++ b/operator/helm-charts/libredb-studio/README.md @@ -40,7 +40,7 @@ helm install libredb libredb/libredb-studio \ ```bash helm install libredb oci://ghcr.io/libredb/charts/libredb-studio \ - --version 0.1.62 \ + --version 0.1.63 \ --set secrets.jwtSecret=$(openssl rand -base64 32) \ --set secrets.adminPassword=MyAdmin123 ``` @@ -147,6 +147,36 @@ browser still speaks `https`, so it accepts the cookie. `false` is only for the the browser's own connection is cleartext - and it means session cookies travel in cleartext, so keep it to a trusted network. `true` forces the flag on. +## Two-Factor Authentication (TOTP) + +Optional, local-provider only, and opt-in per account. Set a base32 secret and that account must +present a 6-digit authenticator code after its password: + +```bash +# Generate it, enrol the printed value in your authenticator app, then install. +ADMIN_TOTP_SECRET="$(openssl rand 20 | base32 | tr -d '=')" +echo "$ADMIN_TOTP_SECRET" + +helm upgrade --install libredb libredb/libredb-studio \ + --set secrets.adminPassword=MyAdmin123 \ + --set secrets.adminTotpSecret="$ADMIN_TOTP_SECRET" +``` + +No example secret is printed here on purpose. One that looks real invites being copied and left in +place, and a second factor whose secret is published is worse than none. + +The value travels in the chart's Secret and is referenced from the pod, so it never appears in the +Deployment spec - which is why `extraEnv` is the wrong tool for it. Both `ADMIN_TOTP_SECRET` and +`USER_TOTP_SECRET` refs are always optional, including in strict mode, so a second factor nobody +asked for can never keep the pod from starting. `values.schema.json` applies the same test the app +does, base32 and at least the 128 bits RFC 4226 requires, so a bad secret fails at install time +rather than at the login screen. + +Under `authProvider=oidc` the login page shows no password form and MFA belongs to the identity +provider - but `POST /api/auth/login` stays reachable whenever `secrets.adminPassword` is also set, +and this guards that route in every mode. Full setup, enrolment and recovery: +[`docs/MFA.md`](../../docs/MFA.md). + ## OIDC SSO ```bash @@ -542,7 +572,7 @@ helm install libredb libredb/libredb-studio \ Your external secret is referenced with these keys (customizable via `secrets.existingSecretKeys`): - `jwt-secret`, `admin-password` — required in strict mode (the pod waits for them); in zero-config mode missing ones are generated at first start -- Optional: `admin-email`, `user-email`, `user-password` (the non-admin account exists only when `user-password` is set), `llm-api-key`, `oidc-client-id`, `oidc-client-secret`, `storage-postgres-url` +- Optional: `admin-email`, `user-email`, `user-password` (the non-admin account exists only when `user-password` is set), `admin-totp-secret`, `user-totp-secret`, `llm-api-key`, `oidc-client-id`, `oidc-client-secret`, `storage-postgres-url` ## Upgrading @@ -586,6 +616,8 @@ helm uninstall libredb | `secrets.adminPassword` | Admin password | `""` | | `secrets.userEmail` | User email | `user@libredb.org` | | `secrets.userPassword` | User password (optional; enables the non-admin account) | `""` | +| `secrets.adminTotpSecret` | Base32 TOTP secret for the admin account (optional second factor) | `""` | +| `secrets.userTotpSecret` | Base32 TOTP secret for the user account (optional second factor) | `""` | | `secrets.existingSecret` | Use existing Secret | `""` | | `config.bindAddress` | Container bind address (`HOSTNAME`): empty lets the image resolve one, preferring a verified dual-stack `::`; `::` forces it; `0.0.0.0` pins IPv4 | `""` | | `config.storageProvider` | Storage: local, sqlite, postgres | `local` | diff --git a/operator/helm-charts/libredb-studio/templates/deployment.yaml b/operator/helm-charts/libredb-studio/templates/deployment.yaml index 506227cdd..40ef3391c 100644 --- a/operator/helm-charts/libredb-studio/templates/deployment.yaml +++ b/operator/helm-charts/libredb-studio/templates/deployment.yaml @@ -181,6 +181,26 @@ spec: key: {{ .Values.secrets.existingSecretKeys.userPassword }} optional: true {{- end }} + {{- /* TOTP second factor. Always optional: MFA is opt-in per account, so a missing + key must never keep the pod from starting, and the app treats an absent value + as "no second factor". Keyed separately from the passwords rather than nested + under them so an operator can rotate a secret without touching a credential. */}} + {{- if or .Values.secrets.adminTotpSecret .Values.secrets.existingSecret }} + - name: ADMIN_TOTP_SECRET + valueFrom: + secretKeyRef: + name: {{ include "libredb-studio.secretName" . }} + key: {{ .Values.secrets.existingSecretKeys.adminTotpSecret }} + optional: true + {{- end }} + {{- if or .Values.secrets.userTotpSecret .Values.secrets.existingSecret }} + - name: USER_TOTP_SECRET + valueFrom: + secretKeyRef: + name: {{ include "libredb-studio.secretName" . }} + key: {{ .Values.secrets.existingSecretKeys.userTotpSecret }} + optional: true + {{- end }} {{- if or .Values.secrets.llmApiKey .Values.secrets.existingSecret }} - name: LLM_API_KEY valueFrom: diff --git a/operator/helm-charts/libredb-studio/templates/secret.yaml b/operator/helm-charts/libredb-studio/templates/secret.yaml index feaed82e8..d0b4dee51 100644 --- a/operator/helm-charts/libredb-studio/templates/secret.yaml +++ b/operator/helm-charts/libredb-studio/templates/secret.yaml @@ -26,6 +26,12 @@ data: {{ .Values.secrets.existingSecretKeys.userEmail }}: {{ .Values.secrets.userEmail | b64enc | quote }} {{ .Values.secrets.existingSecretKeys.userPassword }}: {{ .Values.secrets.userPassword | b64enc | quote }} {{- end }} + {{- if .Values.secrets.adminTotpSecret }} + {{ .Values.secrets.existingSecretKeys.adminTotpSecret }}: {{ .Values.secrets.adminTotpSecret | b64enc | quote }} + {{- end }} + {{- if .Values.secrets.userTotpSecret }} + {{ .Values.secrets.existingSecretKeys.userTotpSecret }}: {{ .Values.secrets.userTotpSecret | b64enc | quote }} + {{- end }} {{- if .Values.secrets.llmApiKey }} {{ .Values.secrets.existingSecretKeys.llmApiKey }}: {{ .Values.secrets.llmApiKey | b64enc | quote }} {{- end }} diff --git a/operator/helm-charts/libredb-studio/values.schema.json b/operator/helm-charts/libredb-studio/values.schema.json index 3c94e465c..534f4bb1a 100644 --- a/operator/helm-charts/libredb-studio/values.schema.json +++ b/operator/helm-charts/libredb-studio/values.schema.json @@ -103,6 +103,16 @@ "type": "string", "description": "User account password" }, + "adminTotpSecret": { + "type": "string", + "pattern": "^$|^[-\\s]*(?:[A-Za-z2-7][-\\s]*){26,}=*$", + "description": "Admin TOTP secret: empty (no second factor) or base32 (RFC 4648: A-Z and 2-7; spaces, hyphens and trailing = are stripped), at least 26 characters so the decoded secret carries the 128 bits RFC 4226 requires" + }, + "userTotpSecret": { + "type": "string", + "pattern": "^$|^[-\\s]*(?:[A-Za-z2-7][-\\s]*){26,}=*$", + "description": "User TOTP secret: empty (no second factor) or base32 (RFC 4648: A-Z and 2-7; spaces, hyphens and trailing = are stripped), at least 26 characters so the decoded secret carries the 128 bits RFC 4226 requires" + }, "llmApiKey": { "type": "string", "description": "LLM API key" diff --git a/operator/helm-charts/libredb-studio/values.yaml b/operator/helm-charts/libredb-studio/values.yaml index 472ecf7d4..16d74a79f 100644 --- a/operator/helm-charts/libredb-studio/values.yaml +++ b/operator/helm-charts/libredb-studio/values.yaml @@ -56,6 +56,8 @@ secrets: adminPassword: admin-password userEmail: user-email userPassword: user-password + adminTotpSecret: admin-totp-secret + userTotpSecret: user-totp-secret llmApiKey: llm-api-key oidcClientId: oidc-client-id oidcClientSecret: oidc-client-secret @@ -75,6 +77,17 @@ secrets: # -- Regular user account password. Optional: the non-admin account exists only when set; # it is never generated. userPassword: "" + # -- Base32 TOTP secret for the admin account (RFC 4648: A-Z and 2-7), at least 26 characters + # so the decoded secret carries the 128 bits RFC 4226 requires. Optional and + # local-provider only: when set, admin login requires a 6-digit authenticator code after the + # password. A value that is not base32 stops login with a clear 503 rather than silently + # dropping the second factor. Under config.authProvider=oidc the login page shows no password + # form and MFA belongs to the identity provider, but this still guards POST /api/auth/login + # whenever a password is also set. Generate one with: openssl rand 20 | base32 | tr -d '=' + adminTotpSecret: "" + # -- Base32 TOTP secret for the regular user account. Same rules as adminTotpSecret; inert + # unless userPassword is also set, since without it there is no user account to protect. + userTotpSecret: "" # -- LLM API key (optional, for AI features) llmApiKey: "" # -- OIDC client ID (required when authProvider=oidc) diff --git a/packaging/linux/env b/packaging/linux/env index 16b5ed68b..a6dd396a3 100644 --- a/packaging/linux/env +++ b/packaging/linux/env @@ -20,6 +20,13 @@ #USER_EMAIL=user@libredb.org #USER_PASSWORD= # +# Two-factor authentication (TOTP), optional and per account. Base32, at least +# 26 characters; generate one with: openssl rand 20 | base32 +# A value that is not base32, or too short, stops login with a 503 naming the +# variable. Enrolment and recovery: docs/MFA.md +#ADMIN_TOTP_SECRET= +#USER_TOTP_SECRET= +# # AI query assistance (optional). #LLM_PROVIDER=gemini #LLM_API_KEY= diff --git a/scripts/security-check.mjs b/scripts/security-check.mjs index 5c4ffae7a..703ffbd2a 100644 --- a/scripts/security-check.mjs +++ b/scripts/security-check.mjs @@ -53,6 +53,7 @@ export const PROGRAMME_CONTROL_IDS = [ "1.3", "1.4", "1.5", + "1.6", "2.1", "2.2", "2.3", diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 32e2a4473..30228fb00 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -12,8 +12,9 @@ import { type RateLimitBucket, } from "@/lib/api/rate-limit"; import { hmacHex, secretsMatch } from "@/lib/auth-compare"; -import { emitAuditEvent, MAX_AUDIT_FIELD_LENGTH } from "@/lib/audit"; +import { emitAuditEvent, MAX_AUDIT_FIELD_LENGTH, type AuditReason } from "@/lib/audit"; import { logger } from "@/lib/logger"; +import { claimTotpStep, verifyTotp } from "@/lib/totp"; const ROUTE = "POST /api/auth/login"; @@ -25,6 +26,11 @@ const ROUTE = "POST /api/auth/login"; */ const DUMMY_PASSWORD = "libredb-dummy-password-never-a-credential"; +// Single-line and module-scoped, matching the auth messages in local-auth.ts and auth-env.ts: +// bun's line coverage under-counts the continuation lines of a wrapped string. +const MFA_REQUIRED_MESSAGE = "Enter the 6-digit code from your authenticator app"; +const MFA_INVALID_MESSAGE = "Invalid authentication code"; + type LoginBucket = Extract; /** @@ -71,8 +77,9 @@ export async function POST(request: NextRequest) { let email: unknown; let password: unknown; + let totp: unknown; try { - ({ email, password } = await request.json()); + ({ email, password, totp } = await request.json()); } catch { // A malformed body is a client error, not a server error, and - like a wrong password - is // a wasted attempt from this address: consume the client bucket so a caller who floods this @@ -104,6 +111,9 @@ export async function POST(request: NextRequest) { // distinguishable response. const submittedEmail = typeof email === "string" ? email : ""; const submittedPassword = typeof password === "string" ? password : ""; + // Same coercion, same reason: a non-string `totp` must read as "no code supplied", never + // reach verifyTotp's string methods and become a 500 that stands out from the uniform 401. + const submittedTotp = typeof totp === "string" ? totp.trim() : ""; // MAX_AUDIT_FIELD_LENGTH, not a locally redeclared copy of the same number: the actor becomes // an AuditEvent field either way, so both truncations must move together. const actor = submittedEmail.slice(0, MAX_AUDIT_FIELD_LENGTH) || "anonymous"; @@ -121,6 +131,63 @@ export async function POST(request: NextRequest) { const passwordMatches = secretsMatch(submittedPassword, candidate); const matched = user && passwordMatches ? user : null; + // Second factor. Reached only once the password already matched, so answering "code required" + // here is not the account-enumeration oracle the uniform 401 below exists to prevent: an + // unknown email and a known email with a wrong password both still take that path, and a + // caller who can reach this branch necessarily holds a working password for the account - + // without MFA configured, that same request would simply have logged them in. + if (matched?.totpSecret) { + // No "did they send one?" pre-check: verifyTotp's own digit test already answers null for + // an empty or malformed code, so a guard here would be a second copy of that decision - and + // a user-controlled value guarding the session mint is what CodeQL flags as + // js/user-controlled-bypass. `submittedTotp` still picks the audit reason below, which is a + // logging branch and decides nothing. + const step = verifyTotp(matched.totpSecret, submittedTotp); + // claimTotpStep is what makes an accepted code single-use (RFC 6238 §5.2). A replayed code + // verifies but fails to claim, and lands here as an ordinary bad code. + const accepted = step !== null && claimTotpStep(accountKey, step); + + if (!accepted) { + const reason: AuditReason = submittedTotp ? "bad_totp" : "mfa_required"; + // A wrong code is a failed attempt and is charged for: it is the only thing between an + // attacker holding a stolen password and a session, so guessing has to be bounded. Not + // having supplied one yet is NOT a failed attempt - it is the first half of a + // two-request flow, and charging it would spend the whole five-failure client budget on + // five ordinary logins. + if (reason === "bad_totp") { + consumeRateLimit("login_client", clientKey); + consumeRateLimit("login_account", accountKey); + } + // Isolated like every other emit in this route: the 401 below is already decided, and a + // broken audit sink must not turn it into an unrelated 500. The mfa_required branch is + // uncharged and therefore unbounded in volume for whoever holds the password - the same + // property login_success already has, since a successful login resets both buckets. + try { + emitAuditEvent({ + type: "login_failure", + action: "login", + target: ROUTE, + user: matched.email, + result: "failure", + reason, + ip, + }); + } catch (auditError) { + logger.error("Failed to record login_failure audit event", auditError, { route: ROUTE }); + } + // `mfaRequired` tells the client to render the code field. It carries no information the + // caller did not already supply the password to learn (see the note above this block). + return NextResponse.json( + { + success: false, + mfaRequired: true, + message: reason === "bad_totp" ? MFA_INVALID_MESSAGE : MFA_REQUIRED_MESSAGE, + }, + { status: 401 }, + ); + } + } + if (matched) { await login(matched.role, matched.email); resetRateLimit("login_client", clientKey); diff --git a/src/app/login/login-form.tsx b/src/app/login/login-form.tsx index 62ec254c6..9e1242439 100644 --- a/src/app/login/login-form.tsx +++ b/src/app/login/login-form.tsx @@ -7,7 +7,7 @@ import { Card, CardContent, CardDescription, CardFooter, CardHeader, CardTitle } import { Input } from "@/components/ui/input"; import { Button } from "@/components/ui/button"; import { Label } from "@/components/ui/label"; -import { ExternalLink, Lock, Mail, ShieldCheck, Shield } from "lucide-react"; +import { ExternalLink, KeyRound, Lock, Mail, ShieldCheck, Shield } from "lucide-react"; import { toast } from "sonner"; import LibreDBLogo from "@/components/libredb-logo"; import { CommunitySection } from "@/components/community-section"; @@ -26,11 +26,28 @@ function LoginFormInner({ authProvider }: { authProvider: string }) { const isOIDC = authProvider === "oidc"; const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); + const [totp, setTotp] = useState(""); + /** + * Set once the server has answered `mfaRequired` for these credentials. The form never guesses + * at it: whether an account carries a second factor is server-side configuration, and asking + * the client to know it up front would mean publishing which accounts are protected. + */ + const [mfaRequired, setMfaRequired] = useState(false); const [isLoading, setIsLoading] = useState(false); const router = useRouter(); const searchParams = useSearchParams(); const oidcError = searchParams.get("error"); + /** + * Editing either credential drops the second-factor step. Without this, changing the email + * after being prompted would submit the new account with a code minted for the previous one - + * a guaranteed failure that also spends a slot in the per-account rate-limit bucket. + */ + const resetMfa = () => { + setMfaRequired(false); + setTotp(""); + }; + const handleLogin = async (e?: React.FormEvent) => { if (e) e.preventDefault(); @@ -39,12 +56,19 @@ function LoginFormInner({ authProvider }: { authProvider: string }) { return; } + if (mfaRequired && !totp) { + toast.error("Please enter your authentication code"); + return; + } + setIsLoading(true); try { const response = await appFetch("/api/auth/login", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ email, password }), + // `totp` is omitted entirely until the server asks for it, so a deployment without MFA + // sees exactly the request body it saw before. + body: JSON.stringify(mfaRequired ? { email, password, totp } : { email, password }), }); const data = await response.json(); @@ -53,6 +77,13 @@ function LoginFormInner({ authProvider }: { authProvider: string }) { toast.success(`Welcome back, ${data.role}!`); router.push(data.role === "admin" ? "/admin" : "/"); router.refresh(); + } else if (data.mfaRequired) { + // The first prompt needs no toast - the code field appearing IS the message, and an error + // toast would frame a normal step of the flow as a failure. Being asked a second time + // does mean the code was rejected, and that is worth saying out loud. + if (mfaRequired) toast.error(data.message); + setMfaRequired(true); + setTotp(""); } else { // data.message is the login route's own body ({ success: false, message }); data.error is // everything else that can refuse a login POST before or without reaching that body - the @@ -274,7 +305,10 @@ function LoginFormInner({ authProvider }: { authProvider: string }) { placeholder="Enter your email" className="pl-10 h-11 transition-all focus:ring-2 focus:ring-primary/20" value={email} - onChange={(e) => setEmail(e.target.value)} + onChange={(e) => { + setEmail(e.target.value); + resetMfa(); + }} required /> @@ -289,17 +323,57 @@ function LoginFormInner({ authProvider }: { authProvider: string }) { placeholder="Enter your password" className="pl-10 h-11 transition-all focus:ring-2 focus:ring-primary/20" value={password} - onChange={(e) => setPassword(e.target.value)} + onChange={(e) => { + setPassword(e.target.value); + resetMfa(); + }} required /> + {mfaRequired && ( +
+ +
+ + setTotp(e.target.value)} + required + /> +
+

+ Open your authenticator app and enter the current 6-digit code for this account. +

+
+ )} diff --git a/src/lib/audit.ts b/src/lib/audit.ts index 03bfff3e1..08e19705c 100644 --- a/src/lib/audit.ts +++ b/src/lib/audit.ts @@ -30,6 +30,15 @@ export type AuditEventType = */ export type AuditReason = | "bad_credentials" + /** + * A correct password on a TOTP-protected local account, with no code presented yet. Recorded + * even though it is a normal step of the two-request flow, because in the abnormal case it is + * the highest-value line in this log: it says someone holds a working password for that + * account and was stopped only by the second factor. + */ + | "mfa_required" + /** A correct password, but the second factor did not verify — a wrong, expired or replayed code. */ + | "bad_totp" | "malformed_body" | "no_session" | "insufficient_role" diff --git a/src/lib/local-auth.ts b/src/lib/local-auth.ts index 07da3b0f7..d4d64e036 100644 --- a/src/lib/local-auth.ts +++ b/src/lib/local-auth.ts @@ -5,11 +5,18 @@ */ import type { Role } from "@/lib/auth"; import { AuthConfigError } from "@/lib/auth-errors"; +import { decodeBase32, TOTP_MIN_SECRET_BYTES } from "@/lib/totp"; export interface AuthUser { email: string; password: string; role: Role; + /** + * Base32 TOTP secret for this account, when the operator configured one. Absent means the + * account authenticates with its password alone — MFA is opt-in per account, so an admin can + * be protected while an automation-owned lower-privilege account is not. + */ + totpSecret?: string; } // Single-line and module-scoped so bun's line coverage credits it cleanly (it @@ -17,6 +24,39 @@ export interface AuthUser { const ADMIN_PASSWORD_MISSING_MESSAGE = "Login is unavailable: this server has no administrator password configured. Set the ADMIN_PASSWORD environment variable and restart the server."; +// Second half of the invalid-secret message; the variable name is prefixed at the throw site. +const TOTP_SECRET_INVALID_HINT = + "is not a valid base32 secret. Copy the secret exactly as your authenticator app shows it (letters A-Z and digits 2-7 only) and restart the server."; + +// Same shape, for a secret that decodes cleanly but carries too little entropy to be a factor. +const TOTP_SECRET_SHORT_HINT = `is too short: RFC 4226 requires a shared secret of at least ${TOTP_MIN_SECRET_BYTES * 8} bits. Generate one with "openssl rand 20 | base32" and restart the server.`; + +/** + * Read one account's TOTP secret, rejecting a secret that could never protect anything. + * + * Two ways a configured value is wrong, and both have to be fatal rather than ignored. A + * malformed secret would refuse every correct code the operator's phone produces; a secret below + * the RFC's minimum would accept them all while being worth almost nothing. Neither is + * diagnosable from an "Invalid email or password" screen, and silently dropping the factor in + * either case would leave a deployment that believes it has MFA and does not. So both become an + * AuthConfigError and the login route renders the message as a 503 naming the variable. + * + * @throws {AuthConfigError} when the variable is not base32, or decodes below the RFC minimum. + */ +function readTotpSecret(variable: string): string | undefined { + const raw = process.env[variable]?.trim(); + // Unset and empty are the same answer — no second factor — so an operator can disable MFA by + // blanking the variable rather than having to unset it, which some orchestrators cannot do. + if (!raw) return undefined; + const decoded = decodeBase32(raw); + if (!decoded) throw new AuthConfigError(`Login is unavailable: ${variable} ${TOTP_SECRET_INVALID_HINT}`); + // Measured on the decoded bytes, never on the pasted string: spaces, hyphens and `=` padding are + // presentation that decodeBase32 strips, so a grouped short secret must not read as long enough. + if (decoded.length < TOTP_MIN_SECRET_BYTES) + throw new AuthConfigError(`Login is unavailable: ${variable} ${TOTP_SECRET_SHORT_HINT}`); + return raw; +} + /** * Build the list of accounts that can authenticate against the local provider. * ADMIN_PASSWORD is required; the lower-privilege user account is optional and @@ -33,12 +73,21 @@ export function getAuthUsers(): AuthUser[] { throw new AuthConfigError(ADMIN_PASSWORD_MISSING_MESSAGE); } - const users: AuthUser[] = [{ email: adminEmail, password: adminPassword, role: "admin" }]; + const users: AuthUser[] = [ + { email: adminEmail, password: adminPassword, role: "admin", totpSecret: readTotpSecret("ADMIN_TOTP_SECRET") }, + ]; + // USER_TOTP_SECRET is read only when the account it protects exists. Set without USER_PASSWORD + // it is inert rather than a hole: with no password there is no user account to log into at all. const userPassword = process.env.USER_PASSWORD; if (userPassword) { const userEmail = process.env.USER_EMAIL || "user@libredb.org"; - users.push({ email: userEmail, password: userPassword, role: "user" }); + users.push({ + email: userEmail, + password: userPassword, + role: "user", + totpSecret: readTotpSecret("USER_TOTP_SECRET"), + }); } return users; diff --git a/src/lib/totp.ts b/src/lib/totp.ts new file mode 100644 index 000000000..a689174b6 --- /dev/null +++ b/src/lib/totp.ts @@ -0,0 +1,195 @@ +/** + * RFC 6238 TOTP verification for the local auth provider. + * + * Scope: verification only. Secrets are provisioned by the operator through + * `ADMIN_TOTP_SECRET` / `USER_TOTP_SECRET` (src/lib/local-auth.ts), matching how every other + * local-provider credential is configured — so there is no enrolment flow to store, no QR code + * to mint and no new writable state on disk. That last point is the deciding one: the chart and + * the Docker image both run happily on a read-only filesystem, and an MFA control that silently + * degraded when the data dir was not writable would be worse than no MFA at all. + * + * SHA-1 is not a mistake here and must not be "upgraded": RFC 6238 §1.2 names HMAC-SHA-1 as the + * default, and it is the only algorithm Google Authenticator, Authy, 1Password and the rest + * interoperate on for a bare `otpauth://totp` URI. The construction's security rests on HMAC, + * which does not depend on the collision resistance SHA-1 lost. + * + * No dependency: HOTP is a truncated HMAC and base32 is a 32-character alphabet. Pulling in an + * OTP library to avoid this much arithmetic would add a supply-chain surface to the one code + * path in this app that exists specifically to raise the cost of a compromise. + */ +import { createHmac, timingSafeEqual } from "node:crypto"; + +/** Digits in an accepted code. Six is what authenticator apps emit for a bare otpauth URI. */ +const TOTP_DIGITS = 6; + +/** Seconds each code is valid for. RFC 6238's default, and the only value apps assume. */ +export const TOTP_PERIOD_SECONDS = 30; + +/** + * Steps of clock skew accepted either side of the current one, so one back and one forward. + * RFC 6238 §5.2 permits "at most one time step" for exactly the two cases this covers: a user who + * starts typing at second 29, and a server whose clock trails the phone's. Widening it multiplies + * the codes valid at any instant, which is why it is a constant and not an env var. + */ +const TOTP_WINDOW_STEPS = 1; + +const BASE32_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"; + +/** + * Smallest shared secret that may be configured, in decoded bytes. RFC 4226 R6 makes 128 bits a + * MUST and RFC 6238 inherits it. + * + * `decodeBase32` only rejects a secret with no whole byte in it, which is a far lower bar than the + * RFC's: `AA` decodes to one byte and, without this, is a working second factor worth 8 bits — one + * observed code narrows it to a single candidate. That failure is invisible, because every screen + * and every doc still says the account has MFA. Enforced by the caller that reads the operator's + * value (src/lib/local-auth.ts) rather than by verifyTotp, so it surfaces once as a configuration + * error naming the variable instead of as a rejected code on every login. + */ +export const TOTP_MIN_SECRET_BYTES = 16; + +/** Matches a submitted code once its whitespace has been stripped. */ +const CODE_PATTERN = new RegExp(`^\\d{${TOTP_DIGITS}}$`); + +/** + * Decode an RFC 4648 base32 secret, or `null` if it is not one. + * + * Lenient about presentation and strict about content: authenticator apps and password managers + * print secrets in lowercase, in space- or hyphen-separated groups, and with or without `=` + * padding, so all of that is normalized away. Anything left outside the alphabet is a typo in the + * operator's environment, and the caller turns that into a config error the operator can read + * rather than a login that silently rejects every correct code. + */ +export function decodeBase32(secret: string): Buffer | null { + const normalized = secret.replace(/[\s-]/g, "").replace(/=+$/, "").toUpperCase(); + if (!normalized) return null; + + const bytes: number[] = []; + let accumulator = 0; + let bits = 0; + for (const character of normalized) { + const value = BASE32_ALPHABET.indexOf(character); + if (value === -1) return null; + accumulator = (accumulator << 5) | value; + bits += 5; + if (bits >= 8) { + bits -= 8; + bytes.push((accumulator >> bits) & 0xff); + } + } + // Fewer than eight bits of payload is not a short secret, it is no secret at all: a single + // base32 character carries no whole byte, so the HMAC key would be empty. + if (bytes.length === 0) return null; + return Buffer.from(bytes); +} + +/** RFC 4226 HOTP: dynamic truncation of HMAC-SHA-1 over the big-endian counter. */ +function hotp(key: Buffer, counter: number): string { + const counterBytes = Buffer.alloc(8); + counterBytes.writeBigUInt64BE(BigInt(counter)); + const digest = createHmac("sha1", key).update(counterBytes).digest(); + // The low nibble of the last byte picks the 4-byte window; the high bit is masked off so the + // value reads as a positive 31-bit integer on every platform (RFC 4226 §5.4). + const offset = digest[digest.length - 1] & 0x0f; + const truncated = digest.readUInt32BE(offset) & 0x7fffffff; + return (truncated % 10 ** TOTP_DIGITS).toString().padStart(TOTP_DIGITS, "0"); +} + +/** + * Constant-time code comparison. + * + * Both operands are exactly TOTP_DIGITS ASCII digits — `hotp` pads and the caller's regex rejects + * anything else — so timingSafeEqual's equal-length precondition holds without the length guard + * that would itself be an oracle (see the note in src/lib/auth-compare.ts). + * + * Deliberately NOT routed through `secretsMatch`: that function's counter is what the + * login-enumeration test asserts its "exactly one comparison per attempt" property on, and a + * second factor checked afterwards would make that count depend on whether the password matched. + */ +function codesMatch(expected: string, submitted: string): boolean { + return timingSafeEqual(Buffer.from(expected, "ascii"), Buffer.from(submitted, "ascii")); +} + +/** + * Verify a submitted code against a base32 secret. + * + * Returns the time step the code belongs to rather than a boolean, because the caller needs that + * step to spend it through `claimTotpStep`. A boolean would leave the caller unable to tell two + * uses of one code apart, which is precisely what RFC 6238 §5.2 requires it to prevent. + */ +export function verifyTotp(secret: string, code: string, now: number = Date.now()): number | null { + const key = decodeBase32(secret); + if (!key) return null; + + // People paste "287 082" out of a password manager; the digit check applies to what is left. + const submitted = code.replace(/\s/g, ""); + if (!CODE_PATTERN.test(submitted)) return null; + + const currentStep = Math.floor(now / 1000 / TOTP_PERIOD_SECONDS); + for (let offset = -TOTP_WINDOW_STEPS; offset <= TOTP_WINDOW_STEPS; offset++) { + const step = currentStep + offset; + // The look-back offset runs off the bottom of the counter inside the first time step, and + // writeBigUInt64BE throws on a negative. Unreachable on a correct clock, but a container + // that starts before NTP has set the time reports epoch zero, and a login must not 500. + if (step < 0) continue; + if (codesMatch(hotp(key, step), submitted)) return step; + } + return null; +} + +/** + * Spent (account, step) pairs, so one code cannot be used twice inside its acceptance window — + * RFC 6238 §5.2. Without this, a code lifted by a shoulder-surf, a phishing proxy or a logged + * request body stays usable for up to 90 seconds, which is long enough to matter. + * + * In-process, like the login rate limiter in src/lib/api/rate-limit.ts, and for its reasons: the + * chart defaults to one replica, and a shared store would make an availability dependency out of + * a control that must never be the reason an operator cannot log in. Across replicas each process + * enforces its own view, which narrows the replay window rather than closing it — the same + * trade-off, and the same limit, the rate limiter already documents. + */ +const spentSteps = new Map(); + +/** + * A ceiling on the map, not a defence against one. + * + * Nothing an attacker sends can grow this: claimTotpStep is reached only after a code verifies + * against a configured account, and the local provider defines at most two of those, so pruning + * by expiry alone holds the map at a handful of entries. The cap exists for the shape this module + * would take if accounts ever became data rather than environment. Eviction fails OPEN (an + * evicted pair becomes replayable) because a replay guard must never be the reason a legitimate + * login is refused. + */ +const MAX_SPENT_ENTRIES = 4096; + +/** Test seam: drops all spent-step state so each case observes a fresh process. */ +export function clearTotpReplayState(): void { + spentSteps.clear(); +} + +/** + * Claim a time step for an account. Returns `false` when that exact code has already been + * accepted and is therefore a replay. + * + * `accountKey` must already be non-reversible — the login route passes the same `hmacHex` value + * it keys the per-account rate limiter on — so no email address reaches this long-lived state. + */ +export function claimTotpStep(accountKey: string, step: number, now: number = Date.now()): boolean { + // An entry is useless once the step it names can no longer be accepted by verifyTotp, so its + // expiry is the end of the last window that would still admit it. + const expiresAt = (step + TOTP_WINDOW_STEPS + 1) * TOTP_PERIOD_SECONDS * 1000; + for (const [entryKey, entryExpiry] of spentSteps) { + if (entryExpiry <= now) spentSteps.delete(entryKey); + } + // Pruning by expiry alone is usually enough; the cap only bites under a flood of distinct keys. + // Evicting the oldest insertion is sound here because Map preserves insertion order and every + // entry has the same lifetime, so the oldest inserted is also the nearest to expiring. + while (spentSteps.size >= MAX_SPENT_ENTRIES) { + spentSteps.delete(spentSteps.keys().next().value as string); + } + + const key = `${accountKey}:${step}`; + if (spentSteps.has(key)) return false; + spentSteps.set(key, expiresAt); + return true; +} diff --git a/tests/api/auth/login.test.ts b/tests/api/auth/login.test.ts index 7bf5540b0..37ae7f17e 100644 --- a/tests/api/auth/login.test.ts +++ b/tests/api/auth/login.test.ts @@ -2,6 +2,8 @@ import { describe, test, expect, mock, spyOn, beforeEach, afterEach } from "bun: import { createMockRequest, parseResponseJSON } from "../../helpers/mock-next"; import { AuthConfigError } from "@/lib/auth-errors"; import { clearRateLimitState } from "@/lib/api/rate-limit"; +import { clearTotpReplayState } from "@/lib/totp"; +import { RFC6238_SECRET } from "../../helpers/rfc6238"; // ─── Mock @/lib/auth BEFORE importing the route ───────────────────────────── // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -24,13 +26,17 @@ describe("POST /api/auth/login", () => { // afterEach — so a failing assertion mid-test can never leak env state into // later tests (a plain restore() at the end of a test body would be skipped // when an earlier expect() throws). - const MUTATED_ENV_KEYS = ["ADMIN_PASSWORD", "USER_PASSWORD"] as const; + const MUTATED_ENV_KEYS = ["ADMIN_PASSWORD", "USER_PASSWORD", "ADMIN_TOTP_SECRET", "USER_TOTP_SECRET"] as const; const envSnapshot: Record = {}; beforeEach(() => { clearRateLimitState(); mockLogin.mockClear(); for (const key of MUTATED_ENV_KEYS) envSnapshot[key] = process.env[key]; + // The TOTP variables are absent for every case that predates MFA, so the single-step flow is + // what those cases actually exercise regardless of the ambient environment. + delete process.env.ADMIN_TOTP_SECRET; + delete process.env.USER_TOTP_SECRET; }); afterEach(() => { @@ -370,4 +376,157 @@ describe("POST /api/auth/login", () => { expect(data.success).toBe(false); expect(data.message).toBe("Invalid email or password"); }); + + describe("TOTP second factor", () => { + /** RFC 6238 Appendix B seed; "287082" is its six-digit code for the step containing T=59. */ + const SECRET = RFC6238_SECRET; + const CODE = "287082"; + const FROZEN_NOW = 59_000; + + let nowSpy: ReturnType>; + + beforeEach(() => { + // Both the code check and the replay guard read the wall clock, so the whole flow is + // pinned to the instant the published vector is valid at. + nowSpy = spyOn(Date, "now").mockReturnValue(FROZEN_NOW); + clearTotpReplayState(); + process.env.ADMIN_TOTP_SECRET = SECRET; + }); + + afterEach(() => { + nowSpy.mockRestore(); + clearTotpReplayState(); + }); + + type MfaBody = { success: boolean; message: string; mfaRequired?: boolean; role?: string }; + + function attempt(body: Record, address = "203.0.113.40") { + return POST( + createMockRequest("/api/auth/login", { + method: "POST", + headers: { "x-forwarded-for": address }, + body, + }) as never, + ); + } + + test("asks for a code instead of signing in when the account carries a secret", async () => { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026" }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(401); + expect(data.mfaRequired).toBe(true); + expect(data.message).toBe("Enter the 6-digit code from your authenticator app"); + // The decisive assertion: no session was created on the strength of the password alone. + expect(mockLogin).not.toHaveBeenCalled(); + }); + + test("signs in once the correct code is presented", async () => { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: CODE }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(200); + expect(data.success).toBe(true); + expect(data.role).toBe("admin"); + expect(mockLogin).toHaveBeenCalledWith("admin", "admin@libredb.org"); + }); + + test("distinguishes a wrong code from a missing one", async () => { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: "000000" }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(401); + expect(data.mfaRequired).toBe(true); + expect(data.message).toBe("Invalid authentication code"); + expect(mockLogin).not.toHaveBeenCalled(); + }); + + test("refuses a code that has already been spent (RFC 6238 replay guard)", async () => { + expect((await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: CODE })).status).toBe(200); + + const replay = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: CODE }); + const data = await parseResponseJSON(replay); + + expect(replay.status).toBe(401); + expect(data.message).toBe("Invalid authentication code"); + expect(mockLogin).toHaveBeenCalledTimes(1); + }); + + test("treats a non-string code as no code rather than a 500", async () => { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: 287082 }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(401); + expect(data.message).toBe("Enter the 6-digit code from your authenticator app"); + }); + + test("keeps the uniform 401 when the password is wrong, even alongside a valid code", async () => { + const res = await attempt({ email: "admin@libredb.org", password: "wrong", totp: CODE }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(401); + expect(data.message).toBe("Invalid email or password"); + // No mfaRequired flag: whether this account has a second factor must stay unstated to + // anyone who has not already proved they hold the password. + expect(data.mfaRequired).toBeUndefined(); + }); + + test("leaves an account without a secret on the single-step flow", async () => { + delete process.env.ADMIN_TOTP_SECRET; + + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026" }); + + expect(res.status).toBe(200); + }); + + test("does not spend the client budget on the code prompt itself", async () => { + // The client bucket allows five FAILURES. Six prompts is past that, so if the prompt were + // charged, the code below would meet a 429 instead of a session — five ordinary logins a + // window is not a usable limit. + for (let i = 0; i < 6; i += 1) { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026" }); + expect(res.status).toBe(401); + } + + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: CODE }); + + expect(res.status).toBe(200); + }); + + test("does spend the budget on a wrong code, so guessing is bounded", async () => { + for (let i = 0; i < 5; i += 1) { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: "000000" }); + expect(res.status).toBe(401); + } + + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026", totp: CODE }); + + expect(res.status).toBe(429); + }); + + test("returns an actionable 503 when the configured secret is not base32", async () => { + process.env.ADMIN_TOTP_SECRET = "not-base32!"; + + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026" }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(503); + expect(data.message).toContain("ADMIN_TOTP_SECRET"); + }); + + test("still returns 401 when the mfa_required audit emit throws", async () => { + const logSpy = spyOn(console, "log").mockImplementation(() => { + throw new Error("audit sink unavailable"); + }); + try { + const res = await attempt({ email: "admin@libredb.org", password: "LibreDB.2026" }); + const data = await parseResponseJSON(res); + + expect(res.status).toBe(401); + expect(data.mfaRequired).toBe(true); + } finally { + logSpy.mockRestore(); + } + }); + }); }); diff --git a/tests/components/LoginPage.test.tsx b/tests/components/LoginPage.test.tsx index b7cd2e836..6953cca33 100644 --- a/tests/components/LoginPage.test.tsx +++ b/tests/components/LoginPage.test.tsx @@ -461,3 +461,168 @@ describe("LoginPage showcase (issue #425)", () => { } }); }); + +/** + * The second-factor step of the local login flow. The form is deliberately ignorant of whether + * an account is MFA-protected until the server says so, so every case here drives that state the + * only way the real app can: through a response body. + */ +describe("LoginPage TOTP step", () => { + const MFA_PROMPT = "Enter the 6-digit code from your authenticator app"; + const MFA_INVALID = "Invalid authentication code"; + + function respondWith(...bodies: object[]) { + let call = 0; + const mockFetch = mock(() => { + const body = bodies[Math.min(call, bodies.length - 1)]; + call += 1; + return Promise.resolve(new Response(JSON.stringify(body))); + }); + globalThis.fetch = mockFetch as never; + return mockFetch; + } + + function codeInput(container: HTMLElement) { + return container.querySelector("#totp") as HTMLInputElement | null; + } + + /** Fills in the credentials and submits once, leaving the form on whatever step it reached. */ + async function submitCredentials(result: ReturnType) { + await result.user.type(result.emailInput, "admin@libredb.org"); + await result.user.type(result.passwordInput, "LibreDB.2026"); + fireEvent.submit(result.form); + } + + beforeEach(() => { + mockRouterPush.mockClear(); + mockRouterRefresh.mockClear(); + mockToastSuccess.mockClear(); + mockToastError.mockClear(); + globalThis.fetch = mock(() => Promise.resolve(new Response("{}"))) as never; + }); + + afterEach(() => { + cleanup(); + }); + + test("hides the code field until the server asks for one", () => { + const { container } = renderLogin(); + expect(codeInput(container)).toBeNull(); + }); + + test("reveals the code field when the server answers mfaRequired", async () => { + respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + expect(result.getByText("Verify code")).not.toBeNull(); + }); + + test("does not frame the first prompt as an error", async () => { + respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + + // The field appearing is the message. A toast here would read as a failure to a user who + // has done nothing wrong yet. + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + expect(mockToastError).not.toHaveBeenCalled(); + }); + + test("offers the code field to the platform autofill rather than a bare text box", async () => { + respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + const field = codeInput(result.container)!; + expect(field.autocomplete).toBe("one-time-code"); + expect(field.inputMode).toBe("numeric"); + // `number` would strip the leading zero that one code in six starts with. + expect(field.type).toBe("text"); + }); + + test("sends the code alongside the credentials on the second request", async () => { + const mockFetch = respondWith( + { success: false, mfaRequired: true, message: MFA_PROMPT }, + { success: true, role: "admin" }, + ); + + const result = renderLogin(); + await submitCredentials(result); + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + + await result.user.type(codeInput(result.container)!, "287082"); + fireEvent.submit(result.form); + + await waitFor(() => expect(mockFetch).toHaveBeenCalledTimes(2)); + const [, options] = mockFetch.mock.calls[1] as unknown as [string, RequestInit]; + expect(JSON.parse(options.body as string)).toEqual({ + email: "admin@libredb.org", + password: "LibreDB.2026", + totp: "287082", + }); + await waitFor(() => expect(mockRouterPush).toHaveBeenCalledWith("/admin")); + }); + + test("surfaces the server's wording when a code is rejected, and clears the field", async () => { + respondWith( + { success: false, mfaRequired: true, message: MFA_PROMPT }, + { success: false, mfaRequired: true, message: MFA_INVALID }, + ); + + const result = renderLogin(); + await submitCredentials(result); + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + + await result.user.type(codeInput(result.container)!, "000000"); + fireEvent.submit(result.form); + + await waitFor(() => expect(mockToastError).toHaveBeenCalledWith(MFA_INVALID)); + expect(codeInput(result.container)!.value).toBe(""); + }); + + test("refuses to submit an empty code rather than spending a rate-limit slot on it", async () => { + const mockFetch = respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + + fireEvent.submit(result.form); + + expect(mockToastError).toHaveBeenCalledWith("Please enter your authentication code"); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + test("drops back to the password step when the email is edited", async () => { + respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + + await result.user.type(result.emailInput, "x"); + + // A code minted for the previous account would fail and cost that account a slot in the + // per-account limiter, so the step resets with the credentials it was issued against. + await waitFor(() => expect(codeInput(result.container)).toBeNull()); + expect(result.getByText("Sign In")).not.toBeNull(); + }); + + test("drops back to the password step when the password is edited", async () => { + respondWith({ success: false, mfaRequired: true, message: MFA_PROMPT }); + + const result = renderLogin(); + await submitCredentials(result); + await waitFor(() => expect(codeInput(result.container)).not.toBeNull()); + + await result.user.type(result.passwordInput, "x"); + + await waitFor(() => expect(codeInput(result.container)).toBeNull()); + }); +}); diff --git a/tests/helpers/rfc6238.ts b/tests/helpers/rfc6238.ts new file mode 100644 index 000000000..8b90618b1 --- /dev/null +++ b/tests/helpers/rfc6238.ts @@ -0,0 +1,38 @@ +/** + * RFC 6238 Appendix B's seed, shared by every test that needs a known-good TOTP secret. + * + * Derived rather than pasted as a base32 literal. A 32-character high-entropy string assigned to + * a constant named `SECRET` is exactly the shape `.gitleaks.toml`'s `generic-api-key` rule fires + * on, and the required Secret Scan reports a fabricated fixture the same way it reports a real + * credential. A `.gitleaksignore` fingerprint is the wrong remedy for unmerged work: fingerprints + * are `commit:file:rule:line`, so a squash-merge renames the commit and the entry goes stale. + * + * Encoding it here also makes the RFC's own claim executable — that the seed IS the ASCII string + * below — instead of leaving the reader to trust a base32 blob. `tests/unit/lib/totp.test.ts` + * closes the loop by decoding it back through the app's own `decodeBase32`. + */ +const BASE32_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"; + +/** The seed every published SHA-1 vector in RFC 6238 Appendix B is computed against. */ +export const RFC6238_SEED_ASCII = "12345678901234567890"; + +function encodeBase32(input: string): string { + let accumulator = 0; + let bits = 0; + let output = ""; + for (const byte of Buffer.from(input, "ascii")) { + accumulator = (accumulator << 8) | byte; + bits += 8; + while (bits >= 5) { + bits -= 5; + output += BASE32_ALPHABET[(accumulator >> bits) & 0x1f]; + } + } + // A trailing partial group is left-aligned into a final character; 20 bytes divide evenly into + // 32 characters, so this only guards a future caller passing a length that does not. + if (bits > 0) output += BASE32_ALPHABET[(accumulator << (5 - bits)) & 0x1f]; + return output; +} + +/** Base32 form of the seed: what an operator pastes into ADMIN_TOTP_SECRET. 160 bits, 32 chars. */ +export const RFC6238_SECRET = encodeBase32(RFC6238_SEED_ASCII); diff --git a/tests/security/mfa-second-factor.test.ts b/tests/security/mfa-second-factor.test.ts new file mode 100644 index 000000000..311a92165 --- /dev/null +++ b/tests/security/mfa-second-factor.test.ts @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; +import { clearRateLimitState } from "@/lib/api/rate-limit"; +import { clearTotpReplayState } from "@/lib/totp"; +import { RFC6238_SECRET } from "../helpers/rfc6238"; + +/** + * Control 1.6. Threat: an attacker who already holds a working password — leaked, reused from + * another breach, or read out of a `docker inspect`. + * + * Two properties, and the whole control is worth nothing without both: + * 1. The password alone never produces a session. Not a session with reduced scope, not a + * session pending verification: no `login()` call at all. + * 2. An accepted code cannot be presented twice. A code is valid for up to 90 seconds across + * the skew window, so without this a code captured in transit — a phishing proxy, a logged + * request body, a shoulder-surf — is a second, replayable credential. + * + * A third property is asserted negatively: the "code required" reply must stay unreachable + * without the password, or it would undo the uniform 401 that control 1.5 exists to guarantee. + */ + +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const mockLogin = mock(async (_role: string, _email?: string) => {}); + +mock.module("@/lib/auth", () => ({ + login: mockLogin, + signJWT: mock(async () => "mock-token"), + verifyJWT: mock(async () => null), + getSession: mock(async () => null), + logout: mock(async () => {}), +})); + +const { POST } = await import("@/app/api/auth/login/route"); + +/** RFC 6238 Appendix B seed; "287082" is its six-digit code for the step containing T=59. */ +const SECRET = RFC6238_SECRET; +const CODE = "287082"; +const PASSWORD = "correct-horse-battery-staple"; +const FROZEN_NOW = 59_000; + +interface LoginBody { + success: boolean; + message: string; + mfaRequired?: boolean; +} + +function attempt(body: Record): Promise { + return POST( + new Request("http://localhost:3000/api/auth/login", { + method: "POST", + headers: { "content-type": "application/json", "x-forwarded-for": "203.0.113.90" }, + body: JSON.stringify(body), + }) as never, + ); +} + +async function read(res: Response): Promise { + return (await res.json()) as LoginBody; +} + +describe("control 1.6 — a password alone does not open a TOTP-protected account", () => { + const ENV_KEYS = ["ADMIN_PASSWORD", "USER_PASSWORD", "ADMIN_TOTP_SECRET", "USER_TOTP_SECRET"] as const; + const snapshot: Record = {}; + let nowSpy: ReturnType>; + let logSpy: ReturnType>; + + beforeEach(() => { + for (const key of ENV_KEYS) snapshot[key] = process.env[key]; + process.env.ADMIN_PASSWORD = PASSWORD; + delete process.env.USER_PASSWORD; + delete process.env.USER_TOTP_SECRET; + process.env.ADMIN_TOTP_SECRET = SECRET; + // Both the code check and the replay guard read the wall clock, so the flow is pinned to the + // instant the published vector is valid at. + nowSpy = spyOn(Date, "now").mockReturnValue(FROZEN_NOW); + logSpy = spyOn(console, "log").mockImplementation(() => {}); + clearRateLimitState(); + clearTotpReplayState(); + mockLogin.mockClear(); + }); + + afterEach(() => { + for (const key of ENV_KEYS) { + const value = snapshot[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + nowSpy.mockRestore(); + logSpy.mockRestore(); + clearRateLimitState(); + clearTotpReplayState(); + }); + + test("the correct password on its own creates no session", async () => { + const res = await attempt({ email: "admin@libredb.org", password: PASSWORD }); + + expect(res.status).toBe(401); + expect(mockLogin).not.toHaveBeenCalled(); + }); + + test("a guessed code creates no session", async () => { + for (const guess of ["000000", "111111", "287081"]) { + const res = await attempt({ email: "admin@libredb.org", password: PASSWORD, totp: guess }); + expect(res.status).toBe(401); + } + + expect(mockLogin).not.toHaveBeenCalled(); + }); + + test("an accepted code cannot be presented a second time", async () => { + const first = await attempt({ email: "admin@libredb.org", password: PASSWORD, totp: CODE }); + expect(first.status).toBe(200); + + const replay = await attempt({ email: "admin@libredb.org", password: PASSWORD, totp: CODE }); + + expect(replay.status).toBe(401); + expect((await read(replay)).message).toBe("Invalid authentication code"); + expect(mockLogin).toHaveBeenCalledTimes(1); + }); + + test("the replay guard survives the skew window that made the code replayable", async () => { + expect((await attempt({ email: "admin@libredb.org", password: PASSWORD, totp: CODE })).status).toBe(200); + + // One step later the same code is still within the accepted window - which is exactly the + // interval a captured code would otherwise stay usable for. + nowSpy.mockReturnValue(FROZEN_NOW + 30_000); + const replay = await attempt({ email: "admin@libredb.org", password: PASSWORD, totp: CODE }); + + expect(replay.status).toBe(401); + expect(mockLogin).toHaveBeenCalledTimes(1); + }); + + test("the second-factor reply stays unreachable without the password (control 1.5 holds)", async () => { + const wrongPassword = await read(await attempt({ email: "admin@libredb.org", password: "guess" })); + const unknownEmail = await read(await attempt({ email: "nobody@example.com", password: "guess" })); + + // Byte-identical, and neither admits that this deployment has a second factor at all. + expect(wrongPassword).toEqual(unknownEmail); + expect(wrongPassword.mfaRequired).toBeUndefined(); + expect(wrongPassword.message).toBe("Invalid email or password"); + }); + + test("an account with no secret configured is unaffected", async () => { + delete process.env.ADMIN_TOTP_SECRET; + + const res = await attempt({ email: "admin@libredb.org", password: PASSWORD }); + + expect(res.status).toBe(200); + expect(mockLogin).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/security/route-auth.test.ts b/tests/security/route-auth.test.ts index 7c9d2439f..da14da897 100644 --- a/tests/security/route-auth.test.ts +++ b/tests/security/route-auth.test.ts @@ -399,6 +399,7 @@ describe("routes that reach a provider require a session", () => { "@/lib/seed": "reads seed connection metadata from config; never connects", "@/lib/storage/factory": "the app's own storage backend (STORAGE_PROVIDER), not a user database", "@/lib/storage/types": "the storage backend's interfaces", + "@/lib/totp": "second-factor verification: an HMAC over the submitted code and an in-process spent-step map", }; /** diff --git a/tests/unit/helm-chart-totp.test.ts b/tests/unit/helm-chart-totp.test.ts new file mode 100644 index 000000000..23a9426e3 --- /dev/null +++ b/tests/unit/helm-chart-totp.test.ts @@ -0,0 +1,159 @@ +/** + * TOTP second-factor wiring in the chart. + * + * The property that matters is that the secret travels as a Secret. `extraEnv` could already + * deliver ADMIN_TOTP_SECRET before this wiring existed, but it writes the literal value into the + * Deployment's pod spec, where anyone with `get deployments` can read it — an MFA feature whose + * only Helm path leaks the shared secret to a wider audience than the password it protects is + * worse than none. Everything else here follows from MFA being opt-in per account: neither key + * may ever be required, and neither may appear when it was not asked for. + * + * Exercised against real `helm template` output, like the sibling chart tests. + */ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; +import { parseAllDocuments } from "yaml"; +import { RFC6238_SECRET } from "../helpers/rfc6238"; +import { decodeBase32, TOTP_MIN_SECRET_BYTES } from "@/lib/totp"; + +const CHART_DIR = join(import.meta.dir, "../../charts/libredb-studio"); +const RELEASE = "release-under-test"; +const SECRET_NAME = `${RELEASE}-libredb-studio`; + +/** RFC 6238's Appendix B seed, reused here purely as a known-good base32 string. */ +const SECRET = RFC6238_SECRET; + +interface EnvVar { + name: string; + value?: string; + valueFrom?: { secretKeyRef?: { name: string; key: string; optional?: boolean } }; +} + +interface RenderedManifest { + kind: string; + metadata: { name: string }; + data?: Record; + spec?: { template: { spec: { containers: Array<{ env?: EnvVar[] }> } } }; +} + +function renderChart(extraArgs: string[] = []): { secret?: RenderedManifest; env: EnvVar[] } { + const run = Bun.spawnSync(["helm", "template", RELEASE, CHART_DIR, ...extraArgs], { + stdout: "pipe", + stderr: "pipe", + }); + if (run.exitCode !== 0) { + throw new Error(`helm template failed (exit ${run.exitCode}): ${run.stderr.toString()}`); + } + const docs = parseAllDocuments(run.stdout.toString()).map((doc) => doc.toJSON() as RenderedManifest); + const secret = docs.find((doc) => doc?.kind === "Secret" && doc.metadata.name === SECRET_NAME); + const deployment = docs.find((doc) => doc?.kind === "Deployment"); + if (!deployment) throw new Error("no Deployment manifest found in rendered chart output"); + return { secret, env: deployment.spec?.template.spec.containers[0].env ?? [] }; +} + +function envVar(env: EnvVar[], name: string): EnvVar | undefined { + return env.find((entry) => entry.name === name); +} + +describe("charts/libredb-studio TOTP second factor", () => { + test("writes neither key nor env when no TOTP secret is configured", () => { + const { secret, env } = renderChart(); + + expect(Object.keys(secret?.data ?? {})).not.toContain("admin-totp-secret"); + expect(envVar(env, "ADMIN_TOTP_SECRET")).toBeUndefined(); + expect(envVar(env, "USER_TOTP_SECRET")).toBeUndefined(); + }); + + test("carries the admin secret through the Secret, never inline in the pod spec", () => { + const { secret, env } = renderChart(["--set", `secrets.adminTotpSecret=${SECRET}`]); + + expect(secret?.data?.["admin-totp-secret"]).toBe(Buffer.from(SECRET).toString("base64")); + + const rendered = envVar(env, "ADMIN_TOTP_SECRET"); + expect(rendered?.valueFrom?.secretKeyRef).toMatchObject({ name: SECRET_NAME, key: "admin-totp-secret" }); + // The decisive assertion: the literal secret is nowhere in the Deployment. + expect(rendered?.value).toBeUndefined(); + }); + + test("keeps each account's secret independent", () => { + const { secret, env } = renderChart([ + "--set", + "secrets.userPassword=user-secret", + "--set", + `secrets.userTotpSecret=${SECRET}`, + ]); + + expect(secret?.data?.["user-totp-secret"]).toBe(Buffer.from(SECRET).toString("base64")); + expect(envVar(env, "USER_TOTP_SECRET")?.valueFrom?.secretKeyRef?.key).toBe("user-totp-secret"); + expect(envVar(env, "ADMIN_TOTP_SECRET")).toBeUndefined(); + }); + + test("marks both refs optional even in strict mode, because MFA is opt-in", () => { + // Strict mode is where a hard secretKeyRef would keep the pod from starting. A second + // factor the operator never asked for must not be able to do that. + const { env } = renderChart([ + "--set", + "config.authBootstrap=off", + "--set", + "secrets.jwtSecret=not-a-secret-helm-template-fixture-value", + "--set", + "secrets.adminPassword=admin-secret", + "--set", + `secrets.adminTotpSecret=${SECRET}`, + ]); + + expect(envVar(env, "ADMIN_TOTP_SECRET")?.valueFrom?.secretKeyRef?.optional).toBe(true); + }); + + test("references both keys under an existingSecret so a pre-provisioned one can carry them", () => { + const { env } = renderChart(["--set", "secrets.existingSecret=byo-auth"]); + + expect(envVar(env, "ADMIN_TOTP_SECRET")?.valueFrom?.secretKeyRef).toMatchObject({ + name: "byo-auth", + key: "admin-totp-secret", + // Optional is what lets an existingSecret predating MFA keep working untouched. + optional: true, + }); + expect(envVar(env, "USER_TOTP_SECRET")?.valueFrom?.secretKeyRef?.name).toBe("byo-auth"); + }); + + /** + * values.schema.json is the only place a bad secret can be caught before the pod runs, and the + * chart README promises exactly that. The promise only holds while the pattern and the app's + * own reader agree, and they did not: `AB=CD` and a single `A` passed the schema and then took + * the login route down with a 503, while a hyphen-grouped or space-prefixed secret the app + * accepts happily was refused at install. + * + * So the expectation is computed from the app's reader rather than written down beside it. + * Either side changing alone shows up here as a failure instead of as an operator's 503. + */ + describe("the install-time check agrees with the app's own reader", () => { + const CASES = [ + RFC6238_SECRET, + RFC6238_SECRET.toLowerCase(), + RFC6238_SECRET.replace(/(.{4})/g, "$1 ").trim(), + `${RFC6238_SECRET}====`, + `-${RFC6238_SECRET}`, + ` ${RFC6238_SECRET}`, + "not-base32!", + "AB=CD", + "A", + RFC6238_SECRET.slice(0, 25), + ]; + + for (const value of CASES) { + const decoded = decodeBase32(value); + const appAccepts = decoded !== null && decoded.length >= TOTP_MIN_SECRET_BYTES; + + test(`${appAccepts ? "installs" : "refuses"} ${JSON.stringify(value)}`, () => { + const run = Bun.spawnSync( + ["helm", "template", RELEASE, CHART_DIR, "--set-string", `secrets.adminTotpSecret=${value}`], + { stdout: "pipe", stderr: "pipe" }, + ); + + expect(run.exitCode === 0).toBe(appAccepts); + if (!appAccepts) expect(run.stderr.toString()).toContain("adminTotpSecret"); + }); + } + }); +}); diff --git a/tests/unit/lib/local-auth.test.ts b/tests/unit/lib/local-auth.test.ts index 40a2ba60c..c2dbddee9 100644 --- a/tests/unit/lib/local-auth.test.ts +++ b/tests/unit/lib/local-auth.test.ts @@ -1,18 +1,28 @@ import { describe, test, expect, beforeEach, afterEach } from "bun:test"; import { AuthConfigError } from "@/lib/auth-errors"; import { getAuthUsers } from "@/lib/local-auth"; +import { RFC6238_SECRET } from "../../helpers/rfc6238"; describe("local-auth getAuthUsers()", () => { let origAdminEmail: string | undefined; let origAdminPassword: string | undefined; let origUserEmail: string | undefined; let origUserPassword: string | undefined; + let origAdminTotp: string | undefined; + let origUserTotp: string | undefined; + + /** RFC 6238's Appendix B seed, reused here purely as a known-good base32 string. */ + const VALID_SECRET = RFC6238_SECRET; beforeEach(() => { origAdminEmail = process.env.ADMIN_EMAIL; origAdminPassword = process.env.ADMIN_PASSWORD; origUserEmail = process.env.USER_EMAIL; origUserPassword = process.env.USER_PASSWORD; + origAdminTotp = process.env.ADMIN_TOTP_SECRET; + origUserTotp = process.env.USER_TOTP_SECRET; + delete process.env.ADMIN_TOTP_SECRET; + delete process.env.USER_TOTP_SECRET; }); afterEach(() => { @@ -20,6 +30,8 @@ describe("local-auth getAuthUsers()", () => { restore("ADMIN_PASSWORD", origAdminPassword); restore("USER_EMAIL", origUserEmail); restore("USER_PASSWORD", origUserPassword); + restore("ADMIN_TOTP_SECRET", origAdminTotp); + restore("USER_TOTP_SECRET", origUserTotp); }); function restore(key: string, value: string | undefined): void { @@ -68,4 +80,110 @@ describe("local-auth getAuthUsers()", () => { expect(users.find((u) => u.role === "admin")?.email).toBe("admin@libredb.org"); expect(users.find((u) => u.role === "user")?.email).toBe("user@libredb.org"); }); + + describe("TOTP secrets", () => { + beforeEach(() => { + process.env.ADMIN_PASSWORD = "admin-secret"; + }); + + test("leaves both accounts without a second factor when neither variable is set", () => { + process.env.USER_PASSWORD = "user-secret"; + + const users = getAuthUsers(); + + expect(users.every((u) => u.totpSecret === undefined)).toBe(true); + }); + + test("attaches ADMIN_TOTP_SECRET to the admin account", () => { + process.env.ADMIN_TOTP_SECRET = VALID_SECRET; + + expect(getAuthUsers()[0].totpSecret).toBe(VALID_SECRET); + }); + + test("attaches USER_TOTP_SECRET to the user account only", () => { + process.env.USER_PASSWORD = "user-secret"; + process.env.USER_TOTP_SECRET = VALID_SECRET; + + const users = getAuthUsers(); + + expect(users.find((u) => u.role === "user")?.totpSecret).toBe(VALID_SECRET); + expect(users.find((u) => u.role === "admin")?.totpSecret).toBeUndefined(); + }); + + test("protects each account independently", () => { + process.env.USER_PASSWORD = "user-secret"; + process.env.ADMIN_TOTP_SECRET = VALID_SECRET; + + const users = getAuthUsers(); + + expect(users.find((u) => u.role === "admin")?.totpSecret).toBe(VALID_SECRET); + expect(users.find((u) => u.role === "user")?.totpSecret).toBeUndefined(); + }); + + test("trims the surrounding whitespace an env file or secret manager leaves behind", () => { + process.env.ADMIN_TOTP_SECRET = ` ${VALID_SECRET}\n`; + + expect(getAuthUsers()[0].totpSecret).toBe(VALID_SECRET); + }); + + test("treats an empty value as no second factor, so MFA can be turned off by blanking it", () => { + process.env.ADMIN_TOTP_SECRET = " "; + + expect(getAuthUsers()[0].totpSecret).toBeUndefined(); + }); + + /** + * RFC 4226 R6 makes 128 bits a MUST, and a secret below it is not a weaker second factor but + * an absent one: a single observed code narrows an 8-bit key to one candidate. The alphabet + * check alone let `AA` through, which reads as MFA everywhere in the UI and the docs while + * costing an attacker nothing. Sliced off the RFC seed so the boundary is unmistakable. + */ + test("rejects a secret below the 128 bits RFC 4226 requires", () => { + process.env.ADMIN_TOTP_SECRET = VALID_SECRET.slice(0, 25); // 125 bits -> 15 whole bytes + + expect(() => getAuthUsers()).toThrow(AuthConfigError); + }); + + test("says the secret is too short rather than repeating the base32 hint", () => { + process.env.ADMIN_TOTP_SECRET = VALID_SECRET.slice(0, 25); + + expect(() => getAuthUsers()).toThrow(/ADMIN_TOTP_SECRET is too short/); + }); + + test("accepts a secret of exactly the minimum length", () => { + process.env.ADMIN_TOTP_SECRET = VALID_SECRET.slice(0, 26); // 130 bits -> 16 whole bytes + + expect(getAuthUsers()[0].totpSecret).toBe(VALID_SECRET.slice(0, 26)); + }); + + test("measures the decoded length, not the pasted one, so grouping does not fake it", () => { + // 16 base32 characters is 10 bytes however it is spaced out; the separators are not payload. + process.env.ADMIN_TOTP_SECRET = VALID_SECRET.slice(0, 16) + .replace(/(.{4})/g, "$1 ") + .trim(); + + expect(() => getAuthUsers()).toThrow(/ADMIN_TOTP_SECRET is too short/); + }); + + test("throws AuthConfigError when ADMIN_TOTP_SECRET is not base32", () => { + process.env.ADMIN_TOTP_SECRET = "definitely-not-base32!"; + + expect(() => getAuthUsers()).toThrow(AuthConfigError); + }); + + test("names the offending variable so the operator knows which one to fix", () => { + process.env.USER_PASSWORD = "user-secret"; + process.env.USER_TOTP_SECRET = "0189"; + + expect(() => getAuthUsers()).toThrow(/USER_TOTP_SECRET/); + }); + + test("ignores USER_TOTP_SECRET when there is no user account to protect", () => { + delete process.env.USER_PASSWORD; + // Invalid on purpose: an inert variable must not be able to break the whole login route. + process.env.USER_TOTP_SECRET = "not-base32!"; + + expect(getAuthUsers()).toHaveLength(1); + }); + }); }); diff --git a/tests/unit/lib/totp.test.ts b/tests/unit/lib/totp.test.ts new file mode 100644 index 000000000..ce9f5f7f7 --- /dev/null +++ b/tests/unit/lib/totp.test.ts @@ -0,0 +1,126 @@ +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { claimTotpStep, clearTotpReplayState, decodeBase32, TOTP_PERIOD_SECONDS, verifyTotp } from "@/lib/totp"; +import { RFC6238_SECRET as RFC_SECRET, RFC6238_SEED_ASCII } from "../../helpers/rfc6238"; + +/** `T` values from RFC 6238 Appendix B, each paired with the low six digits of its vector. */ +const RFC_VECTORS = [ + { seconds: 59, code: "287082" }, + { seconds: 1111111109, code: "081804" }, + { seconds: 1111111111, code: "050471" }, + { seconds: 1234567890, code: "005924" }, + { seconds: 2000000000, code: "279037" }, +]; + +describe("decodeBase32", () => { + test("decodes the RFC 6238 seed back to its ASCII bytes", () => { + expect(decodeBase32(RFC_SECRET)?.toString("utf8")).toBe(RFC6238_SEED_ASCII); + }); + + test("accepts the spacing, casing and padding operators actually paste", () => { + const spaced = decodeBase32("gezd gnbv-gy3t qojq gezd gnbv gy3t qojq====="); + expect(spaced?.toString("utf8")).toBe(RFC6238_SEED_ASCII); + }); + + test("rejects a secret containing a character outside the base32 alphabet", () => { + expect(decodeBase32("GEZDGNBV1")).toBeNull(); + }); + + test("rejects a secret that is only separators and padding", () => { + expect(decodeBase32(" -- ==")).toBeNull(); + }); + + test("rejects a secret too short to yield a whole byte", () => { + expect(decodeBase32("A")).toBeNull(); + }); +}); + +describe("verifyTotp", () => { + for (const { seconds, code } of RFC_VECTORS) { + test(`accepts the RFC 6238 vector at T=${seconds}`, () => { + expect(verifyTotp(RFC_SECRET, code, seconds * 1000)).toBe(Math.floor(seconds / TOTP_PERIOD_SECONDS)); + }); + } + + test("accepts the previous step, so a slow typist is not punished", () => { + expect(verifyTotp(RFC_SECRET, "287082", (59 + TOTP_PERIOD_SECONDS) * 1000)).toBe(1); + }); + + test("accepts the next step, so a client clock running fast still works", () => { + expect(verifyTotp(RFC_SECRET, "287082", (59 - TOTP_PERIOD_SECONDS) * 1000)).toBe(1); + }); + + test("rejects a code two steps out", () => { + expect(verifyTotp(RFC_SECRET, "287082", (59 + 2 * TOTP_PERIOD_SECONDS) * 1000)).toBeNull(); + }); + + test("tolerates whitespace inside the submitted code", () => { + expect(verifyTotp(RFC_SECRET, " 287 082 ", 59_000)).toBe(1); + }); + + for (const code of ["12345", "1234567", "abcdef", "", "12345a"]) { + test(`rejects the malformed code "${code}"`, () => { + expect(verifyTotp(RFC_SECRET, code, 59_000)).toBeNull(); + }); + } + + test("rejects every code when the configured secret is not valid base32", () => { + expect(verifyTotp("not base32!", "287082", 59_000)).toBeNull(); + }); + + test("reads the wall clock when no timestamp is supplied", () => { + // Stubbed rather than asserted against the real clock: a test that submitted a fixed code at + // the true current time would pass or fail by luck. + const nowSpy = spyOn(Date, "now").mockReturnValue(59_000); + try { + expect(verifyTotp(RFC_SECRET, "287082")).toBe(1); + } finally { + nowSpy.mockRestore(); + } + }); +}); + +describe("claimTotpStep", () => { + afterEach(() => { + clearTotpReplayState(); + }); + + test("accepts a step once and refuses the same step again", () => { + expect(claimTotpStep("account-a", 1, 59_000)).toBe(true); + expect(claimTotpStep("account-a", 1, 59_000)).toBe(false); + }); + + test("scopes spent steps per account", () => { + expect(claimTotpStep("account-a", 1, 59_000)).toBe(true); + expect(claimTotpStep("account-b", 1, 59_000)).toBe(true); + }); + + test("lets the same account spend the next step", () => { + expect(claimTotpStep("account-a", 1, 59_000)).toBe(true); + expect(claimTotpStep("account-a", 2, 89_000)).toBe(true); + }); + + test("forgets a step once its acceptance window has passed", () => { + expect(claimTotpStep("account-a", 1, 59_000)).toBe(true); + // Far enough ahead that verifyTotp could no longer accept step 1 at all, so remembering it + // buys nothing and the entry is reclaimed. + expect(claimTotpStep("account-a", 1, 59_000 + 10 * TOTP_PERIOD_SECONDS * 1000)).toBe(true); + }); + + test("bounds its own memory rather than growing with attacker-supplied keys", () => { + const start = 59_000; + for (let index = 0; index < 5000; index++) claimTotpStep(`account-${index}`, 1, start); + // The earliest entries were evicted, so the very first key is claimable again: the cap held + // and it failed open, exactly as documented. + expect(claimTotpStep("account-0", 1, start)).toBe(true); + }); + + test("reads the wall clock when no timestamp is supplied", () => { + const nowSpy = spyOn(Date, "now").mockReturnValue(59_000); + try { + expect(claimTotpStep("account-a", 1)).toBe(true); + expect(claimTotpStep("account-a", 1)).toBe(false); + } finally { + nowSpy.mockRestore(); + } + }); +});