mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 15:02:04 +00:00
feat(withings): random-nonce OAuth state with single-use ledger row
The legacy OAuth state shape `${user.id}:${random16}` (v1.4.42-) put
the user id on both the URL state param and the `withings_state`
cookie. httpOnly + Secure + sameSite:lax shut the XSS exfiltration
surface, but the user id still leaked into any request-log entry
where the cookie is not redacted and into network-traffic captures
of the redirect. Cosmetic today; a future refactor flipping the
cookie to non-httpOnly would silently expose it.
The v1.4.43 security audit (L-1) recommended switching the state to
a fully-random nonce backed by a short-lived `(nonce -> userId)`
ledger row, with the callback resolving the user via the row rather
than parsing it out of the cookie. This change lands the migration
end-to-end:
- New `WithingsOAuthState` table (migration `0076`): nonce PK, userId
FK with cascade, createdAt default-now, expiresAt + index for the
cleanup sweep.
- `withings/connect` mints a 22-char base64url nonce
(`randomBytes(16).toString("base64url")` — 128 bits, OAuth 2.0
§10.10 recommended threshold), persists the row with a 10-minute
TTL, and sets the cookie to JUST the nonce (no user id).
- `withings/callback` verifies the cookie/URL pair byte-for-byte
(timing-safe), then looks up the row, asserts `expiresAt > now()`
AND `row.userId === session.user.id` (defence-in-depth — the audit
rec was to resolve user identity from the row; cross-checking it
against the session covers a stolen-cookie + cross-user scenario),
and deletes the row on every exit branch (single-use semantics, so
a replay of the same nonce fails the second time).
- Daily cleanup cron at 03:20 Europe/Berlin slots between the
audit-log cleanup (:15) and the mood-reminder cleanup (:25); the
helper module mirrors the audit-log-cleanup shape so the worker
contract is symmetric.
Tests:
- 3 unit cases pin the cleanup helper contract (deleteMany shape,
default-now branch, idempotent re-run).
- 2 unit cases pin the mint helper + constants (22-char base64url,
10-minute TTL, cookie name preserved across the deploy window).
- 5 integration cases pin the connect -> callback ledger lifecycle
end-to-end against real Postgres: row mint, no-credentials short-
circuit, single-use consumption + replay rejection, expired-row
rejection, cross-user mismatch rejection.
- The existing `withings-oauth.test.ts` integration cases updated to
reflect the new state shape; the legacy `${user.id}:${random32}`
assertion was the only test relying on the old contract.
Files: 12. Tests: 5076 -> 5108 (+32 unit; integration adds 5 cases
plus 2 case updates on the existing file).
This commit is contained in:
1 parent
887d448ecc
commit
185e626651
12 files changed
+809
-24
No files matched your search
@@ -0,0 +1,59 @@
|
||||
-- v1.4.47 W6 — Withings OAuth state nonce ledger.
|
||||
--
|
||||
-- The v1.4.43 security audit (L-1) flagged the legacy state shape
|
||||
-- `${user.id}:${random16}` as cosmetic-but-fragile: the user id is
|
||||
-- recoverable from any request log entry where the cookie isn't
|
||||
-- redacted, and from a network-traffic capture of the redirect. The
|
||||
-- cookie is `httpOnly` + `Secure` + `sameSite:lax` today so the XSS
|
||||
-- exfiltration surface is closed — but a future refactor that flips
|
||||
-- the cookie to a non-httpOnly variant would silently expose the user
|
||||
-- id. Audit recommendation was to switch the state to a fully-random
|
||||
-- 16-byte nonce backed by a short-lived `(nonce → userId)` row.
|
||||
--
|
||||
-- This table is that ledger. Each `withings/connect` mint persists one
|
||||
-- row; `withings/callback` looks it up, asserts `expiresAt > now()`,
|
||||
-- and DELETEs the row in a single transaction so a replay of the same
|
||||
-- state nonce against a fresh user session fails the CSRF check (the
|
||||
-- row is gone after first consumption). A daily cron sweeps any
|
||||
-- abandoned rows whose `expiresAt` has passed (user closed the
|
||||
-- Withings tab without completing the handshake).
|
||||
--
|
||||
-- Schema choices:
|
||||
--
|
||||
-- * `nonce` is the primary key — it's already random and
|
||||
-- 22-character base64url, so a dedicated `id` column would just
|
||||
-- duplicate the key without buying anything.
|
||||
-- * `userId` carries `ON DELETE CASCADE` so removing a user cleans
|
||||
-- up their in-flight nonces atomically.
|
||||
-- * `expiresAt` is indexed for the cleanup sweep — without it the
|
||||
-- daily `DELETE … WHERE expiresAt < now()` would seq-scan the
|
||||
-- entire table once a day even though the typical row count is
|
||||
-- bounded by `users × concurrent in-flight handshakes`.
|
||||
--
|
||||
-- Idempotent guards (`IF NOT EXISTS`) match the 0067 / 0070 / 0071 /
|
||||
-- 0074 / 0075 pattern so reruns are safe.
|
||||
--
|
||||
-- Reversibility: down migration is `DROP TABLE IF EXISTS
|
||||
-- "withings_oauth_states"`. Additive-only; no existing row is
|
||||
-- touched.
|
||||
|
||||
CREATE TABLE IF NOT EXISTS "withings_oauth_states" (
|
||||
"nonce" TEXT NOT NULL,
|
||||
"user_id" TEXT NOT NULL,
|
||||
"created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
"expires_at" TIMESTAMP(3) NOT NULL,
|
||||
|
||||
CONSTRAINT "withings_oauth_states_pkey" PRIMARY KEY ("nonce")
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS "withings_oauth_states_expires_at_idx"
|
||||
ON "withings_oauth_states" ("expires_at");
|
||||
|
||||
DO $$ BEGIN
|
||||
ALTER TABLE "withings_oauth_states"
|
||||
ADD CONSTRAINT "withings_oauth_states_user_id_fkey"
|
||||
FOREIGN KEY ("user_id") REFERENCES "users"("id")
|
||||
ON DELETE CASCADE ON UPDATE CASCADE;
|
||||
EXCEPTION
|
||||
WHEN duplicate_object THEN NULL;
|
||||
END $$;
|
||||
@@ -234,6 +234,13 @@ model User {
|
||||
refreshTokens RefreshToken[]
|
||||
auditLogs AuditLog[]
|
||||
withingsConnection WithingsConnection?
|
||||
// v1.4.47 W6 — short-lived `(nonce → userId)` ledger backing the
|
||||
// Withings OAuth state check. The connect route mints a row with a
|
||||
// 10-minute TTL; the callback resolves the user via this relation
|
||||
// instead of parsing the cookie value, closing the L-1 audit gap
|
||||
// where the legacy `${user.id}:${nonce}` shape leaked the user id
|
||||
// into request logs / network captures.
|
||||
withingsOAuthStates WithingsOAuthState[]
|
||||
moodEntries MoodEntry[]
|
||||
notificationChannels NotificationChannel[]
|
||||
pushSubscriptions PushSubscription[]
|
||||
@@ -1192,6 +1199,40 @@ model WithingsConnection {
|
||||
@@map("withings_connections")
|
||||
}
|
||||
|
||||
/// v1.4.47 W6 — short-lived ledger of in-flight OAuth state nonces.
|
||||
///
|
||||
/// The legacy state shape `${user.id}:${random16}` (v1.4.42-) persisted
|
||||
/// the user id in plaintext on both the URL state param and the
|
||||
/// `withings_state` cookie. The cookie is httpOnly + Secure +
|
||||
/// sameSite:lax today so XSS exfiltration is closed, but the user id
|
||||
/// is still recoverable from any request-log entry where the cookie
|
||||
/// isn't redacted and from a network-traffic capture of the redirect.
|
||||
/// The v1.4.43 security audit (L-1) recommended decoupling the two by
|
||||
/// switching the state to a fully-random 16-byte nonce backed by this
|
||||
/// short-lived row — the callback resolves the user via the row's
|
||||
/// `userId` instead of the cookie value.
|
||||
///
|
||||
/// Lifecycle:
|
||||
/// * `connect` mints a row with `expiresAt = now() + 10 min` and
|
||||
/// sets the cookie to JUST the nonce (no user id).
|
||||
/// * `callback` looks up the row, asserts `expiresAt > now()`, then
|
||||
/// DELETEs it in the same transaction — replay of the same nonce
|
||||
/// fails the second time because the row is gone.
|
||||
/// * A daily cleanup cron sweeps any abandoned rows whose
|
||||
/// `expiresAt` has passed (user closed the Withings tab without
|
||||
/// completing the handshake).
|
||||
model WithingsOAuthState {
|
||||
nonce String @id
|
||||
userId String @map("user_id")
|
||||
createdAt DateTime @default(now()) @map("created_at")
|
||||
expiresAt DateTime @map("expires_at")
|
||||
|
||||
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
|
||||
|
||||
@@index([expiresAt])
|
||||
@@map("withings_oauth_states")
|
||||
}
|
||||
|
||||
// ─── Mood Entries (moodLog) ───────────────────────────────────
|
||||
|
||||
model MoodEntry {
|
||||
|
||||
Reference in new issue
Block a user