From 418e8509e123dbc600dc07638a5be04eb3cf8f9e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 20 May 2026 18:48:47 +0200 Subject: [PATCH 1/2] test(frontend): pin allowed_accounts UI enforcement (closes #313) Add allowed-accounts.test.ts covering the three frontend surfaces that rely on the backend's allowed_accounts filtering: 1. Account chip: when listAccounts returns a filtered subset the chip option list contains exactly those accounts plus the All Accounts sentinel. Disallowed accounts never appear. Zero-account case yields only the sentinel. 2. History list: the UI renders exactly the rows returned by getHistory with no additional client-side account filtering. Empty response yields zero rendered rows. 3. 403 on Cancel: when cancelPurchase rejects with a 403 (disallowed account accessed via deep link or stale tab) the handler surfaces a user-friendly error toast instead of an unhandled exception. --- .../src/__tests__/allowed-accounts.test.ts | 286 ++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 frontend/src/__tests__/allowed-accounts.test.ts diff --git a/frontend/src/__tests__/allowed-accounts.test.ts b/frontend/src/__tests__/allowed-accounts.test.ts new file mode 100644 index 000000000..3f533a0b1 --- /dev/null +++ b/frontend/src/__tests__/allowed-accounts.test.ts @@ -0,0 +1,286 @@ +/** + * allowed_accounts UI enforcement tests (issue #313). + * + * The backend enforces `allowed_accounts` on every API endpoint; these + * tests pin the UI surfaces that rely on that filtering: + * + * 1. Account chip in the topbar populates exclusively from the + * `listAccounts` response. When the backend returns a filtered + * subset (reflecting the user's `allowed_accounts`), only those + * accounts appear — disallowed accounts are never shown. + * + * 2. History list renders exactly the rows the API returns. The + * frontend does no client-side account filtering of its own; + * dropping that responsibility on the backend prevents the flicker + * scenario where disallowed rows briefly render before being hidden. + * + * 3. A 403 response on a mutate-by-id path (Cancel purchase) surfaces + * a user-friendly error toast instead of an unhandled exception. + * + * 4. When `listAccounts` returns an empty list (zero allowed accounts), + * the account chip collapses to the "All Accounts" sentinel only — + * no stale option from a previous session bleeds through. + * + * Related: backend enforcement tests in #307. + */ + +// --------------------------------------------------------------------------- +// Shared mocks (must be declared before any imports) +// --------------------------------------------------------------------------- + +jest.mock('../api', () => ({ + listAccounts: jest.fn(), + getHistory: jest.fn(), + cancelPurchase: jest.fn(), +})); + +jest.mock('../toast', () => ({ + showToast: jest.fn(), +})); + +jest.mock('../confirmDialog', () => ({ + confirmDialog: jest.fn(), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), +})); + +jest.mock('../utils', () => ({ + formatCurrency: jest.fn((val: number) => `$${val || 0}`), + formatDate: jest.fn((val: string) => (val ? new Date(val).toLocaleDateString() : '')), + formatTerm: jest.fn((years: number) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)), + escapeHtml: jest.fn((str: string) => str || ''), + populateAccountFilter: jest.fn(() => Promise.resolve()), +})); + +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(), + getCurrentProvider: jest.fn().mockReturnValue(''), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), +})); + +// --------------------------------------------------------------------------- +// Imports (after mocks) +// --------------------------------------------------------------------------- + +import { initTopbarFilters } from '../topbar-filters'; +import { loadHistory } from '../history'; +import * as api from '../api'; +import * as state from '../state'; +import { showToast } from '../toast'; +import { confirmDialog } from '../confirmDialog'; +import { getCurrentUser } from '../state'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const ADMIN = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' }; + +function setupTopbarSlot(): void { + while (document.body.firstChild) document.body.removeChild(document.body.firstChild); + const slot = document.createElement('div'); + slot.id = 'topbar-filters'; + document.body.appendChild(slot); +} + +function setupHistoryDOM(): void { + while (document.body.firstChild) document.body.removeChild(document.body.firstChild); + const mkInput = (id: string): HTMLInputElement => { + const el = document.createElement('input'); + el.type = 'date'; + el.id = id; + return el; + }; + const mkDiv = (id: string): HTMLDivElement => { + const el = document.createElement('div'); + el.id = id; + return el; + }; + document.body.appendChild(mkInput('history-start')); + document.body.appendChild(mkInput('history-end')); + document.body.appendChild(mkDiv('history-summary')); + document.body.appendChild(mkDiv('history-list')); + document.body.appendChild(mkDiv('purchases-approval-queue')); +} + +function makeHistoryRow(overrides: Record = {}) { + return { + purchase_id: 'exec-1', + timestamp: '2024-03-01T00:00:00Z', + provider: 'aws', + service: 'ec2', + resource_type: 't3.medium', + region: 'us-east-1', + account_id: 'allowed-1', + cloud_account_id: 'allowed-1', + count: 1, + term: 1, + upfront_cost: 200, + estimated_savings: 80, + plan_name: '', + status: 'pending', + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// 1. Account chip - allowed_accounts enforcement +// --------------------------------------------------------------------------- + +describe('Account chip - allowed_accounts enforcement (issue #313)', () => { + beforeEach(() => { + jest.clearAllMocks(); + setupTopbarSlot(); + state.setCurrentProvider(''); + state.setCurrentAccountIDs([]); + }); + + afterEach(() => { + state.setCurrentProvider(''); + state.setCurrentAccountIDs([]); + }); + + test('chip lists only the accounts returned by listAccounts (backend-filtered subset)', async () => { + // Simulate backend returning only the two accounts the user is allowed + // to access (the rest are filtered server-side by allowed_accounts). + (api.listAccounts as jest.Mock).mockResolvedValue([ + { id: 'allowed-1', name: 'Prod AWS', external_id: '111111111111' }, + { id: 'allowed-2', name: 'Staging AWS', external_id: '222222222222' }, + ]); + + initTopbarFilters(); + // Drain the async populateAccountOptions call. + await new Promise((r) => setTimeout(r, 0)); + + // Open the account chip (second .chip-select trigger). + const triggers = document.querySelectorAll('.chip-select'); + const accountTrigger = triggers[1] as HTMLButtonElement; + accountTrigger.click(); + + const options = Array.from( + document.querySelectorAll('.chip-select-option'), + ).map((el) => el.dataset['value']); + + // "All Accounts" sentinel + exactly the two allowed accounts. + expect(options).toEqual(['', 'allowed-1', 'allowed-2']); + }); + + test('chip does not contain accounts absent from the listAccounts response', async () => { + (api.listAccounts as jest.Mock).mockResolvedValue([ + { id: 'allowed-only', name: 'Allowed Prod', external_id: '999999999999' }, + ]); + + initTopbarFilters(); + await new Promise((r) => setTimeout(r, 0)); + + const triggers = document.querySelectorAll('.chip-select'); + const accountTrigger = triggers[1] as HTMLButtonElement; + accountTrigger.click(); + + const optionValues = Array.from( + document.querySelectorAll('.chip-select-option'), + ).map((el) => el.dataset['value']); + + expect(optionValues).not.toContain('disallowed-acct'); + expect(optionValues).toContain('allowed-only'); + }); + + test('chip shows only All Accounts when listAccounts returns empty list (zero allowed accounts)', async () => { + (api.listAccounts as jest.Mock).mockResolvedValue([]); + + initTopbarFilters(); + await new Promise((r) => setTimeout(r, 0)); + + const triggers = document.querySelectorAll('.chip-select'); + const accountTrigger = triggers[1] as HTMLButtonElement; + accountTrigger.click(); + + const options = Array.from( + document.querySelectorAll('.chip-select-option'), + ).map((el) => el.dataset['value']); + + // Only the sentinel option — no stale accounts from a prior session. + expect(options).toEqual(['']); + }); +}); + +// --------------------------------------------------------------------------- +// 2 + 3. History list and 403 on Cancel +// --------------------------------------------------------------------------- + +describe('History list - allowed_accounts enforcement (issue #313)', () => { + beforeEach(() => { + setupHistoryDOM(); + jest.clearAllMocks(); + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN); + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.listAccounts as jest.Mock).mockResolvedValue([]); + }); + + test('renders exactly the rows returned by getHistory (no extra client-side rows)', async () => { + // Simulate backend returning only two allowed-account rows. + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [ + makeHistoryRow({ purchase_id: 'exec-allowed-1', account_id: 'allowed-1' }), + makeHistoryRow({ purchase_id: 'exec-allowed-2', account_id: 'allowed-1' }), + ], + }); + + await loadHistory(); + + const list = document.getElementById('history-list')!; + // history.ts renders rows as (see history.ts renderHistoryList). + const rows = list.querySelectorAll('tr[data-execution-id]'); + // Two rows — exactly what the API returned. + expect(rows.length).toBe(2); + }); + + test('renders empty list when getHistory returns zero rows (all accounts disallowed)', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [], + }); + + await loadHistory(); + + const list = document.getElementById('history-list')!; + // No purchase rows rendered. + const rows = list.querySelectorAll('tr[data-execution-id]'); + expect(rows.length).toBe(0); + }); + + test('403 on Cancel surfaces a user-friendly error toast, not an unhandled exception', async () => { + // Silence the expected console.error from history.ts's catch block. + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeHistoryRow({ purchase_id: 'exec-1', created_by_user_id: ADMIN.id })], + }); + + // Simulate the backend returning 403 (disallowed account). + const forbidden = new Error('Forbidden') as Error & { status?: number }; + forbidden.status = 403; + (api.cancelPurchase as jest.Mock).mockRejectedValue(forbidden); + + await loadHistory(); + + const btn = document.querySelector('.history-cancel-btn'); + btn?.click(); + await new Promise((r) => setTimeout(r, 10)); + + // A toast with kind:'error' must surface — not a blank crash. + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'error' }), + ); + + spy.mockRestore(); + }); +}); From 57bd5ddba730ee92f6333d652ec0219e6a54688d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 27 May 2026 23:59:13 +0200 Subject: [PATCH 2/2] test(frontend/allowed-accounts): tighten async wait + correct comment per CR Replace the arbitrary 10ms setTimeout with waitFor from @testing-library/dom so the 403-cancel toast assertion waits deterministically for the mock to be called. Also correct the comment from "history.ts's catch block" to "cancel button handler's catch block" which accurately identifies the code path. --- frontend/src/__tests__/allowed-accounts.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/frontend/src/__tests__/allowed-accounts.test.ts b/frontend/src/__tests__/allowed-accounts.test.ts index 3f533a0b1..03a9fb022 100644 --- a/frontend/src/__tests__/allowed-accounts.test.ts +++ b/frontend/src/__tests__/allowed-accounts.test.ts @@ -68,6 +68,7 @@ jest.mock('../state', () => ({ // Imports (after mocks) // --------------------------------------------------------------------------- +import { waitFor } from '@testing-library/dom'; import { initTopbarFilters } from '../topbar-filters'; import { loadHistory } from '../history'; import * as api from '../api'; @@ -257,7 +258,7 @@ describe('History list - allowed_accounts enforcement (issue #313)', () => { }); test('403 on Cancel surfaces a user-friendly error toast, not an unhandled exception', async () => { - // Silence the expected console.error from history.ts's catch block. + // Silence the expected console.error from the cancel button handler's catch block. const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); (api.getHistory as jest.Mock).mockResolvedValue({ @@ -274,12 +275,13 @@ describe('History list - allowed_accounts enforcement (issue #313)', () => { const btn = document.querySelector('.history-cancel-btn'); btn?.click(); - await new Promise((r) => setTimeout(r, 10)); // A toast with kind:'error' must surface — not a blank crash. - expect(showToast).toHaveBeenCalledWith( - expect.objectContaining({ kind: 'error' }), - ); + await waitFor(() => { + expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'error' }), + ); + }); spy.mockRestore(); });