From ecdd1bd493ecce49252c96c4dc5098d8c8170a9c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 8 Jun 2026 13:18:58 -0700 Subject: [PATCH] fix(ci): repair pre-existing base-branch test failures (closes #993) Two tests fail on a pristine checkout of feat/multicloud-web-frontend, turning base CI red and marking every open PR UNSTABLE: 1. internal/server TestAuthServiceAdapter_ValidateCSRFToken The mock service was built via NewService without a CSRFKey, so it generated a random ephemeral key each run. The test then asserted a literal "csrf-token" validated, which the HMAC derivation can never match. Make the test hermetic: pin a deterministic key with the new auth.TestCSRFKey() and assert the real derived token via auth.DeriveTestCSRFToken(). The CSRF validation logic is untouched. 2. frontend utils.test.ts (formatDate / getDateParts) A date-only string ("2024-03-15") was parsed as UTC midnight, so toLocaleDateString / getDate() shifted it to the previous day in any browser west of UTC. This is a real off-by-one for users (purchase scheduled_date, reservation end_date) and made the test pass only in UTC. Fix at root: parse bare "YYYY-MM-DD" as local midnight via a new parseDateInput helper (with round-trip validation so out-of-range digits like "2024-13-45" still reject). Full timestamps and Date objects are left untouched. Add timezone-invariance regression tests. Issue #993 (TestLogin_WithMFA_NoSecret) no longer reproduces on the current base: service.go already routes every login-failure path through the single genericLoginError constant, so unknown-email and wrong-password return a byte-identical message (no enumeration oracle). Closing it here with the other base-CI repairs. --- frontend/src/__tests__/utils.test.ts | 25 +++++++++++++++-- frontend/src/utils.ts | 41 ++++++++++++++++++++++++++-- internal/auth/test_helpers.go | 19 +++++++++++++ internal/server/adapter_test.go | 13 ++++++++- 4 files changed, 93 insertions(+), 5 deletions(-) diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts index ed58dec6f..14d025d61 100644 --- a/frontend/src/__tests__/utils.test.ts +++ b/frontend/src/__tests__/utils.test.ts @@ -58,8 +58,10 @@ describe('formatCurrency', () => { describe('formatDate', () => { test('renders canonical "Mon DD, YYYY" form regardless of browser locale', () => { - // 2024-03-15 → "Mar 15, 2024" in en-US short-month format. The check - // is locale-invariant because formatDate forces en-US. + // 2024-03-15 → "Mar 15, 2024" in en-US short-month format. The check is + // locale-invariant (formatDate forces en-US) and timezone-invariant: a + // bare "YYYY-MM-DD" is parsed as local midnight, so the displayed day + // stays 15 even in browsers west of UTC (previously rendered "Mar 14"). expect(formatDate('2024-03-15')).toBe('Mar 15, 2024'); }); @@ -75,8 +77,18 @@ describe('formatDate', () => { test('returns empty string for invalid date', () => { expect(formatDate('not-a-date')).toBe(''); + // Digit-shaped but out-of-range: must not silently roll over (month 13, + // day 45) into a valid date — parseDateInput round-trips and rejects it. expect(formatDate('2024-13-45')).toBe(''); }); + + test('does not shift a bare calendar day across the day boundary', () => { + // A date-only string is the intended calendar day; it must render the + // same day regardless of the runner's timezone (regression for the + // UTC-midnight off-by-one that showed "Mar 14" west of UTC). + expect(formatDate('2024-01-01')).toBe('Jan 1, 2024'); + expect(formatDate('2024-12-31')).toBe('Dec 31, 2024'); + }); }); describe('formatDateTime', () => { @@ -135,6 +147,15 @@ describe('getDateParts', () => { test('returns zeros for invalid date', () => { expect(getDateParts('invalid')).toEqual({ day: 0, month: '' }); + // Out-of-range digits must be rejected, not rolled over. + expect(getDateParts('2024-13-45')).toEqual({ day: 0, month: '' }); + }); + + test('keeps a bare calendar day stable across timezones', () => { + // Regression: a date-only string parsed as UTC midnight previously + // reported the previous day's date west of UTC. + expect(getDateParts('2024-03-15').day).toBe(15); + expect(getDateParts('2024-01-01').day).toBe(1); }); }); diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index 4870da2bf..ecc27c164 100644 --- a/frontend/src/utils.ts +++ b/frontend/src/utils.ts @@ -38,6 +38,43 @@ export function formatCurrency( })}`; } +/** + * Parse a date input into a Date, treating bare calendar dates correctly. + * + * A date-only ISO string ("2024-03-15") is parsed by `new Date()` as UTC + * midnight; rendering it with a locale formatter then shifts it into the + * browser's local timezone, so a user west of UTC sees the *previous* day + * ("Mar 14") for a date the backend meant as a plain calendar day (e.g. a + * purchase's scheduled_date or a reservation end_date). Parse bare + * "YYYY-MM-DD" values as local midnight instead so the displayed day matches + * the intended calendar day in every timezone. Strings carrying a time/zone + * component ("2024-03-15T14:30:00Z") and Date objects are left untouched. + */ +function parseDateInput(date: string | Date): Date { + if (typeof date === 'string') { + const m = /^(\d{4})-(\d{2})-(\d{2})$/.exec(date); + if (m) { + const year = Number(m[1]); + const month = Number(m[2]); // 1-based as written + const day = Number(m[3]); + // Local midnight for the given calendar day (Date month is 0-based). + const d = new Date(year, month - 1, day); + // Reject out-of-range components (e.g. "2024-13-45"): the Date + // constructor silently rolls them over instead of producing NaN, so + // verify the round-trip and return an Invalid Date if it doesn't match. + if ( + d.getFullYear() !== year || + d.getMonth() !== month - 1 || + d.getDate() !== day + ) { + return new Date(NaN); + } + return d; + } + } + return new Date(date); +} + /** * Canonical date-only format used everywhere in the UI: "Apr 17, 2026". * The en-US locale + `month: 'short'` removes ambiguity from pure numeric @@ -46,7 +83,7 @@ export function formatCurrency( */ export function formatDate(date: string | Date | null | undefined): string { if (!date) return ''; - const d = new Date(date); + const d = parseDateInput(date); if (isNaN(d.getTime())) return ''; return d.toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' }); } @@ -111,7 +148,7 @@ export interface DateParts { */ export function getDateParts(date: string | Date | null | undefined): DateParts { if (!date) return { day: 0, month: '' }; - const d = new Date(date); + const d = parseDateInput(date); if (isNaN(d.getTime())) return { day: 0, month: '' }; return { day: d.getDate(), diff --git a/internal/auth/test_helpers.go b/internal/auth/test_helpers.go index b1f42357c..974d66c04 100644 --- a/internal/auth/test_helpers.go +++ b/internal/auth/test_helpers.go @@ -217,6 +217,25 @@ var _ EmailSenderInterface = (*MockEmailSender)(nil) // calling a real key-generation path. var testCSRFKey = []byte("test-csrf-key-32-bytes-padded---") +// TestCSRFKey returns the fixed 32-byte CSRF key shared by test services. +// Pass it as ServiceConfig.CSRFKey so a service built via NewService derives +// CSRF tokens deterministically (no ephemeral random key), letting tests in +// other packages reproduce the expected token with DeriveTestCSRFToken. +func TestCSRFKey() []byte { + // Return a copy so callers cannot mutate the package-level key. + key := make([]byte, len(testCSRFKey)) + copy(key, testCSRFKey) + return key +} + +// DeriveTestCSRFToken returns the CSRF token a service configured with +// TestCSRFKey expects for the given raw session token. It reuses the +// production derivation (HMAC-SHA256(key, rawSessionToken)) so tests assert +// the real contract rather than duplicating the crypto. +func DeriveTestCSRFToken(rawSessionToken string) string { + return deriveCSRFToken(testCSRFKey, rawSessionToken) +} + // newTestService returns a minimal Service with fast bcrypt for unit tests. // It has no store or email sender — use createTestService() for tests that need mocks. func newTestService() *Service { diff --git a/internal/server/adapter_test.go b/internal/server/adapter_test.go index b9d0933c5..1e8b54dbb 100644 --- a/internal/server/adapter_test.go +++ b/internal/server/adapter_test.go @@ -18,6 +18,11 @@ func createMockAuthService(t *testing.T) (*authServiceAdapter, *auth.MockStore) service := auth.NewService(auth.ServiceConfig{ Store: mockStore, SessionDuration: time.Hour, + // Pin a deterministic CSRF key so CSRF-token derivation is + // reproducible and hermetic. Without this, NewService generates a + // random ephemeral key, so any test asserting a specific derived CSRF + // token (TestAuthServiceAdapter_ValidateCSRFToken) cannot reproduce it. + CSRFKey: auth.TestCSRFKey(), }) adapter := newAuthServiceAdapter(service) return adapter, mockStore @@ -238,7 +243,13 @@ func TestAuthServiceAdapter_ValidateCSRFToken(t *testing.T) { ExpiresAt: time.Now().Add(time.Hour), }, nil) - err := adapter.ValidateCSRFToken(ctx, "session-token", "csrf-token") + // ValidateCSRFToken recomputes the expected token as + // HMAC-SHA256(csrfKey, rawSessionToken); it never reads the stored + // CSRFToken column. Derive the matching token from the same fixed key the + // service was built with (auth.TestCSRFKey) so the assertion is hermetic + // and does not depend on an ambient CSRFKey from the environment. + csrfToken := auth.DeriveTestCSRFToken("session-token") + err := adapter.ValidateCSRFToken(ctx, "session-token", csrfToken) require.NoError(t, err) }