fix: admin-scoped 2FA mint targets the CUSTOMER — user authentication for saved cards, never the admin
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.
This commit is contained in:
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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.
|
||||
//
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user