From 03d85c6d133a1d5b35f860f45979ab9f115560a2 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Fri, 14 Aug 2026 15:27:19 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20admin-scoped=202FA=20mint=20targets=20th?= =?UTF-8?q?e=20CUSTOMER=20=E2=80=94=20user=20authentication=20for=20saved?= =?UTF-8?q?=20cards,=20never=20the=20admin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The till and admin payment modal 'Request a new code' buttons previously called the session-scoped POST /api/user/2fa/code, which mints a code for the ADMIN's session — a code that can never satisfy the card-owner gate and is delivered to the admin's log line, not the customer. - New POST /api/admin/users/{id}/2fa/code (AdminSendVerificationCodeHandler, RequireAdmin + per-user limiter): mints/reuses a code for the TARGET user (the card owner/customer), keyed to the CUSTOMER's userID so the [2FA] delivery log carries the customer's ID — the customer, never the admin, is the authentication subject for their card - Shared useTwoFactorCodeForSavedCard composable gains an optional mint() option; admin surfaces (PaymentModal, TillPurchases) pass the customer-scoped mint, customer surfaces keep the session default - Frontend: adminRequestNewTwoFactorCode(userID) in square.ts; PaymentModal mints for booking.user_id, TillPurchases for selectedCustomer.id - Tests: admin mint keys the code to the customer's userID (log line contains customer ID, NOT the admin ID) + pending hash persisted for the customer; unknown target user 404s Backend 26/26 packages; frontend 72/72 + build clean. --- backend/handlers/user/admin_twofa_test.go | 54 +++++++++++++++ backend/handlers/user/twofa.go | 65 +++++++++++++++++++ backend/main.go | 5 ++ .../lib/components/admin/TillPurchases.svelte | 16 +++-- .../components/payments/PaymentModal.svelte | 10 ++- frontend/src/lib/square/square.ts | 30 +++++++++ .../src/lib/stores/twoFactorCode.svelte.ts | 14 +++- 7 files changed, 183 insertions(+), 11 deletions(-) diff --git a/backend/handlers/user/admin_twofa_test.go b/backend/handlers/user/admin_twofa_test.go index 2497885..8a790c4 100644 --- a/backend/handlers/user/admin_twofa_test.go +++ b/backend/handlers/user/admin_twofa_test.go @@ -10,11 +10,15 @@ package user // the package shares db.Conn state. import ( + "bytes" "context" "database/sql" "encoding/json" + "log" "net/http" "net/http/httptest" + "os" + "regexp" "testing" "time" @@ -191,3 +195,53 @@ func TestTwoFAVerify_Enforced_UpdatesLastUsedAt(t *testing.T) { require.True(t, enabled) require.True(t, lastUsed.Valid, "two_factor_last_used_at should be set after a successful verify") } + +// TestAdminSendVerificationCode_MintsForCustomer verifies the admin-scoped mint +// keys the code to the TARGET user (the customer whose saved card is being +// charged), NOT the admin session. The [2FA] delivery log line must carry the +// customer's userID — so the code is delivered to the customer and can satisfy +// the card-owner gate — and must never carry the admin's ID. +func TestAdminSendVerificationCode_MintsForCustomer(t *testing.T) { + twofaEnvEnforced(t) + var buf bytes.Buffer + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + + ctx, tx := testutils.SetupTestTx(t) + customerID, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + _, err = tx.Exec(ctx, `UPDATE users SET two_factor_enabled = true, two_factor_method = 'email' WHERE id = $1`, customerID) + require.NoError(t, err) + + // Admin session userID differs from the target customer. + w := makeAdmin2FARequest(http.HandlerFunc(AdminSendVerificationCodeHandler), http.MethodPost, "/api/admin/users/"+customerID+"/2fa/code", "admin", ctx) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + var resp map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Equal(t, "Code sent", resp["message"]) + _, hasCode := resp["code"] + require.False(t, hasCode, "enforced env must NOT return the code in the response") + + logOut := buf.String() + require.Contains(t, logOut, customerID, "delivery log must key the code to the CUSTOMER's userID") + require.NotContains(t, logOut, "admin-test-id", "delivery log must NOT key the code to the admin session") + require.Regexp(t, regexp.MustCompile(`\[2FA\].*\d{6}`), logOut, "endpoint must log the code as the delivery channel") + + // The customer now has a pending code that would satisfy the card-owner gate. + var pendingHash sql.NullString + require.NoError(t, tx.QueryRow(ctx, `SELECT two_factor_pending_code_hash FROM users WHERE id = $1`, customerID).Scan(&pendingHash)) + require.True(t, pendingHash.Valid, "admin mint must persist a pending code for the customer") +} + +// TestAdminSendVerificationCode_UnknownCustomer_NotFound verifies the admin +// mint 404s for a nonexistent target user (the route-level RequireAdmin +// middleware — applied in main.go — gates role access; the handler owns the +// target-user contract). +func TestAdminSendVerificationCode_UnknownCustomer_NotFound(t *testing.T) { + twofaEnvEnforced(t) + ctx, _ := testutils.SetupTestTx(t) + + w := makeAdmin2FARequest(http.HandlerFunc(AdminSendVerificationCodeHandler), http.MethodPost, "/api/admin/users/no-such-user/2fa/code", "admin", ctx) + require.Equal(t, http.StatusNotFound, w.Code, "unknown target user must 404") +} diff --git a/backend/handlers/user/twofa.go b/backend/handlers/user/twofa.go index b7437f3..829e64e 100644 --- a/backend/handlers/user/twofa.go +++ b/backend/handlers/user/twofa.go @@ -20,6 +20,7 @@ import ( "crussell/mw" "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" ) // twoFARequired reports whether 2FA enforcement is active in this deployment. @@ -614,6 +615,70 @@ func SendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) { } } +// AdminSendVerificationCodeHandler mints (or reuses) a 2FA code for a TARGET +// user, not the session user. The saved-card charge gate verifies the code +// against the CARD OWNER (customer) — never the admin session (till.go:951, +// handlers.go:797) — so a session-scoped mint would key the code to the admin +// and could never authorize the customer's charge. Delivering keyed to the +// customer preserves the invariant that the customer, not the admin, is the +// authentication subject for their card. +func AdminSendVerificationCodeHandler(w http.ResponseWriter, r *http.Request) { + targetUserID := chi.URLParam(r, "id") + if targetUserID == "" { + http.Error(w, "user_id is required", http.StatusBadRequest) + return + } + + var enabled bool + err := db.Conn.QueryRow(r.Context(), `SELECT two_factor_enabled FROM users WHERE id = $1`, targetUserID).Scan(&enabled) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + http.Error(w, "User not found", http.StatusNotFound) + return + } + log.Printf("failed to check 2FA state for user %s: %v", targetUserID, err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + if !enabled { + http.Error(w, "Two-factor authentication is not enabled for this user", http.StatusConflict) + return + } + + // The per-user mutex serializes the mint with the charge gate's verify + // critical section for the CUSTOMER, so concurrent mints (admin + customer + // requesting simultaneously) cannot race the cooldown or lockout counters. + st := twoFAAttemptStateFor(targetUserID) + st.Mu.Lock() + defer st.Mu.Unlock() + + code, remaining, err := ensurePendingTwoFACode(r, targetUserID, st, "saved-card charge") + if err != nil { + if errors.Is(err, errTwoFAMintThrottled) { + http.Error(w, "Too many attempts. Wait before requesting a new code.", http.StatusTooManyRequests) + return + } + if errors.Is(err, errTwoFADeliveryUnavailable) { + http.Error(w, err.Error(), http.StatusServiceUnavailable) + return + } + log.Printf("failed to prepare 2FA code for user %s: %v", targetUserID, err) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + + resp := map[string]any{"message": "Code sent", "remaining_seconds": int(remaining.Seconds())} + if !twoFARequired() && code != "" { + // Dev convenience (matches setup): return the freshly minted code so + // the request path is testable without grepping the backend log. The + // code is never included when 2FA is enforced. + resp["code"] = code + } + if err := json.NewEncoder(w).Encode(resp); err != nil { + log.Printf("failed to encode 2FA code response: %v", err) + } +} + // POST /api/user/2fa/disable // Turns 2FA off and clears method + pending fields for the authenticated user. // diff --git a/backend/main.go b/backend/main.go index 17d643e..2e25732 100644 --- a/backend/main.go +++ b/backend/main.go @@ -764,6 +764,11 @@ func main() { r.Get("/{id}/giftcard-balance", payments.GetUserGiftCardBalanceAdmin) r.Get("/{id}/payment-methods", payments.AdminGetUserPaymentMethods) r.Post("/{id}/2fa/remove", user.AdminRemoveUser2FAHandler) + // Admin-scoped 2FA mint: the operator requests a code FOR the + // customer whose saved card is being charged at the till/admin + // payment modal. Mints keyed to the CUSTOMER so the code is + // delivered to the customer and can satisfy the card-owner gate. + r.With(mw.RateLimitByUser(10, time.Minute)).Post("/{id}/2fa/code", user.AdminSendVerificationCodeHandler) }) r.Route("/admin/today", func(r chi.Router) { diff --git a/frontend/src/lib/components/admin/TillPurchases.svelte b/frontend/src/lib/components/admin/TillPurchases.svelte index 892da7f..72be188 100644 --- a/frontend/src/lib/components/admin/TillPurchases.svelte +++ b/frontend/src/lib/components/admin/TillPurchases.svelte @@ -8,11 +8,13 @@ import { apiFetch } from '$lib/utils/api'; import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte'; import TwoFactorCodeInput from '$lib/components/payments/TwoFactorCodeInput.svelte'; - import { - isSquareConfigured, - isTwoFactorVerificationGateFailure, - submitPaymentWithRetry - } from '$lib/square/square'; + import { + isSquareConfigured, + isTwoFactorVerificationGateFailure, + submitPaymentWithRetry, + adminRequestNewTwoFactorCode, + requestNewTwoFactorCode + } from '$lib/square/square'; import { authStore } from '$lib/stores/auth.svelte'; import { useTwoFactorCodeForSavedCard } from '$lib/stores/twoFactorCode.svelte'; @@ -121,7 +123,9 @@ const savedCardChargeRequires2FACode = $derived(authStore.savedCardChargeRequires2FACode); const twoFactor = useTwoFactorCodeForSavedCard({ enabled: () => true, - gateActive: () => savedCardChargeRequires2FACode && paymentMethod === 'saved_card' + gateActive: () => savedCardChargeRequires2FACode && paymentMethod === 'saved_card', + mint: () => + selectedCustomer?.id ? adminRequestNewTwoFactorCode(selectedCustomer.id) : requestNewTwoFactorCode() }); // The saved-card option is hidden outright unless a customer is selected diff --git a/frontend/src/lib/components/payments/PaymentModal.svelte b/frontend/src/lib/components/payments/PaymentModal.svelte index e29bcb0..4ea31bc 100644 --- a/frontend/src/lib/components/payments/PaymentModal.svelte +++ b/frontend/src/lib/components/payments/PaymentModal.svelte @@ -14,7 +14,9 @@ isTwoFactorVerificationGateFailure, sanitizeDecimalInput, SAVED_CARD_VERIFICATION_MESSAGE, - submitPaymentWithRetry + submitPaymentWithRetry, + adminRequestNewTwoFactorCode, + requestNewTwoFactorCode } from '$lib/square/square'; import { authStore } from '$lib/stores/auth.svelte'; import TwoFactorCodeInput from '$lib/components/payments/TwoFactorCodeInput.svelte'; @@ -88,7 +90,11 @@ // irrelevant to the backend gate, so `enabled` is always true. const twoFactor = useTwoFactorCodeForSavedCard({ enabled: () => true, - gateActive: () => twoFactorEnforced && customerTwoFactorEnabled + gateActive: () => twoFactorEnforced && customerTwoFactorEnabled, + mint: () => { + const customerID = booking.user_id ?? booking.user?.id; + return customerID ? adminRequestNewTwoFactorCode(customerID) : requestNewTwoFactorCode(); + } }); // Focus the verification-code input whenever the saved-card screen shows it diff --git a/frontend/src/lib/square/square.ts b/frontend/src/lib/square/square.ts index 2d43a53..7e49ff0 100644 --- a/frontend/src/lib/square/square.ts +++ b/frontend/src/lib/square/square.ts @@ -179,6 +179,36 @@ export async function requestNewTwoFactorCode(): Promise { + const headers: Record = {}; + if (typeof localStorage !== 'undefined') { + const token = localStorage.getItem('authToken'); + if (token) headers['Authorization'] = `Bearer ${token}`; + } + try { + const response = await fetch(`/api/admin/users/${encodeURIComponent(userID)}/2fa/code`, { method: 'POST', headers }); + if (response.ok) { + const data = (await response.json().catch(() => null)) as { message?: unknown } | null; + const message = + typeof data?.message === 'string' ? data.message : 'A new verification code has been sent.'; + return { status: response.status, ok: true, message }; + } + const body = await response.text(); + return { + status: response.status, + ok: false, + message: extractServerErrorMessage(body) || 'Failed to request a new verification code' + }; + } catch { + return { status: 0, ok: false, message: 'Network error requesting a new code' }; + } +} + /** Minimal `{"error"|"message": "..."}` extractor for the 2FA code-request * endpoint bodies (429/503), kept inline so square.ts stays import-free for * the vitest suite. */ diff --git a/frontend/src/lib/stores/twoFactorCode.svelte.ts b/frontend/src/lib/stores/twoFactorCode.svelte.ts index 111d7d9..5b00401 100644 --- a/frontend/src/lib/stores/twoFactorCode.svelte.ts +++ b/frontend/src/lib/stores/twoFactorCode.svelte.ts @@ -24,10 +24,18 @@ import { requestNewTwoFactorCode } from '$lib/square/square'; * card is selected, or a new card is being saved for reuse. * The surface passes its exact gate expression so each * surface's gate semantics are preserved verbatim. + * - `mint()` — optional; the code-request call. Customer surfaces omit + * it (defaults to the session-scoped /api/user/2fa/code: + * session user == card owner). Admin surfaces MUST pass + * () => adminRequestNewTwoFactorCode(customerUserID) so the + * mint targets the CUSTOMER and the code is delivered to + * them — the admin's session never authenticates the + * customer's card. */ export function useTwoFactorCodeForSavedCard(options: { enabled: () => boolean; gateActive: () => boolean; + mint?: () => ReturnType; }) { // Kept populated across retries so an invalid/expired code can be corrected // without re-typing it. @@ -36,8 +44,7 @@ export function useTwoFactorCodeForSavedCard(options: { // CARD OWNER, so even a session user whose own flag is unset must be able // to enter the code. Revealing the input makes the failure recoverable. let reveal = $state(false); - // POST /api/user/2fa/code mint state for the "Request a new code" button - // (session user = card owner, so a minted code authorizes their charge). + // Code-request state for the "Request a new code" button. let requesting = $state(false); // Show the code input whenever the pending charge hits the backend's 2FA @@ -49,7 +56,8 @@ export function useTwoFactorCodeForSavedCard(options: { if (requesting) return; requesting = true; try { - const result = await requestNewTwoFactorCode(); + const mint = options.mint ?? requestNewTwoFactorCode; + const result = await mint(); if (result.ok) { code = ''; toast.success(result.message);