diff --git a/frontend/src/__tests__/apikeys.test.ts b/frontend/src/__tests__/apikeys.test.ts index 771768ec2..ecf634f16 100644 --- a/frontend/src/__tests__/apikeys.test.ts +++ b/frontend/src/__tests__/apikeys.test.ts @@ -602,6 +602,10 @@ describe('API Keys Module', () => { }); describe('handleCreateApiKey', () => { + beforeEach(() => { + document.getElementById('create-apikey-modal')?.classList.remove('hidden'); + }); + test('prevents default form submission', async () => { (api.createApiKey as jest.Mock).mockResolvedValue({ api_key: 'test-key', @@ -940,6 +944,17 @@ describe('API Keys Module', () => { expect(api.getApiKeys).toHaveBeenCalledTimes(2); // Initial load + after revoke }); + test('toasts a failure while the hidden create modal still has its error element (issue #717)', async () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + (api.revokeApiKey as jest.Mock).mockRejectedValue(new Error('Revoke failed')); + + await revokeApiKey('key-1'); + + expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key'); + expect(document.getElementById('create-apikey-error')?.classList.contains('hidden')).toBe(true); + consoleError.mockRestore(); + }); + test('shows error on API failure', async () => { // Remove the create-apikey-error element so showError falls back to alert document.getElementById('create-apikey-error')?.remove(); @@ -1079,8 +1094,41 @@ describe('API Keys Module', () => { }); }); + describe('secret handling (issue #732)', () => { + test('a second submit while the first is in flight mints only one key and disables submit', async () => { + document.getElementById('create-apikey-modal')?.classList.remove('hidden'); + (document.getElementById('apikey-name') as HTMLInputElement).value = 'Test Key'; + (document.getElementById('apikey-password') as HTMLInputElement).value = 'pw'; + (document.getElementById('apikey-expires-at') as HTMLInputElement).value = '2999-01-01'; + let resolve!: (v: unknown) => void; + (api.createApiKey as jest.Mock).mockReturnValue(new Promise((r) => { resolve = r; })); + (api.getApiKeys as jest.Mock).mockResolvedValue({ api_keys: [] }); + const event = { preventDefault: jest.fn() } as unknown as Event; + + const first = handleCreateApiKey(event); + await handleCreateApiKey(event); + expect((document.getElementById('create-apikey-submit-btn') as HTMLButtonElement).disabled).toBe(true); + resolve({ api_key: 'k', key_id: 'key-1', key: { id: 'key-1', name: 'Test Key', key_prefix: 'abc' } }); + await first; + + expect(api.createApiKey).toHaveBeenCalledTimes(1); + }); + + test('Escape does not dismiss the created-key modal; the button does', () => { + showKeyCreatedModal('secret-key'); + const modal = document.getElementById('apikey-created-modal')!; + modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + expect(document.getElementById('apikey-created-modal')).not.toBeNull(); + expect(modal.classList.contains('hidden')).toBe(false); + + document.getElementById('close-apikey-created-btn')!.click(); + expect(document.getElementById('apikey-created-modal')).toBeNull(); + }); + }); + describe('Error display', () => { test('shows error in error element when available', async () => { + document.getElementById('create-apikey-modal')?.classList.remove('hidden'); const event = { preventDefault: jest.fn() } as unknown as Event; await handleCreateApiKey(event); diff --git a/frontend/src/__tests__/auth-mfa-enroll.test.ts b/frontend/src/__tests__/auth-mfa-enroll.test.ts index 7213d8630..d12b50d6f 100644 --- a/frontend/src/__tests__/auth-mfa-enroll.test.ts +++ b/frontend/src/__tests__/auth-mfa-enroll.test.ts @@ -49,6 +49,7 @@ import * as state from '../state'; // trigger it indirectly by clicking the user-email element after // updateUserUI mounts it. import { updateUserUI } from '../auth'; +import { closeModal } from '../modal'; beforeEach(() => { document.body.innerHTML = ` @@ -200,4 +201,45 @@ describe('MFA regenerate-recovery-codes flow', () => { expect(api.regenerateMFARecoveryCodes).toHaveBeenCalledWith('123456'); expect(document.body.textContent).toContain('NEW1-XXXX'); }); + + test('Escape and Cancel cannot discard recovery codes until acknowledged (issue #732)', async () => { + (api.regenerateMFARecoveryCodes as jest.Mock).mockResolvedValue({ recovery_codes: ['NEW1-XXXX'] }); + await openProfile(); + document.getElementById('mfa-regenerate-btn')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + (document.getElementById('mfa-regen-code') as HTMLInputElement).value = '123456'; + document.getElementById('mfa-regen-submit')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + await new Promise((r) => setTimeout(r, 10)); + + const modal = document.getElementById('profile-modal')!; + modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + expect(modal.classList.contains('hidden')).toBe(false); + expect((document.getElementById('profile-cancel') as HTMLButtonElement).disabled).toBe(true); + + document.getElementById('mfa-codes-done')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + modal.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + expect(modal.classList.contains('hidden')).toBe(true); + }); + + test('Save Changes and Enter cannot close the profile modal while codes are pending (issue #732)', async () => { + (api.regenerateMFARecoveryCodes as jest.Mock).mockResolvedValue({ recovery_codes: ['NEW1-XXXX'] }); + await openProfile(); + document.getElementById('mfa-regenerate-btn')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + (document.getElementById('mfa-regen-code') as HTMLInputElement).value = '123456'; + document.getElementById('mfa-regen-submit')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + await new Promise((r) => setTimeout(r, 10)); + + const modal = document.getElementById('profile-modal')!; + const save = modal.querySelector('#profile-form button[type="submit"]')!; + expect(save.disabled).toBe(true); + document.getElementById('profile-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + await new Promise((r) => setTimeout(r, 10)); + expect(modal.classList.contains('hidden')).toBe(false); + closeModal(modal); + expect(modal.classList.contains('hidden')).toBe(false); + + document.getElementById('mfa-codes-done')!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(save.disabled).toBe(false); + closeModal(modal); + expect(modal.classList.contains('hidden')).toBe(true); + }); }); diff --git a/frontend/src/__tests__/auth.test.ts b/frontend/src/__tests__/auth.test.ts index cede48c71..4b2e4af67 100644 --- a/frontend/src/__tests__/auth.test.ts +++ b/frontend/src/__tests__/auth.test.ts @@ -287,6 +287,43 @@ describe('Auth Module', () => { expect(sendBtn).toBeTruthy(); }); + test('submitting the reset form (Enter) sends the reset, not a login error (issue #733)', async () => { + (api.requestPasswordReset as jest.Mock).mockResolvedValue({}); + await showLoginModal(); + document.getElementById('forgot-password-link') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + await new Promise(resolve => setTimeout(resolve, 10)); + + (document.getElementById('reset-email') as HTMLInputElement).value = 'test@example.com'; + document.getElementById('login-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(api.requestPasswordReset).toHaveBeenCalledWith('test@example.com'); + expect(api.login).not.toHaveBeenCalled(); + expect(document.body.textContent).not.toContain('Enter email and password'); + }); + + test('Enter in the reset form never runs the login handler (issue #733)', async () => { + await showLoginModal(); + document.getElementById('forgot-password-link') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + await new Promise(resolve => setTimeout(resolve, 10)); + + // Both handlers write synchronously and the reset handler writes last, + // so a stale login handler is only visible in the mutation records. + const writes: string[] = []; + const observer = new MutationObserver(records => { + for (const r of records) r.addedNodes.forEach(n => writes.push(n.textContent ?? '')); + }); + observer.observe(document.getElementById('login-modal')!, { childList: true, characterData: true, subtree: true }); + document.getElementById('login-form')!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true })); + await new Promise(resolve => setTimeout(resolve, 20)); + observer.disconnect(); + + expect(writes.some(text => /Enter email|Enter password/.test(text))).toBe(false); + expect(api.login).not.toHaveBeenCalled(); + }); + test('password reset swaps modal body to confirmation panel (issue #457)', async () => { (api.requestPasswordReset as jest.Mock).mockResolvedValue({}); await showLoginModal(); diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index ebf066a7f..7fe35322c 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -533,6 +533,83 @@ describe('Recommendations Module', () => { expect(cardValue(/Recommendations/)).toBe('1'); }); + test('row checkboxes carry distinct accessible names (issue #746)', async () => { + const recs = [ + { id: 'rec-a', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment: 'no-upfront', savings: 150, upfront_cost: 1000 }, + { id: 'rec-b', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'eu-west-1', count: 1, term: 3, payment: 'all-upfront', savings: 200, upfront_cost: 0 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 2, total_monthly_savings: 350, total_upfront_cost: 1000, avg_payback_months: 1 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + await loadRecommendations(); + + const label = (id: string) => + document.querySelector(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? ''; + expect(label('rec-a')).toContain('ec2'); + expect(label('rec-a')).toContain('t3.medium'); + expect(label('rec-a')).toContain('us-east-1'); + expect(label('rec-b')).toContain('rds'); + expect(label('rec-a')).not.toBe(label('rec-b')); + }); + + test('variants differing only by payment get distinct checkbox names (issue #746)', async () => { + const base = { provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 150, upfront_cost: 1000 }; + const recs = [ + { ...base, id: 'rec-np', payment: 'no-upfront' }, + { ...base, id: 'rec-au', payment: 'all-upfront' }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 2, total_monthly_savings: 300, total_upfront_cost: 2000, avg_payback_months: 1 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + await loadRecommendations(); + document.querySelector('.rec-cell-chevron')!.click(); + + const label = (id: string) => + document.querySelector(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? ''; + expect(label('rec-np')).toContain('No Upfront'); + expect(label('rec-au')).toContain('All Upfront'); + expect(label('rec-np')).not.toBe(label('rec-au')); + }); + + test('engine-only rows get distinct and unmapped-payment rows get clean checkbox names (issue #746)', async () => { + const base = { provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.r5.large', region: 'us-east-1', count: 1, term: 1, savings: 150, upfront_cost: 0 }; + const recs = [ + { ...base, id: 'rec-my', engine: 'mysql', payment: 'no-upfront' }, + { ...base, id: 'rec-pg', engine: 'postgresql', payment: 'no-upfront' }, + { ...base, id: 'rec-x', payment: 'weird-option' }, + { ...base, id: 'rec-none', resource_type: 'db.r5.xlarge' }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 4, total_monthly_savings: 600, total_upfront_cost: 0, avg_payback_months: 1 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + await loadRecommendations(); + document.querySelectorAll('.rec-cell-chevron').forEach(b => b.click()); + + const label = (id: string) => + document.querySelector(`input[data-rec-id="${id}"]`)?.getAttribute('aria-label') ?? ''; + expect(label('rec-my')).toContain('db.r5.large (mysql)'); + expect(label('rec-my')).not.toBe(label('rec-pg')); + expect(label('rec-x')).not.toContain('weird-option'); + for (const id of ['rec-my', 'rec-pg', 'rec-x', 'rec-none']) { + expect(label(id)).not.toMatch(/\s{2,}|\s$/); + } + }); + test('select-all change event updates summary cards in place (PR #283 CR pass-2)', async () => { // Verifies the select-all checkbox event path: after checking select-all, // summary cards reflect the picked-best-per-cell selection WITHOUT a diff --git a/frontend/src/__tests__/settings-accounts.test.ts b/frontend/src/__tests__/settings-accounts.test.ts index 0061d8f03..bd8db15fd 100644 --- a/frontend/src/__tests__/settings-accounts.test.ts +++ b/frontend/src/__tests__/settings-accounts.test.ts @@ -438,6 +438,25 @@ describe('Account form submit', () => { expect(api.listAccounts).toHaveBeenCalled(); // list refreshed }); + test('disables the Save button while the account request is in flight (issue #735)', async () => { + const btn = document.createElement('button'); + btn.type = 'submit'; + document.getElementById('account-form')!.appendChild(btn); + let resolve!: (v: unknown) => void; + (api.createAccount as jest.Mock).mockReturnValue(new Promise((r) => { resolve = r; })); + + document.getElementById('add-aws-account-btn')!.click(); + (document.getElementById('account-name') as HTMLInputElement).value = 'Acct'; + (document.getElementById('account-external-id') as HTMLInputElement).value = '123456789012'; + document.getElementById('account-form')!.dispatchEvent(new Event('submit')); + await new Promise(r => setTimeout(r, 0)); + expect(btn.disabled).toBe(true); + + resolve({ id: 'new-id', name: 'Acct', provider: 'aws', external_id: '123456789012', enabled: true }); + await new Promise(r => setTimeout(r, 0)); + expect(btn.disabled).toBe(false); + }); + test('updates existing account when account-id is set', async () => { (api.updateAccount as jest.Mock).mockResolvedValue({ id: 'existing-id', name: 'Updated', provider: 'aws', external_id: '123456789012', enabled: true diff --git a/frontend/src/__tests__/users.test.ts b/frontend/src/__tests__/users.test.ts index cf77085cb..084956523 100644 --- a/frontend/src/__tests__/users.test.ts +++ b/frontend/src/__tests__/users.test.ts @@ -3184,3 +3184,198 @@ describe('contradictory group combination validation (issue #1405)', () => { }); }); }); + +describe('synthetic current-user row (issue #274)', () => { + const real = { id: '1', email: 'a@test.com', groups: [], mfa_enabled: true }; + const synthetic = { id: 'current', email: 'me@test.com', groups: [], mfa_enabled: false }; + + beforeEach(() => { + document.body.innerHTML = '
'; + userState.setAllUsers([synthetic, real] as any); + userState.setFilteredUsers([synthetic, real] as any); + userState.clearSelectedUserIds(); + }); + + it('disables select, Edit and Delete on the synthetic row only', () => { + userList.renderUsers([synthetic, real] as any); + const row = (id: string) => document.querySelector(`tr.user-row[data-user-id="${id}"]`)!; + expect(row('current').querySelector('.user-checkbox')!.disabled).toBe(true); + expect(row('current').querySelector('.edit-user-btn')!.disabled).toBe(true); + expect(row('current').querySelector('.delete-user-btn')!.disabled).toBe(true); + expect(row('1').querySelector('.edit-user-btn')!.disabled).toBe(false); + }); + + it('select-all skips the synthetic row', () => { + userList.renderUsers([synthetic, real] as any); + const all = document.getElementById('select-all-users') as HTMLInputElement; + all.checked = true; + all.dispatchEvent(new Event('change')); + expect([...userState.selectedUserIds]).toEqual(['1']); + }); + + it('carries the session user mfa_enabled into the synthetic row and the MFA stat', async () => { + require('../state').setCurrentUser({ id: 'u9', email: 'me@test.com', groups: [], mfa_enabled: true }); + (api.listUsers as jest.Mock).mockResolvedValue({ users: [{ ...real, mfa_enabled: false }] }); + (api.listGroups as jest.Mock).mockResolvedValue({ groups: [] }); + await userActions.loadUsers(); + const row = document.querySelector('tr.user-row[data-user-id="current"]')!; + expect(row.textContent).toContain('Enabled'); + const card = [...document.querySelectorAll('.stat-card')].find(c => c.textContent!.includes('MFA Enabled'))!; + expect(card.querySelector('.stat-value')!.textContent).toBe('1'); + }); +}); + +describe('users table checkbox names (issue #746)', () => { + it('labels select-all and each row checkbox with the user email', () => { + document.body.innerHTML = '
'; + const users = [ + { id: 'u1', email: 'a@test.com', groups: [], mfa_enabled: false }, + { id: 'u2', email: 'b@test.com', groups: [], mfa_enabled: false }, + ]; + userList.renderUsers(users as any); + expect(document.getElementById('select-all-users')!.getAttribute('aria-label')).toBe('Select all users'); + const labels = [...document.querySelectorAll('.user-checkbox')].map(c => c.getAttribute('aria-label')); + expect(labels).toEqual(['Select a@test.com', 'Select b@test.com']); + }); +}); + +describe('group toggle confirmation (issue #736)', () => { + const ADMIN = '00000000-0000-5000-8000-000000000001'; + const users = [ + { id: '1', email: 'me@test.com', groups: [ADMIN], mfa_enabled: false }, + { id: '2', email: 'bob@test.com', groups: [ADMIN], mfa_enabled: false }, + ]; + const groups = [{ id: ADMIN, name: 'Administrators', permissions: [], description: '', allowed_accounts: [] }]; + + const toggle = async (userId: string, checked: boolean) => { + document.querySelector(`.user-expand-btn[data-user-id="${userId}"]`)!.click(); + const cb = document.querySelector(`.group-assign-checkbox[data-user-id="${userId}"]`)!; + cb.checked = checked; + cb.dispatchEvent(new Event('change', { bubbles: true })); + for (let i = 0; i < 5; i++) await Promise.resolve(); + return cb; + }; + + beforeEach(() => { + document.body.innerHTML = '
'; + userState.setAllUsers(users as any); + userState.setFilteredUsers(users as any); + userState.setAvailableGroups(groups as any); + (api.updateUser as jest.Mock).mockReset().mockResolvedValue({}); + mockConfirmDialog.mockReset().mockResolvedValue(true); + require('../state').setCurrentUser({ id: '1', email: 'me@test.com', groups: [ADMIN] }); + userList.renderUsers(users as any); + }); + + it('asks before changing membership and skips the API when declined', async () => { + mockConfirmDialog.mockResolvedValueOnce(false); + const cb = await toggle('2', false); + expect(mockConfirmDialog).toHaveBeenCalledTimes(1); + expect(api.updateUser).not.toHaveBeenCalled(); + expect(cb.checked).toBe(true); + }); + + it('applies the change once confirmed', async () => { + await toggle('2', false); + expect(api.updateUser).toHaveBeenCalledWith('2', { groups: [] }); + }); + + it('warns when the admin removes their own Administrators membership', async () => { + await toggle('1', false); + expect(mockConfirmDialog.mock.calls[0]![0]).toMatchObject({ + body: expect.stringContaining('removing yourself'), + destructive: true, + }); + }); + + it('detects self-demotion by user id, not email', async () => { + require('../state').setCurrentUser({ id: '1', email: 'ME@test.com', groups: [ADMIN] }); + await toggle('1', false); + expect(mockConfirmDialog.mock.calls[0]![0]).toMatchObject({ body: expect.stringContaining('removing yourself') }); + }); + + it('saves against the groups current after the confirm resolves', async () => { + let release!: (v: boolean) => void; + mockConfirmDialog.mockReturnValueOnce(new Promise((r) => { release = r; })); + document.querySelector('.user-expand-btn[data-user-id="2"]')!.click(); + const cb = document.querySelector('.group-assign-checkbox[data-user-id="2"]')!; + cb.checked = false; + cb.dispatchEvent(new Event('change', { bubbles: true })); + userState.setAllUsers(users.map(u => (u.id === '2' ? { ...u, groups: [ADMIN, 'g-new'] } : u)) as any); + release(true); + for (let i = 0; i < 5; i++) await Promise.resolve(); + expect(api.updateUser).toHaveBeenCalledWith('2', { groups: ['g-new'] }); + }); +}); + +describe('group toggle re-validation after confirm (issue #736)', () => { + const viewOnly = { id: 'v1', name: 'Viewers', permissions: [{ action: 'view', resource: 'aws' }], description: '', allowed_accounts: [] }; + const writer = { id: 'w1', name: 'Editors', permissions: [{ action: 'create', resource: 'aws' }], description: '', allowed_accounts: [] }; + const user = { id: '2', email: 'bob@test.com', groups: [], mfa_enabled: false }; + + it('refuses to save when a concurrent change makes the combination contradictory', async () => { + document.body.innerHTML = '
'; + userState.setAllUsers([user] as any); + userState.setFilteredUsers([user] as any); + userState.setAvailableGroups([viewOnly, writer] as any); + (api.updateUser as jest.Mock).mockReset().mockResolvedValue({}); + let release!: (v: boolean) => void; + mockConfirmDialog.mockReset().mockReturnValueOnce(new Promise((r) => { release = r; })); + userList.renderUsers([user] as any); + document.querySelector('.user-expand-btn[data-user-id="2"]')!.click(); + const cb = document.querySelector('.group-assign-checkbox[data-user-id="2"][data-group-id="w1"]')!; + cb.checked = true; + cb.dispatchEvent(new Event('change', { bubbles: true })); + userState.setAllUsers([{ ...user, groups: ['v1'] }] as any); + release(true); + for (let i = 0; i < 5; i++) await Promise.resolve(); + expect(api.updateUser).not.toHaveBeenCalled(); + expect(cb.checked).toBe(false); + expect(document.querySelector('.toast-error .toast-message')?.textContent).toContain('Contradictory'); + }); +}); + +describe('select-all with only the synthetic current-user row (issue #274)', () => { + it('disables the select-all checkbox when no row is selectable', () => { + document.body.innerHTML = '
'; + userList.renderUsers([{ id: 'current', email: 'me@test.com', groups: [], mfa_enabled: false }] as any); + expect((document.getElementById('select-all-users') as HTMLInputElement).disabled).toBe(true); + }); +}); + +describe('saveUser submit button (issue #735)', () => { + it('disables Save User while the request is in flight and re-enables it after', async () => { + document.body.innerHTML = ` + +
`; + userState.setAvailableGroups([{ id: 'users', name: 'Users', permissions: [], description: '' }] as any); + userState.setCurrentEditingUser(null); + userModals.openCreateUserModal(); + const btn = document.querySelector('#user-form button[type="submit"]')!; + (document.getElementById('user-email') as HTMLInputElement).value = 'new@test.com'; + (document.getElementById('user-password') as HTMLInputElement).value = 'SecurePass123!'; + const gs = document.getElementById('user-groups') as HTMLSelectElement; + if (gs.options.length > 0) gs.options[0]!.selected = true; + let resolve!: (v: unknown) => void; + (api.createUser as jest.Mock).mockReturnValueOnce(new Promise((r) => { resolve = r; })); + (api.listUsers as jest.Mock).mockResolvedValue({ users: [] }); + (api.listGroups as jest.Mock).mockResolvedValue({ groups: [] }); + + const event = new Event('submit'); + event.preventDefault = jest.fn(); + const pending = userModals.saveUser(event); + expect(btn.disabled).toBe(true); + resolve({}); + await pending; + expect(btn.disabled).toBe(false); + }); +}); diff --git a/frontend/src/apikeys.ts b/frontend/src/apikeys.ts index d8891c7ff..31cd71265 100644 --- a/frontend/src/apikeys.ts +++ b/frontend/src/apikeys.ts @@ -348,8 +348,11 @@ export async function createApiKey(name: string, password: string, expiresAt: Da /** * Handle create API key form submission */ +let createInFlight = false; + export async function handleCreateApiKey(e: Event): Promise { e.preventDefault(); + if (createInFlight) return; const errorEl = document.getElementById('create-apikey-error'); if (errorEl) errorEl.classList.add('hidden'); @@ -387,6 +390,9 @@ export async function handleCreateApiKey(e: Event): Promise { return; } + createInFlight = true; + const submitBtn = document.getElementById('create-apikey-submit-btn') as HTMLButtonElement | null; + if (submitBtn) submitBtn.disabled = true; try { const response = await createApiKey(name, password, expiresAt, permissions); closeCreateKeyModal(); @@ -395,6 +401,9 @@ export async function handleCreateApiKey(e: Event): Promise { } catch (error) { const err = error as Error; showError(`Failed to create API key: ${err.message}`); + } finally { + createInFlight = false; + updateCreateKeySubmitState(); } } @@ -408,6 +417,7 @@ export function showKeyCreatedModal(apiKey: string): void { const modal = document.createElement('div'); modal.id = 'apikey-created-modal'; modal.className = 'modal'; + modal.dataset.requireAck = 'true'; modal.innerHTML = `

API Key Created Successfully

@@ -459,6 +469,7 @@ export function showKeyCreatedModal(apiKey: string): void { const closeBtn = document.getElementById('close-apikey-created-btn'); if (closeBtn) { closeBtn.addEventListener('click', () => { + delete modal.dataset.requireAck; closeModal(modal); modal.remove(); }); @@ -555,7 +566,8 @@ export function initApiKeys(): void { */ function showError(message: string): void { const errorEl = document.getElementById('create-apikey-error'); - if (errorEl) { + const createModalOpen = !document.getElementById('create-apikey-modal')?.classList.contains('hidden'); + if (errorEl && createModalOpen) { errorEl.textContent = message; errorEl.classList.remove('hidden'); } else { diff --git a/frontend/src/auth.ts b/frontend/src/auth.ts index 64ad596e9..fa0dc89e0 100644 --- a/frontend/src/auth.ts +++ b/frontend/src/auth.ts @@ -780,6 +780,10 @@ function showMFACodeStep(): void { // same Function — arrow IIFEs would create a new function each time. const handleLoginSubmitListener = (e: Event): void => { void handleLogin(e); }; const handleMFACodeSubmitListener = (e: Event): void => { void handleMFACodeSubmit(e); }; +const handlePasswordResetSubmitListener = (e: Event): void => { + e.preventDefault(); + void handlePasswordReset(); +}; async function handleMFACodeSubmit(e: Event): Promise { e.preventDefault(); @@ -823,11 +827,14 @@ function showForgotPasswordForm(modal: HTMLElement): void {

We'll send you a link to reset your password.

- +

Back to login

`; - document.getElementById('send-reset-btn')?.addEventListener('click', () => void handlePasswordReset()); + // Keep the inline "enter your email" message instead of the native bubble. + (form as HTMLFormElement).noValidate = true; + form.removeEventListener('submit', handleLoginSubmitListener); + form.addEventListener('submit', handlePasswordResetSubmitListener); document.getElementById('back-to-login-link')?.addEventListener('click', (e) => { e.preventDefault(); reloadPage(); @@ -850,6 +857,8 @@ async function handlePasswordReset(): Promise { return; } + const sendBtn = document.getElementById('send-reset-btn') as HTMLButtonElement | null; + if (sendBtn) sendBtn.disabled = true; try { await api.requestPasswordReset(email); // Issue #457: previously this surfaced as an alert() that, when @@ -864,6 +873,7 @@ async function handlePasswordReset(): Promise { errorDiv.textContent = 'Failed to send reset email. Please try again.'; errorDiv.classList.remove('hidden'); } + if (sendBtn) sendBtn.disabled = false; } } @@ -1083,6 +1093,7 @@ async function openProfileModal(): Promise { // (Re-)render the MFA section so its state reflects whatever the // user's mfa_enabled flag is right now (could have flipped since // the modal was last opened). See renderMFASection below. + setProfileSecretPending(false); renderMFASection(); // Reset live indicators and any stale error so the previous open @@ -1298,6 +1309,18 @@ async function handleMFAEnrollVerify(): Promise { } } +function setProfileSecretPending(pending: boolean): void { + const modal = document.getElementById('profile-modal'); + if (modal) { + if (pending) modal.dataset.requireAck = 'true'; + else delete modal.dataset.requireAck; + } + const cancel = document.getElementById('profile-cancel') as HTMLButtonElement | null; + if (cancel) cancel.disabled = pending; + const save = document.querySelector('#profile-form button[type="submit"]'); + if (save) save.disabled = pending; +} + /** * Step 3 of enrollment: display the plaintext recovery codes. Shown * exactly once; backend stores only bcrypt hashes. Offers a download @@ -1321,10 +1344,12 @@ function renderMFAEnrollCodesStep(codes: string[]): void {
`; + setProfileSecretPending(true); document.getElementById('mfa-codes-download')?.addEventListener('click', () => downloadRecoveryCodes(codes)); document.getElementById('mfa-codes-done')?.addEventListener('click', () => { // Acknowledged — re-render the section so it now shows the // Enabled view (Disable / Regenerate buttons). + setProfileSecretPending(false); renderMFASection(); }); } @@ -1456,6 +1481,7 @@ async function handleMFARegenerateSubmit(): Promise { */ async function saveProfile(e: Event): Promise { e.preventDefault(); + if (document.getElementById('profile-modal')?.dataset.requireAck === 'true') return; const email = (document.getElementById('profile-email') as HTMLInputElement).value; const currentPassword = (document.getElementById('profile-current-password') as HTMLInputElement).value; diff --git a/frontend/src/modal.ts b/frontend/src/modal.ts index 7f9cfcab4..d01a8e372 100644 --- a/frontend/src/modal.ts +++ b/frontend/src/modal.ts @@ -11,6 +11,10 @@ * openModal(modalEl, { onClose: () => discardCallerState() }); // runs on EVERY close path (issue #331) * closeModal(modalEl); // hides + releases trap + restores focus + runs onClose * + * A modal holding a one-time secret sets `data-require-ack="true"` so + * Escape does not dismiss it; the caller removes the attribute once the user + * acknowledges; until then closeModal is a no-op. + * * The helper toggles the existing `.hidden` CSS class so it slots into the * established show/hide convention without touching the stylesheet. */ @@ -157,6 +161,7 @@ export function openModal( * legacy code paths that call closeModal directly from a callback safe. */ export function closeModal(el: HTMLElement): void { + if (el.dataset.requireAck === 'true') return; const state = openModals.get(el); teardown(el); el.classList.add('hidden'); diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 6f44b1be9..f6999696e 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -3194,8 +3194,19 @@ function buildVariantRowMarkup( const planBtnHtml = canAccess('create', 'plans') ? `` : ''; + const selectLabel = escapeHtml( + [ + 'Select', + rec.service, + rec.engine ? `${rec.resource_type} (${rec.engine})` : rec.resource_type, + rec.region, + accountName, + formatTerm(rec.term), + PAYMENT_DISPLAY_LABELS[rec.payment ?? ''] ?? '', + ].filter(part => part).join(' '), + ); const checkboxCell = showCheckboxes - ? `${planBtnHtml}` + ? `${planBtnHtml}` : ``; return ` diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index e8b0bb332..efbed45af 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -2572,6 +2572,16 @@ async function saveAccountCredentialsIfFilled(accountId: string, provider: Accou */ async function handleAccountFormSubmit(e: Event): Promise { e.preventDefault(); + const submitBtn = document.querySelector('#account-form button[type="submit"]'); + if (submitBtn) submitBtn.disabled = true; + try { + await submitAccountForm(); + } finally { + if (submitBtn) submitBtn.disabled = false; + } +} + +async function submitAccountForm(): Promise { const provider = accountModalProvider; const req = buildAccountRequest(provider); diff --git a/frontend/src/users/userActions.ts b/frontend/src/users/userActions.ts index 8827509d0..a5d9dd991 100644 --- a/frontend/src/users/userActions.ts +++ b/frontend/src/users/userActions.ts @@ -60,7 +60,7 @@ function withCurrentUser(users: api.APIUser[]): api.APIUser[] { id: 'current', email: current.email, groups: Array.isArray(current.groups) ? current.groups : [], - mfa_enabled: false, + mfa_enabled: current.mfa_enabled === true, }; return [synthetic, ...users]; } diff --git a/frontend/src/users/userList.ts b/frontend/src/users/userList.ts index 92261378f..06b39a320 100644 --- a/frontend/src/users/userList.ts +++ b/frontend/src/users/userList.ts @@ -17,6 +17,11 @@ import { import { escapeHtml, formatRelativeTime, formatDate, showSuccess, showError, validateGroupCombination } from './utils'; import { openEditUserModal, deleteUser } from './userActions'; import { ADMINISTRATORS_GROUP_ID } from '../permissions'; +import { confirmDialog } from '../confirmDialog'; +import { getCurrentUser } from '../state'; + +// Placeholder row id withCurrentUser prepends when the API omits the caller. +const SYNTHETIC_USER_ID = 'current'; /** * Single AbortController for the document-level group-assign-checkbox @@ -74,12 +79,13 @@ export function renderUsers(users: APIUser[]): void { return; } + const selectableCount = users.filter(u => u.id !== SYNTHETIC_USER_ID).length; const table = ` @@ -94,7 +100,7 @@ export function renderUsers(users: APIUser[]): void { ${users.map(user => ` @@ -191,7 +197,7 @@ function renderUserExpandPanel(user: APIUser): string { + ${userGroups.has(g.id) ? 'checked' : ''} ${user.id === SYNTHETIC_USER_ID ? 'disabled' : ''}> ${escapeHtml(g.name)} `).join('')} @@ -225,30 +231,63 @@ async function toggleUserGroup(userId: string, groupId: string, checked: boolean const group = availableGroups.find(g => g.id === groupId); const groupLabel = group ? group.name : groupId; - const next = checked - ? [...new Set([...user.groups, groupId])] - : user.groups.filter(g => g !== groupId); + const nextFor = (u: APIUser): string[] => (checked + ? [...new Set([...u.groups, groupId])] + : u.groups.filter(g => g !== groupId)); + const next = nextFor(user); // Validate the resulting group combination before hitting the API. // A view-only group (all permissions are view:*) combined with a // write-capable group is semantically contradictory (issue #1405). - const combinationError = validateGroupCombination(next, availableGroups); - if (combinationError) { - showError(combinationError); - // Revert the checkbox immediately so the DOM reflects reality. + const revertCheckbox = (): void => { const cb = document.querySelector( `.group-assign-checkbox[data-user-id="${userId}"][data-group-id="${groupId}"]`, ); if (cb) cb.checked = !checked; + }; + const combinationError = validateGroupCombination(next, availableGroups); + if (combinationError) { + showError(combinationError); + revertCheckbox(); + return; + } + + const selfDemotion = !checked + && groupId === ADMINISTRATORS_GROUP_ID + && user.id === getCurrentUser()?.id; + const ok = await confirmDialog({ + title: checked + ? `Add ${user.email} to "${groupLabel}"?` + : `Remove ${user.email} from "${groupLabel}"?`, + body: selfDemotion + ? 'You are removing yourself from Administrators. You will lose access to administration, and another administrator must restore it.' + : checked + ? `${user.email} will gain the permissions of the "${groupLabel}" group.` + : `${user.email} will lose the permissions of the "${groupLabel}" group.`, + confirmLabel: checked ? 'Add to group' : 'Remove from group', + destructive: !checked, + }); + if (!ok) { + revertCheckbox(); + return; + } + + // A concurrent toggle may have changed the user while the dialog was open. + const latest = allUsers.find(u => u.id === userId) ?? user; + const groupsToSave = nextFor(latest); + const latestError = validateGroupCombination(groupsToSave, availableGroups); + if (latestError) { + showError(latestError); + revertCheckbox(); return; } try { - await api.updateUser(userId, { groups: next }); + await api.updateUser(userId, { groups: groupsToSave }); // Patch the user in-memory so subsequent renders and effectivePermissions // calculations reflect the change without a full round-trip re-render. - const updatedUser: APIUser = { ...user, groups: next }; + const updatedUser: APIUser = { ...latest, groups: groupsToSave }; setAllUsers(allUsers.map(u => (u.id === userId ? updatedUser : u))); // Re-render the expand panel in place so the checkboxes and permissions @@ -301,13 +340,13 @@ function setupUserTableListeners(): void { selectAllCheckbox.addEventListener('change', (e) => { const checked = (e.target as HTMLInputElement).checked; if (checked) { - filteredUsers.forEach(user => addSelectedUserId(user.id)); + filteredUsers.filter(u => u.id !== SYNTHETIC_USER_ID).forEach(user => addSelectedUserId(user.id)); } else { clearSelectedUserIds(); } // Patch each row's selected class and its individual checkbox without // re-rendering the whole table (preserves open expand panels). - filteredUsers.forEach(user => { + filteredUsers.filter(u => u.id !== SYNTHETIC_USER_ID).forEach(user => { const row = document.querySelector( `tr.user-row[data-user-id="${user.id}"]`, ); @@ -352,7 +391,8 @@ function setupUserTableListeners(): void { const selectAll = document.getElementById('select-all-users') as HTMLInputElement | null; if (selectAll) { selectAll.checked = - selectedUserIds.size > 0 && selectedUserIds.size === filteredUsers.length; + selectedUserIds.size > 0 && + selectedUserIds.size === filteredUsers.filter(u => u.id !== SYNTHETIC_USER_ID).length; } updateBulkActionsBar(); diff --git a/frontend/src/users/userModals.ts b/frontend/src/users/userModals.ts index 85b8a597e..107121362 100644 --- a/frontend/src/users/userModals.ts +++ b/frontend/src/users/userModals.ts @@ -105,7 +105,16 @@ export function closeUserModal(): void { */ export async function saveUser(e: Event): Promise { e.preventDefault(); + const submitBtn = document.querySelector('#user-form button[type="submit"]'); + if (submitBtn) submitBtn.disabled = true; + try { + await submitUserForm(); + } finally { + if (submitBtn) submitBtn.disabled = false; + } +} +async function submitUserForm(): Promise { const email = (document.getElementById('user-email') as HTMLInputElement).value; const password = (document.getElementById('user-password') as HTMLInputElement).value; // role selector removed: PR #912 drops the role column. diff --git a/frontend/tests-e2e/execute-now-warning.spec.ts b/frontend/tests-e2e/execute-now-warning.spec.ts index 2a0f81c96..537a977b2 100644 --- a/frontend/tests-e2e/execute-now-warning.spec.ts +++ b/frontend/tests-e2e/execute-now-warning.spec.ts @@ -21,7 +21,7 @@ for (const sample of [ await page.goto('/opportunities'); const selected = page.locator(`tr.recommendation-row[data-rec-id="${row.id}"]`); await expect(selected).toBeVisible(); - await selected.getByRole('checkbox', { name: 'Select recommendation' }).check(); + await selected.getByRole('checkbox', { name: /^Select / }).check(); await page.locator('#bulk-purchase-btn').click(); await expect(page.locator('#purchase-details')).toContainText(row.resource_type); const warning = page.locator('#purchase-details .direct-execute-warning'); @@ -65,7 +65,7 @@ test('mixed providers use fan-out without cancellation reassurance', async ({ pa await page.goto('/opportunities'); for (const id of ['r01', 'r12']) { await page.locator(`tr.recommendation-row[data-rec-id="${id}"]`) - .getByRole('checkbox', { name: 'Select recommendation' }).check(); + .getByRole('checkbox', { name: /^Select / }).check(); } await page.locator('#bulk-purchase-btn').click(); await expect(page.locator('#fanout-summary')).toBeVisible(); diff --git a/frontend/tests-e2e/recommendations.spec.ts b/frontend/tests-e2e/recommendations.spec.ts index 4e3e632fb..b77073ded 100644 --- a/frontend/tests-e2e/recommendations.spec.ts +++ b/frontend/tests-e2e/recommendations.spec.ts @@ -13,7 +13,7 @@ async function openOpportunities(page: Page): Promise { async function selectRecommendation(page: Page, id: string): Promise { const row = page.locator(`tr.recommendation-row[data-rec-id="${id}"]`); - await row.getByRole('checkbox', { name: 'Select recommendation' }).check(); + await row.getByRole('checkbox', { name: /^Select / }).check(); } function callsTo(handle: MockHandle, suffix: string): Array> {
- 0 && selectedUserIds.size === users.length ? 'checked' : ''}> + 0 && selectedUserIds.size === selectableCount ? 'checked' : ''}> Email
- + @@ -102,7 +108,7 @@ export function renderUsers(users: APIUser[]): void {
${escapeHtml(user.email)} - ${user.id === 'current' ? 'You' : ''} + ${user.id === SYNTHETIC_USER_ID ? 'You' : ''}
@@ -119,8 +125,8 @@ export function renderUsers(users: APIUser[]): void { ${user.last_login ? formatRelativeTime(user.last_login) : 'Never'}
- - + +