popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: dup/modularisation findings — account gift-card 2FA gate, single-source 2FA predicate, login store delegation

Browse Source 
Restart-loop dup/mod review findings:
- Account page gift-card buy flow now passes the 2FA verification-code gate end-to-end: TwoFactorCodeInput + Request-a-new-code wired for saved-card/save-card charges, verification_code in the /api/user/giftcards/buy body, 403/429 gate-failure self-heal, buy button gated on missing code (backend BuyGiftCard gate at giftcards.go:1471 already required it — the frontend never sent it)
- Removed dead requires2FACodeForSavedCard export from square.ts (zero consumers; all surfaces use authStore.savedCardChargeRequires2FACode) + its test; auth store getter documented as THE single source of truth
- Login page now delegates token persistence to authStore.setToken instead of direct localStorage writes (drift-risk closed; setToken persists both tokens identically so the full-reload init still works)
- Pence comment corrected (GBP minor unit)
- account/+page.svelte:84+6; square.ts -16; square.test.ts -26; auth.svelte.ts comment; login/+page.svelte delegation

Frontend 72/72 tests + build clean; backend builds.
This commit is contained in:
popertots
2026-08-22 00:34:50 +01:00
parent 4d5d2cd381
commit a8bf24ee23
5 changed files with 95 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/square/square.test.ts
-26
View File
@@ -11,7 +11,6 @@ import {
isSavedCardVerificationRequired,
isTwoFactorVerificationGateFailure,
requestNewTwoFactorCode,
requires2FACodeForSavedCard,
sanitizeDecimalInput,
submitPaymentWithRetry
} from './square';
@@ -220,31 +219,6 @@ describe('payment failure classification', () => {
});
});
describe('requires2FACodeForSavedCard', () => {
it('is true when the session user has twoFactorRequired set', () => {
expect(requires2FACodeForSavedCard({ twoFactorRequired: true })).toBe(true);
});
it('is false when twoFactorRequired is unset or false', () => {
expect(requires2FACodeForSavedCard({ twoFactorRequired: false })).toBe(false);
expect(requires2FACodeForSavedCard({})).toBe(false);
});
it('is false for a null/undefined session user', () => {
expect(requires2FACodeForSavedCard(null)).toBe(false);
expect(requires2FACodeForSavedCard(undefined)).toBe(false);
});
it('does not depend on the 2FA setup flag (B6/B10: the code is required per charge)', () => {
expect(requires2FACodeForSavedCard({ twoFactorRequired: true, twoFactorEnabled: true })).toBe(
true
);
expect(requires2FACodeForSavedCard({ twoFactorRequired: true, twoFactorEnabled: false })).toBe(
true
);
});
});
describe('isTwoFactorVerificationGateFailure', () => {
it.each([
[403, 'A two-factor verification code is required to use this saved card', true],
frontend/src/lib/square/square.ts
-16
View File
@@ -118,22 +118,6 @@ export function isSavedCardVerificationRequired(status: number, usedSavedCard: b
export const SAVED_CARD_VERIFICATION_MESSAGE =
'Your card issuer requires verification. Please pay with a new card or re-add your card.';
/**
* B6/B10: whether charging a saved card requires the customer's current 2FA
* verification code. True when the session user has `twoFactorRequired` set —
* the profile exposes the backend's `twoFactorEnforced()` posture, so this is
* true for every session user in an enforced environment. Charge surfaces show
* the verification-code input whenever this is set; the backend additionally
* 403s saved-card charges when the code is missing, so surfaces must also
* reveal the input on that error. Single source of truth so the booking,
* account, tip and admin surfaces can't drift on the gate condition.
*/
export function requires2FACodeForSavedCard(
sessionUser: { twoFactorRequired?: boolean } | null | undefined
): boolean {
return !!sessionUser?.twoFactorRequired;
}
/**
* True when a saved-card charge error is a 2FA verification-gate rejection
* (backend/handlers/payments/twofa.go): 403 when no code was supplied or the
frontend/src/lib/stores/auth.svelte.ts
+4 -1
View File
@@ -78,7 +78,10 @@ class AuthStore {
// yet enabled). The backend keys on the CARD OWNER — which the frontend
// usually cannot know — so a 403 for a missing code must also surface the
// input. Single source of truth so the predicate can't drift between the
// booking, account, tip and admin payment surfaces.
// booking, account, tip and admin payment surfaces. This getter is THE
// single source of truth for the 2FA gate — the former square.ts helper
// (requires2FACodeForSavedCard) was dead code and has been removed, so any
// surface gating saved-card charges must read this getter.
get savedCardChargeRequires2FACode() {
return !!this.user?.twoFactorRequired;
}
frontend/src/routes/account/+page.svelte
+84 -6
View File
@@ -8,11 +8,14 @@
import CardEntryUnavailable from '$lib/components/payments/CardEntryUnavailable.svelte';
import CardSelection from '$lib/components/payments/CardSelection.svelte';
import SquareCardInput from '$lib/components/payments/SquareCardInput.svelte';
import TwoFactorCodeInput from '$lib/components/payments/TwoFactorCodeInput.svelte';
import {
canSaveCardsForRole,
isNonceStale,
isSavedCardVerificationRequired,
isSquareConfigured,
isTwoFactorVerificationGateFailure,
requestNewTwoFactorCode,
SAVED_CARD_VERIFICATION_MESSAGE,
submitPaymentWithRetry
} from '$lib/square/square';
@@ -246,6 +249,53 @@
// with the wrong intent.
let buyTokenizedForSaveCard = $state(false);
// B6/B10: gift-card buys charge a saved card (or save a new card for reuse)
// whenever the backend enforces the 2FA gate — the CARD OWNER's current
// verification code must be carried on the charge. Mirrors the customer
// surface (UserPaymentModal). Kept populated across retries so an
// invalid/expired code can be corrected without re-typing it.
let buyTwoFactorCode = $state('');
// Set true when a charge 403s for a missing code — reveals the input even
// if the session profile's 2FA flag is stale, making the failure
// recoverable.
let buyReveal2FACodeInput = $state(false);
const buyTwoFactorEnabled = $derived(!!authStore.currentUser?.twoFactorEnabled);
const buySavedCardChargeRequires2FACode = $derived(authStore.savedCardChargeRequires2FACode);
// Show the code input whenever the pending charge hits the backend's 2FA
// gate: charging a saved card OR saving the new card for reuse.
const buyShow2FACodeInput = $derived(
buyReveal2FACodeInput ||
(buySavedCardChargeRequires2FACode && (buySelectedCard !== '' || buySaveCard))
);
const buyMissing2FACode = $derived(
buyShow2FACodeInput && buyTwoFactorEnabled && buyTwoFactorCode.trim() === ''
);
// 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).
let buyRequesting2FACode = $state(false);
async function handleBuyRequestNew2FACode() {
if (buyRequesting2FACode) return;
buyRequesting2FACode = true;
try {
const result = await requestNewTwoFactorCode();
if (result.ok) {
buyTwoFactorCode = '';
toast.success(result.message);
} else if (result.status === 429) {
toast.error(result.message || 'Too many requests. Wait before requesting a new code.');
} else if (result.status === 503) {
toast.error(
result.message || 'Verification codes are unavailable right now. Try again later.'
);
} else {
toast.error(result.message);
}
} finally {
buyRequesting2FACode = false;
}
}
// Client-side mirror of the £500/day online purchase cap. The backend is
// authoritative — this counter only reflects confirmed purchases made in
// this session, so a user is told they've hit the cap instead of being
@@ -456,12 +506,13 @@
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
amount: buyAmount * 100, // cents
amount: buyAmount * 100, // pence
recipient_type: buyRecipientType,
recipient_email: buyRecipientEmail,
...(cardId ? { card_id: cardId } : {}),
...(newCardToken ? { new_card_token: newCardToken, save_card: buySaveCard } : {}),
...(verificationToken ? { verification_token: verificationToken } : {}),
...(buyShow2FACodeInput ? { verification_code: buyTwoFactorCode } : {}),
idempotency_key: buyIdempotencyKey
})
})
@@ -482,6 +533,8 @@
buyTokenAmount = 0;
buyTokenizedAt = 0;
buyTokenizedForSaveCard = false;
buyTwoFactorCode = '';
buyReveal2FACodeInput = false;
await fetchGiftCardBalance();
} else {
// Capture the status BEFORE consuming the body — the saved-card
@@ -493,11 +546,16 @@
// requires verification — surface the fix instead of the generic
// backend text.
const verificationRequired = isSavedCardVerificationRequired(status, !!buySelectedCard);
toast.error(
verificationRequired
? SAVED_CARD_VERIFICATION_MESSAGE
: extractErrorMessage(errText) || 'Failed to purchase gift card'
);
// B6/B10: a 2FA verification-gate rejection (missing/invalid/expired
// code, brute-force lockout) is recoverable — keep the code populated
// and reveal the input so the charge can be retried with a fresh code.
const buyErrMsg = verificationRequired
? SAVED_CARD_VERIFICATION_MESSAGE
: extractErrorMessage(errText) || 'Failed to purchase gift card';
if (isTwoFactorVerificationGateFailure(status, buyErrMsg)) {
buyReveal2FACodeInput = true;
}
toast.error(buyErrMsg);
// A definitive charge failure (e.g. declined card) consumes the
// nonce + SCA verification token — clear the cached pair so the
// next retry re-tokenizes fresh. The idempotency key stays for
@@ -2572,12 +2630,32 @@
bind:saveCard={buySaveCard}
onValidityChange={(v) => (buyCardSelectionValid = v)}
/>
<!-- B6/B10: saved-card gift-card charges require the card owner's
current 2FA verification code when the backend enforces the gate. -->
<TwoFactorCodeInput
bind:code={buyTwoFactorCode}
showInput={buyShow2FACodeInput}
enabled={buyTwoFactorEnabled}
/>
{#if buyShow2FACodeInput && buyTwoFactorEnabled}
<Button
variant="outline"
size="sm"
class="w-full"
loading={buyRequesting2FACode}
disabled={buyRequesting2FACode}
onclick={handleBuyRequestNew2FACode}
>
Request a new code
</Button>
{/if}
</div>
<Button
onclick={buyGiftCard}
disabled={buyingGiftCard ||
!isBuyCardValid ||
buyMissing2FACode ||
buyDailyTotal + buyAmount > DAILY_GIFT_CARD_BUY_LIMIT}
class="mt-2 w-full"
>
frontend/src/routes/login/+page.svelte
+7 -10
View File
@@ -15,6 +15,7 @@
import * as languageEn from '@zxcvbn-ts/language-en';
import { toast } from 'svelte-sonner';
import { extractErrorMessage, sanitizeText } from '$lib/utils/toast-safe';
import { authStore } from '$lib/stores/auth.svelte';
// set up options so that feedback, dictionary etc. are included
const zxcvbn = new ZxcvbnFactory({
@@ -164,16 +165,12 @@
if (response.ok) {
const data = await response.json();
localStorage.setItem('authToken', data.token);
// B5: the auth store needs the opaque refresh token so it can
// rotate on subsequent refreshes. The `window.location.href`
// below does a full reload, which re-runs initializeAuth() and
// reads it back from localStorage.
const refreshToken = data.refreshToken ?? data.refresh_token;
if (refreshToken) {
localStorage.setItem('authRefreshToken', refreshToken);
}
// Persist the login pair through the auth store — the single
// source of truth for where/how tokens are stored. setToken
// writes BOTH authToken and authRefreshToken to localStorage
// (refresh token only when present), so the full reload below
// still re-runs initializeAuth() and reads them back.
authStore.setToken(data.token, data.refreshToken ?? data.refresh_token ?? null);
// Decode token to check role for redirect
let redirectTo = '/';