From d08080bcd139902fd46ffc0414bce6300f72ee11 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 00:24:43 +0000 Subject: [PATCH] fix(frontend): offer Revoke on scheduled AWS and GCP executions The History Revoke button was gated on provider === 'azure' for every row. That fits completed rows, since only Azure reservations can be returned after purchase, but a scheduled row has not called any cloud yet: the backend cancels it with CancelScheduledExecutionAtomic for every provider, and stamps revocation_window_closes_at with the fire time regardless of provider. AWS and GCP scheduled executions therefore had no UI path to stop them before they fired. Apply the Azure restriction to completed (and legacy blank-status) rows only. Scheduled rows keep the existing window and revoke RBAC checks. The scheduled confirm copy no longer promises an Azure reservation return, since the row may be AWS or GCP. Refs #924 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/history-revoke-button.test.ts | 77 +++++++++++++++++++ frontend/src/history.ts | 32 +++++--- .../history-scheduled-revoke.spec.ts | 69 +++++++++++++++++ 3 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 frontend/tests-e2e/history-scheduled-revoke.spec.ts diff --git a/frontend/src/__tests__/history-revoke-button.test.ts b/frontend/src/__tests__/history-revoke-button.test.ts index 96f2e5428..59ad9b4f0 100644 --- a/frontend/src/__tests__/history-revoke-button.test.ts +++ b/frontend/src/__tests__/history-revoke-button.test.ts @@ -18,6 +18,8 @@ * 4. already-revoked Azure row (revoked_at set) -> hidden. * 5. non-Azure (aws/gcp) row with a window stamped -> hidden. * 6. anonymous (no current user cached) -> hidden. + * 7. scheduled row on any provider (aws/gcp/azure) with a future window -> + * shown; a scheduled row whose fire time has passed -> hidden (#924). */ import { loadHistory } from '../history'; @@ -307,6 +309,81 @@ describe('History inline Revoke button (issue #290)', () => { }); }); +// issue #924: a scheduled row has not called any cloud SDK yet, so the backend +// cancels it for free on every provider. The Azure-only rule applies to +// completed rows (returns) only. +describe('History Revoke on scheduled rows of every provider (issue #924)', () => { + beforeEach(() => { + setupDOM(); + jest.clearAllMocks(); + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + }); + + test.each(['aws', 'gcp', 'azure'])('shows Revoke for a scheduled %s row with a future fire time', async (provider) => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: `exec-${provider}`, provider, status: 'scheduled', revocation_window_closes_at: FUTURE })], + }); + + await loadHistory(); + + expect(revokeIds()).toEqual([`exec-${provider}`]); + }); + + test.each(['aws', 'gcp'])('still hides Revoke for a completed %s row', async (provider) => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: `commit-${provider}`, provider, status: 'completed', revocation_window_closes_at: FUTURE })], + }); + + await loadHistory(); + + expect(revokeIds()).toEqual([]); + }); + + test('hides Revoke on a scheduled AWS row once its fire time has passed', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'exec-aws-late', provider: 'aws', status: 'scheduled', revocation_window_closes_at: PAST })], + }); + + await loadHistory(); + + expect(revokeIds()).toEqual([]); + }); + + test('hides Revoke on a scheduled GCP row without revoke permission', async () => { + (getCurrentUser as jest.Mock).mockReturnValue(NON_REVOKER_USER); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'exec-gcp', provider: 'gcp', status: 'scheduled', revocation_window_closes_at: FUTURE })], + }); + + await loadHistory(); + + expect(revokeIds()).toEqual([]); + }); + + test('scheduled AWS revoke: free cancel with provider-neutral copy, no quote', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'exec-aws', provider: 'aws', status: 'scheduled', revocation_window_closes_at: FUTURE })], + }); + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.revokePurchase as jest.Mock).mockResolvedValue({ status: 'canceled' }); + + await loadHistory(); + document.querySelector('.history-revoke-btn')!.click(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + const body = (confirmDialog as jest.Mock).mock.calls[0][0].body as string; + expect(body).toContain('cancel the scheduled purchase'); + expect(body).not.toContain('Azure'); + expect(api.getRevokeQuote).not.toHaveBeenCalled(); + expect(api.revokePurchase).toHaveBeenCalledWith('exec-aws', undefined); + }); +}); + // platform#96: the backend refuses an Azure return unless the body carries the // quote the user consented to, so the click flow must fetch the quote, show it, // and send amount + currency back. diff --git a/frontend/src/history.ts b/frontend/src/history.ts index b4c183d44..2f61464e2 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -699,7 +699,11 @@ function canRetryFailedRow(p: HistoryPurchase): boolean { // Conditions: // * status must be "completed", "" (legacy blank), or "scheduled" // (pre-fire delay: the cloud SDK has not been called yet -- free cancel); -// * provider must be "azure" (AWS and GCP have no direct cancel API); +// * completed (and legacy blank) rows must be provider "azure": only Azure +// reservations can be returned after purchase, AWS and GCP have no direct +// cancel API. Scheduled rows are allowed for every provider because the +// backend cancels them via CancelScheduledExecutionAtomic before any +// cloud call (issue #924); // * revocation_window_closes_at must be in the future; // for "scheduled" rows this field is populated with scheduled_execution_at // by the backend (issue #290, second-wave CR Finding E); @@ -712,7 +716,7 @@ function canRetryFailedRow(p: HistoryPurchase): boolean { function canRevokeCompletedRow(p: HistoryPurchase): boolean { const status = (p.status || '').toLowerCase(); if (status !== 'completed' && status !== '' && status !== 'scheduled') return false; - if ((p.provider || '').toLowerCase() !== 'azure') return false; + if (status !== 'scheduled' && (p.provider || '').toLowerCase() !== 'azure') return false; if (p.revoked_at) return false; // already revoked if (!p.revocation_window_closes_at) return false; if (new Date(p.revocation_window_closes_at) <= new Date()) return false; @@ -977,15 +981,16 @@ function renderActionCell(p: HistoryPurchase): string { // Cancel button, or be an Azure row still inside its revoke window. const trailingActions: string[] = []; if (p.purchase_id) { - // Completed Azure row within revocation window: Revoke button (issue - // #290). Only Azure supports direct in-app revocation; AWS and GCP have - // no cancel API so the button is suppressed for those providers. + // Revoke button (issue #290): completed Azure rows within the refund + // window, and scheduled rows on any provider before they fire (#924). + // Completed AWS and GCP rows have no cancel API, so they get no button. if (canRevokeCompletedRow(p)) { trailingActions.push(``); } // Completed Standard RI rows (AWS): Cancel listing / Sell on Marketplace - // (issue #292). Mutually exclusive with revoke in practice (revoke is - // Azure-only, marketplace is AWS Standard-RI-only). + // (issue #292). Mutually exclusive with revoke in practice (revoke on a + // completed row is Azure-only, marketplace is AWS Standard-RI-only and + // never applies to a scheduled row). if (canCancelMarketplaceListing(p)) { trailingActions.push(``); } else if (canSellOnMarketplace(p)) { @@ -1461,7 +1466,8 @@ function wireRowActionHandlers(container: HTMLElement): void { }); // Wire the inline Revoke button on completed Azure rows within the - // free-cancel window (issue #290). quote -> confirmDialog -> POST -> reload. + // free-cancel window (issue #290) and on scheduled rows of any provider + // (#924). quote -> confirmDialog -> POST -> reload. // Completed rows fetch the Azure refund quote first so the user consents to // a concrete amount, which the backend re-checks before issuing the refund. // The backend is the security boundary; canRevokeCompletedRow is a @@ -1482,12 +1488,14 @@ function wireRowActionHandlers(container: HTMLElement): void { return; } } - const refundLine = quote - ? `Azure will refund ${quote.refund_amount.toFixed(2)} ${quote.refund_currency}.` - : 'The scheduled purchase has not been submitted yet, so nothing will be charged.'; + // A scheduled row has not reached any cloud yet, so its copy must not + // promise an Azure return: it may be an AWS or GCP purchase (#924). + const body = quote + ? `This will request an Azure reservation return. Azure will refund ${quote.refund_amount.toFixed(2)} ${quote.refund_currency}.` + : 'This will cancel the scheduled purchase. It has not been submitted yet, so nothing will be charged.'; const ok = await confirmDialog({ title: 'Revoke this purchase within the free-cancel window?', - body: `This will request an Azure reservation return. ${refundLine} This action cannot be undone.`, + body: `${body} This action cannot be undone.`, confirmLabel: 'Revoke purchase', destructive: true, }); diff --git a/frontend/tests-e2e/history-scheduled-revoke.spec.ts b/frontend/tests-e2e/history-scheduled-revoke.spec.ts new file mode 100644 index 000000000..1ab10bac9 --- /dev/null +++ b/frontend/tests-e2e/history-scheduled-revoke.spec.ts @@ -0,0 +1,69 @@ +import { test, expect, type Page } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +// Issue #924: a scheduled execution has not called any cloud SDK yet, so the +// backend revokes it for free on every provider. AWS and GCP scheduled rows +// must offer Revoke too; completed AWS and GCP rows still must not. + +const FUTURE = () => new Date(Date.now() + 86_400_000).toISOString(); + +function row(id: string, provider: string, status: string) { + return { + purchase_id: id, timestamp: new Date().toISOString(), provider, + service: 'compute', resource_type: 'm5.large', region: 'us-east-1', count: 1, term: 1, + upfront_cost: 1200, monthly_cost: 50, estimated_savings: 10, + status, account_id: 'acct-001', created_by_user_id: 'user-smoke', + revocation_window_closes_at: FUTURE(), + }; +} + +async function fixture(page: Page) { + await seedAuth(page); + await mockApi(page); + const rows = [ + row('aaaaaaaa-0000-0000-0000-000000000001', 'aws', 'scheduled'), + row('aaaaaaaa-0000-0000-0000-000000000002', 'gcp', 'scheduled'), + row('aaaaaaaa-0000-0000-0000-000000000003', 'aws', 'completed'), + ]; + const posts: string[] = []; + const quoteGets: string[] = []; + await page.route('**/api/auth/me/permissions', route => route.fulfill({ json: { permissions: [ + { action: 'admin', resource: '*' }, { action: 'revoke-any', resource: 'purchases' }, + ] } })); + await page.route('**/api/info/deployment', route => route.fulfill({ json: {} })); + await page.route('**/api/history**', route => route.fulfill({ json: { summary: {}, purchases: rows } })); + await page.route('**/api/purchases/**', async route => { + if (route.request().method() === 'POST') { + posts.push(route.request().url()); + await route.fulfill({ json: { status: 'canceled' } }); + } else { + quoteGets.push(route.request().url()); + await route.fulfill({ json: { refund_amount: 1, refund_currency: 'USD' } }); + } + }); + return { posts, quoteGets }; +} + +test('scheduled AWS and GCP rows offer Revoke; completed AWS rows do not', async ({ page }) => { + await fixture(page); + await page.goto('/purchases/history'); + const ids = page.locator('#history-list .history-revoke-btn'); + await expect(ids).toHaveCount(2); + expect(await ids.evaluateAll(btns => btns.map(b => (b as HTMLElement).dataset['revokeId']))).toEqual([ + 'aaaaaaaa-0000-0000-0000-000000000001', + 'aaaaaaaa-0000-0000-0000-000000000002', + ]); +}); + +test('revoking a scheduled AWS row cancels it without a refund quote or Azure copy', async ({ page }) => { + const { posts, quoteGets } = await fixture(page); + await page.goto('/purchases/history'); + await page.locator('#history-list .history-revoke-btn[data-revoke-id="aaaaaaaa-0000-0000-0000-000000000001"]').click(); + const dialog = page.locator('.modal-confirm-backdrop'); + await expect(dialog).toContainText('cancel the scheduled purchase'); + await expect(dialog).not.toContainText('Azure'); + await page.locator('.modal-confirm-actions button').last().click(); + await expect.poll(() => posts.length).toBe(1); + expect(posts[0]).toContain('/purchases/aaaaaaaa-0000-0000-0000-000000000001/revoke'); + expect(quoteGets.filter(u => u.includes('/calculate'))).toEqual([]); +});