From 697a0f68d582808d2befce50760c04ad91cb2681 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 23:14:45 +0200 Subject: [PATCH] fix(api): make Azure revoke refund guard mandatory and currency-aware POST /api/purchases/{id}/revoke skipped the refund-divergence check when the body omitted expected_refund_amount or Azure returned no quote, and compared amounts without currencies. The guard now requires expected_refund_amount and expected_refund_currency (400), fails closed when CalculateRefund returns no amount or currency (422), and refuses a currency mismatch (422) instead of converting. The History Revoke button now fetches the quote, shows it in the confirm dialog, and sends it back. Scheduled rows (free cancel) take no quote. Closes #96 --- .../__tests__/history-revoke-button.test.ts | 57 ++++++ frontend/src/api/index.ts | 3 +- frontend/src/api/purchases.ts | 30 ++- frontend/src/history.ts | 25 ++- internal/api/handler_purchases_revoke.go | 79 +++++--- .../handler_purchases_revoke_guard_test.go | 179 ++++++++++++++++++ internal/api/handler_purchases_revoke_test.go | 38 ++-- internal/api/router.go | 2 +- 8 files changed, 355 insertions(+), 58 deletions(-) create mode 100644 internal/api/handler_purchases_revoke_guard_test.go diff --git a/frontend/src/__tests__/history-revoke-button.test.ts b/frontend/src/__tests__/history-revoke-button.test.ts index 264ae1f10..470dd1c7f 100644 --- a/frontend/src/__tests__/history-revoke-button.test.ts +++ b/frontend/src/__tests__/history-revoke-button.test.ts @@ -26,6 +26,7 @@ jest.mock('../api', () => ({ getHistory: jest.fn(), getConfig: jest.fn().mockResolvedValue({ global: {} }), revokePurchase: jest.fn(), + getRevokeQuote: jest.fn(), })); jest.mock('../navigation', () => ({ @@ -71,6 +72,7 @@ jest.mock('../state', () => ({ import * as api from '../api'; import { getCurrentUser } from '../state'; +import { confirmDialog } from '../confirmDialog'; // Administrators group GUID -- mirrors ADMINISTRATORS_GROUP_ID in // frontend/src/permissions.ts. Without this, isAdmin() returns false and @@ -304,3 +306,58 @@ describe('History inline Revoke button (issue #290)', () => { expect(revokeIds()).toContain('commit-scheduled'); }); }); + +// 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. +describe('History Revoke click flow (platform#96)', () => { + const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + + beforeEach(() => { + setupDOM(); + jest.clearAllMocks(); + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + }); + + async function clickRevoke(status: string): Promise { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'commit-azure', status, revocation_window_closes_at: FUTURE })], + }); + await loadHistory(); + document.querySelector('.history-revoke-btn')!.click(); + await flush(); + } + + test('completed row: fetches the quote, shows it, and sends it with the revoke', async () => { + const quote = { refund_amount: 4200, refund_currency: 'EUR', quoted_at: '2024-01-16T00:00:00Z' }; + (api.getRevokeQuote as jest.Mock).mockResolvedValue(quote); + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.revokePurchase as jest.Mock).mockResolvedValue({ status: 'revoked' }); + + await clickRevoke('completed'); + + expect(api.getRevokeQuote).toHaveBeenCalledWith('commit-azure'); + expect((confirmDialog as jest.Mock).mock.calls[0][0].body).toContain('4200.00 EUR'); + expect(api.revokePurchase).toHaveBeenCalledWith('commit-azure', quote); + }); + + test('completed row: a failed quote never reaches the revoke call', async () => { + (api.getRevokeQuote as jest.Mock).mockRejectedValue(new Error('boom')); + + await clickRevoke('completed'); + + expect(confirmDialog).not.toHaveBeenCalled(); + expect(api.revokePurchase).not.toHaveBeenCalled(); + }); + + test('scheduled row: free cancel, no quote requested', async () => { + (confirmDialog as jest.Mock).mockResolvedValue(true); + (api.revokePurchase as jest.Mock).mockResolvedValue({ status: 'cancelled' }); + + await clickRevoke('scheduled'); + + expect(api.getRevokeQuote).not.toHaveBeenCalled(); + expect(api.revokePurchase).toHaveBeenCalledWith('commit-azure', undefined); + }); +}); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index dd7f8ffff..b9e2b30a0 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -145,10 +145,11 @@ export { deletePlannedPurchase, createPlannedPurchases, revokePurchase, + getRevokeQuote, createMarketplaceListing, cancelMarketplaceListing } from './purchases'; -export type { RetryPurchaseResult, RevokePurchaseResult, MarketplacePriceTier, MarketplaceListResult } from './purchases'; +export type { RetryPurchaseResult, RevokePurchaseResult, RevokeQuote, MarketplacePriceTier, MarketplaceListResult } from './purchases'; // Re-export users functions export { diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts index a965c74c2..8cc9a4c61 100644 --- a/frontend/src/api/purchases.ts +++ b/frontend/src/api/purchases.ts @@ -7,7 +7,8 @@ import type { Recommendation, PurchaseResult, PurchaseDetails, - PlannedPurchasesResponse + PlannedPurchasesResponse, + RequestOptions } from './types'; /** @@ -84,8 +85,31 @@ export interface RevokePurchaseResult { revoked_via: string; } -export async function revokePurchase(purchaseId: string): Promise { - return apiRequest(`/purchases/${purchaseId}/revoke`, { method: 'POST' }); +/** Azure refund quote from GET /purchases/{id}/revoke/calculate. */ +export interface RevokeQuote { + refund_amount: number; + refund_currency: string; + quoted_at: string; +} + +export async function getRevokeQuote(purchaseId: string): Promise { + return apiRequest(`/purchases/${purchaseId}/revoke/calculate`); +} + +/** + * The backend requires `quote` for completed Azure rows and rejects the + * return if Azure's current quote differs in amount or currency. Scheduled + * (pre-fire) rows are a free cancel and take no quote. + */ +export async function revokePurchase(purchaseId: string, quote?: RevokeQuote): Promise { + const init: RequestOptions = { method: 'POST' }; + if (quote) { + init.body = JSON.stringify({ + expected_refund_amount: quote.refund_amount, + expected_refund_currency: quote.refund_currency, + }); + } + return apiRequest(`/purchases/${purchaseId}/revoke`, init); } /** diff --git a/frontend/src/history.ts b/frontend/src/history.ts index f6a6be317..0f5554c17 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -6,6 +6,7 @@ import * as api from './api'; import * as state from './state'; import { formatCurrency, formatDate, formatTerm, escapeHtml, escapeHtmlAttr, amortizedMonthly } from './utils'; import type { HistoryResponse, HistorySummary, HistoryPurchase } from './types'; +import type { RevokeQuote } from './api'; import { switchTab } from './navigation'; import { confirmDialog } from './confirmDialog'; import { buildApprovalDetailsBody } from './approval-details'; @@ -899,7 +900,7 @@ function renderActionCell(p: HistoryPurchase): string { // #290). Only Azure supports direct in-app revocation; AWS and GCP have // no cancel API so the button is suppressed for those providers. if (canRevokeCompletedRow(p)) { - trailingActions.push(``); + trailingActions.push(``); } // Completed Standard RI rows (AWS): Cancel listing / Sell on Marketplace // (issue #292). Mutually exclusive with revoke in practice (revoke is @@ -1406,7 +1407,9 @@ function wireRowActionHandlers(container: HTMLElement): void { }); // Wire the inline Revoke button on completed Azure rows within the - // free-cancel window (issue #290). confirmDialog -> POST -> reload. + // free-cancel window (issue #290). 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 // UX gate that hides the button when the call would fail, but a stale // cache can still surface a 4xx -- handle it like any other failure. @@ -1414,9 +1417,23 @@ function wireRowActionHandlers(container: HTMLElement): void { btn.addEventListener('click', async () => { const id = btn.dataset['revokeId']; if (!id) return; + let quote: RevokeQuote | undefined; + if (btn.dataset['revokeScheduled'] !== 'true') { + try { + quote = await api.getRevokeQuote(id); + } catch (quoteError) { + console.error('Failed to get refund quote:', quoteError); + const err = quoteError as Error; + showToast({ message: `Failed to get refund quote: ${err.message || 'unknown error'}`, kind: 'error' }); + 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.'; const ok = await confirmDialog({ title: 'Revoke this purchase within the free-cancel window?', - body: 'This will request an Azure reservation return. The charge will be refunded if the request is within the 7-day window. This action cannot be undone.', + body: `This will request an Azure reservation return. ${refundLine} This action cannot be undone.`, confirmLabel: 'Revoke purchase', destructive: true, }); @@ -1424,7 +1441,7 @@ function wireRowActionHandlers(container: HTMLElement): void { const rowActions = sameRowActions(btn); rowActions.forEach((b) => { b.disabled = true; }); try { - await api.revokePurchase(id); + await api.revokePurchase(id, quote); } catch (revokeError) { console.error('Failed to revoke purchase:', revokeError); const err = revokeError as Error; diff --git a/internal/api/handler_purchases_revoke.go b/internal/api/handler_purchases_revoke.go index b4ced4e15..391dae0b2 100644 --- a/internal/api/handler_purchases_revoke.go +++ b/internal/api/handler_purchases_revoke.go @@ -65,12 +65,12 @@ type revokeQuoteResult struct { // revokeConfirmBody is the JSON body expected on // POST /api/purchases/{purchaseId}/revoke. -// ExpectedRefundAmount is the amount the user consented to after seeing the -// quote, used for TOCTOU-divergence detection. +// ExpectedRefundAmount and ExpectedRefundCurrency are the quote the user +// consented to (from GET /revoke/calculate), used for TOCTOU-divergence +// detection. Both are required for Azure revocations. type revokeConfirmBody struct { - // ExpectedRefundAmount is the refund amount the user confirmed. - // Required when the purchase has an Azure revocation window. - ExpectedRefundAmount *float64 `json:"expected_refund_amount"` + ExpectedRefundAmount *float64 `json:"expected_refund_amount"` + ExpectedRefundCurrency string `json:"expected_refund_currency"` } // AzureRevocationWindowDays is the number of days after purchase within which @@ -211,9 +211,9 @@ func (h *Handler) loadAndRevokePurchaseHistory(ctx context.Context, req *events. }, nil } - // Parse the optional expected_refund_amount from the request body. - // Azure revocations require this for TOCTOU-divergence detection - // (the two-step quote-then-confirm flow, issue #290 Finding #4). + // Parse the confirmed quote from the request body. Azure revocations + // require it for TOCTOU-divergence detection (the two-step + // quote-then-confirm flow, issue #290 Finding #4); callAzureReturn enforces it. var body revokeConfirmBody if req.Body != "" { if jsonErr := json.Unmarshal([]byte(req.Body), &body); jsonErr != nil { @@ -221,7 +221,7 @@ func (h *Handler) loadAndRevokePurchaseHistory(ctx context.Context, req *events. } } - return h.dispatchProviderRevoke(ctx, record, body.ExpectedRefundAmount) + return h.dispatchProviderRevoke(ctx, record, body) } // revokeScheduledExecution cancels a Gmail-style pre-fire delayed execution @@ -337,10 +337,10 @@ func (h *Handler) authorizeSessionRevokeExecution(ctx context.Context, session * // dispatchProviderRevoke routes a revocation request to the correct // provider-specific implementation. Extracted from revokePurchase to keep // that function's cyclomatic complexity within the project limit. -func (h *Handler) dispatchProviderRevoke(ctx context.Context, record *config.PurchaseHistoryRecord, expectedRefundAmount *float64) (any, error) { +func (h *Handler) dispatchProviderRevoke(ctx context.Context, record *config.PurchaseHistoryRecord, confirmed revokeConfirmBody) (any, error) { switch record.Provider { case "azure": - return h.revokeAzurePurchase(ctx, record, expectedRefundAmount) + return h.revokeAzurePurchase(ctx, record, confirmed) case "aws": // AWS does not expose a direct RI cancel API. Phase 2 (#291) adds the // AWS Support case path. Return 422 so the frontend hides this button. @@ -417,7 +417,7 @@ func (h *Handler) checkRevokeOwnAccountAccess(ctx context.Context, userID string // // This is the first step of the two-step quote-then-confirm revoke UX // (issue #290 Finding #4). No state is mutated; the result is used by the -// frontend to populate revokeConfirmBody.ExpectedRefundAmount. +// frontend to populate revokeConfirmBody. func (h *Handler) calculateAzureRevoke(ctx context.Context, req *events.LambdaFunctionURLRequest, purchaseID string) (any, error) { _, orderID, reservationID, count, err := h.validateAzureRevokeRequest(ctx, req, purchaseID) if err != nil { @@ -546,7 +546,7 @@ func extractAzureRefundQuote(resp armreservations.CalculateRefundClientPostRespo // Reservations API (CalculateRefund + Return). The reservation order ID and // reservation ID are parsed from the purchase_id ARM resource path stored at // purchase time. -func (h *Handler) revokeAzurePurchase(ctx context.Context, record *config.PurchaseHistoryRecord, expectedRefundAmount *float64) (any, error) { +func (h *Handler) revokeAzurePurchase(ctx context.Context, record *config.PurchaseHistoryRecord, confirmed revokeConfirmBody) (any, error) { // Prefer the window stamped on the row at purchase time (single source of // truth, issue #290). Fall back to recomputing from Timestamp for legacy // rows written before the column was populated, so they remain revocable. @@ -574,7 +574,7 @@ func (h *Handler) revokeAzurePurchase(ctx context.Context, record *config.Purcha return nil, fmt.Errorf("revoke azure: %w", err) } - return h.callAzureReturn(ctx, calcClient, returnClient, record, orderID, reservationID, expectedRefundAmount) + return h.callAzureReturn(ctx, calcClient, returnClient, record, orderID, reservationID, confirmed) } func (h *Handler) getAzureRevokeFactory() azureRevokeClientFactory { @@ -630,17 +630,15 @@ func (h *Handler) buildAzureRevokeClients() (azureCalculateRefundClient, azureRe // CalculateRefund (to get the session ID and quoted amount) followed by Return. // Extracted from revokeAzurePurchase to allow test injection of the two clients. // -// expectedRefundAmount: the amount the user consented to after the -// quote step (GET /revoke/calculate). When provided and the CalculateRefund -// response diverges by more than revokeQuoteEpsilon, the call is rejected with -// 422 so the user can re-quote and confirm the new amount. +// confirmed: the quote the user consented to after the quote step +// (GET /revoke/calculate). It is mandatory; see checkConfirmedRefund. func (h *Handler) callAzureReturn( ctx context.Context, calcClient azureCalculateRefundClient, returnClient azureReturnClient, record *config.PurchaseHistoryRecord, orderID, reservationID string, - expectedRefundAmount *float64, + confirmed revokeConfirmBody, ) (any, error) { // Guard against an order-only ARM path (no /reservations/{id} segment), // which parseAzureReservationIDs returns with an empty reservationID. @@ -650,6 +648,9 @@ func (h *Handler) callAzureReturn( if orderID == "" || reservationID == "" { return nil, NewClientError(422, "cannot determine Azure reservation ID from purchase record; contact Azure Support to request a refund") } + if confirmed.ExpectedRefundAmount == nil || strings.TrimSpace(confirmed.ExpectedRefundCurrency) == "" { + return nil, NewClientError(400, "expected_refund_amount and expected_refund_currency are required; fetch a quote from GET /api/purchases/{id}/revoke/calculate and confirm it") + } // Step 1: CalculateRefund -> sessionID + quoted amount (TOCTOU check). quantity := int32(record.Count) // #nosec G115 -- Azure reservation count validated at purchase; bounded by API limits (< revokeQuoteEpsilon { - return nil, NewClientError(422, fmt.Sprintf( - "refund amount diverged: you confirmed %.2f but Azure now quotes %.2f %s; re-confirm to proceed", - *expectedRefundAmount, *calcRefundAmount, calcRefundCurrency, - )) - } + if quoteErr := checkConfirmedRefund(confirmed, calcRefundAmount, calcRefundCurrency); quoteErr != nil { + return nil, quoteErr } // Partial-success guard (issue #290 Finding #6): flip the in-flight flag @@ -700,6 +691,32 @@ func (h *Handler) callAzureReturn( return h.persistAzureRevocation(ctx, record, calcRefundAmount, calcRefundCurrency) } +// checkConfirmedRefund is the TOCTOU-divergence check: the refund the user +// confirmed must match Azure's current CalculateRefund quote in currency and, +// within revokeQuoteEpsilon, in amount. A mismatch means the quote changed +// between confirmation and the call (e.g. a fee tier boundary was crossed). +// It fails closed when Azure returns no comparable quote, and never converts +// currencies (there is no trusted FX source). +func checkConfirmedRefund(confirmed revokeConfirmBody, calcRefundAmount *float64, calcRefundCurrency string) error { + if calcRefundAmount == nil || strings.TrimSpace(calcRefundCurrency) == "" { + return NewClientError(422, "Azure returned no refund quote to verify against the amount you confirmed; refusing to return the reservation") + } + expectedCurrency := strings.TrimSpace(confirmed.ExpectedRefundCurrency) + if !strings.EqualFold(expectedCurrency, strings.TrimSpace(calcRefundCurrency)) { + return NewClientError(422, fmt.Sprintf( + "refund currency diverged: you confirmed %.2f %s but Azure now quotes %.2f %s; re-confirm to proceed", + *confirmed.ExpectedRefundAmount, expectedCurrency, *calcRefundAmount, calcRefundCurrency, + )) + } + if math.Abs(*confirmed.ExpectedRefundAmount-*calcRefundAmount) > revokeQuoteEpsilon { + return NewClientError(422, fmt.Sprintf( + "refund amount diverged: you confirmed %.2f but Azure now quotes %.2f %s; re-confirm to proceed", + *confirmed.ExpectedRefundAmount, *calcRefundAmount, calcRefundCurrency, + )) + } + return nil +} + // azureCalculateRefund runs the CalculateRefund step and parses out the session // ID (required by Return) and the quoted refund amount/currency (for the TOCTOU // check). Errors are classified into 400 (client) vs 500 (transient). diff --git a/internal/api/handler_purchases_revoke_guard_test.go b/internal/api/handler_purchases_revoke_guard_test.go new file mode 100644 index 000000000..003cf5cf2 --- /dev/null +++ b/internal/api/handler_purchases_revoke_guard_test.go @@ -0,0 +1,179 @@ +package api + +import ( + "context" + "fmt" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + armreservations "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// countingCalcRefundClient returns a fixed CalculateRefund response and counts +// calls, so tests can assert whether Azure was consulted at all. +type countingCalcRefundClient struct { + price *armreservations.Price + calls int +} + +func (s *countingCalcRefundClient) Post(_ context.Context, _ string, _ armreservations.CalculateRefundRequest, _ *armreservations.CalculateRefundClientPostOptions) (armreservations.CalculateRefundClientPostResponse, error) { + s.calls++ + return armreservations.CalculateRefundClientPostResponse{ + CalculateRefundResponse: armreservations.CalculateRefundResponse{ + Properties: &armreservations.RefundResponseProperties{ + SessionID: toPtr("s-guard"), + BillingRefundAmount: s.price, + }, + }, + }, nil +} + +// TestRevokePurchase_AzureRefundGuard drives POST /api/purchases/{id}/revoke +// end to end and asserts the refund-divergence guard is mandatory and +// currency-aware (platform#96): the Return call, which issues an irreversible +// refund, must only happen when the confirmed amount AND currency match +// Azure's current quote. +func TestRevokePurchase_AzureRefundGuard(t *testing.T) { + t.Parallel() + + usd := func(amount float64) *armreservations.Price { + return &armreservations.Price{Amount: toPtr(amount), CurrencyCode: toPtr("USD")} + } + + tests := []struct { + name string + body string + quote *armreservations.Price + wantCode int // 0 means success + wantMsg string + wantCalc int + wantReturn int + }{ + { + name: "missing body rejected before Azure is called", + body: "", + quote: usd(100), + wantCode: 400, + wantMsg: "expected_refund_amount", + }, + { + name: "missing expected currency rejected", + body: `{"expected_refund_amount": 100}`, + quote: usd(100), + wantCode: 400, + wantMsg: "expected_refund_currency", + }, + { + name: "missing expected amount rejected", + body: `{"expected_refund_currency": "USD"}`, + quote: usd(100), + wantCode: 400, + wantMsg: "expected_refund_amount", + }, + { + name: "Azure quote without amount fails closed", + body: `{"expected_refund_amount": 100, "expected_refund_currency": "USD"}`, + quote: nil, + wantCode: 422, + wantMsg: "no refund quote", + wantCalc: 1, + }, + { + name: "Azure quote without currency fails closed", + body: `{"expected_refund_amount": 100, "expected_refund_currency": "USD"}`, + quote: &armreservations.Price{Amount: toPtr(100.0)}, + wantCode: 422, + wantMsg: "no refund quote", + wantCalc: 1, + }, + { + name: "currency mismatch at equal amount rejected", + body: `{"expected_refund_amount": 4200, "expected_refund_currency": "EUR"}`, + quote: usd(4200), + wantCode: 422, + wantMsg: "refund currency diverged", + wantCalc: 1, + }, + { + name: "amount divergence rejected", + body: `{"expected_refund_amount": 4200, "expected_refund_currency": "USD"}`, + quote: usd(1100), + wantCode: 422, + wantMsg: "refund amount diverged", + wantCalc: 1, + }, + { + name: "matching amount and currency revokes", + body: `{"expected_refund_amount": 100, "expected_refund_currency": "usd"}`, + quote: usd(100), + wantCalc: 1, + wantReturn: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { + mockStore.AssertExpectations(t) + mockAuth.AssertExpectations(t) + }) + + r := armReservationRecord() + mockAuth.On("ValidateSession", ctx, "tok").Return(revokeAdminSession(), nil) + mockStore.On("GetExecutionByID", ctx, r.PurchaseID).Return(nil, fmt.Errorf("%w: execution %s", config.ErrNotFound, r.PurchaseID)) + mockStore.On("GetPurchaseHistoryByPurchaseID", ctx, r.PurchaseID).Return(r, nil) + mockStore.On("FlipPurchaseRevocationInFlight", ctx, r.PurchaseID).Return(nil).Maybe() + mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.AnythingOfType("time.Time"), "direct-api", "", mock.Anything, mock.Anything).Return(nil).Maybe() + + calcClient := &countingCalcRefundClient{price: tc.quote} + returnClient := &stubReturnClient{} + h := &Handler{ + config: mockStore, + auth: mockAuth, + azureRevokeFactory: &azureRevokeClientFactory{ + newCredential: func() (azcore.TokenCredential, error) { return nil, nil }, + newCalculateRefundClient: func(azcore.TokenCredential) (azureCalculateRefundClient, error) { + return calcClient, nil + }, + newReturnClient: func(azcore.TokenCredential) (azureReturnClient, error) { + return returnClient, nil + }, + }, + } + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer tok"}, + Body: tc.body, + } + result, err := h.revokePurchase(ctx, req, r.PurchaseID) + + if tc.wantCode == 0 { + require.NoError(t, err) + m, ok := result.(*revokePurchaseResult) + require.True(t, ok) + assert.Equal(t, "revoked", m.Status) + } else { + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "want ClientError, got %v", err) + assert.Equal(t, tc.wantCode, ce.code) + assert.Contains(t, ce.message, tc.wantMsg) + } + assert.Equal(t, tc.wantCalc, calcClient.calls, "CalculateRefund calls") + assert.Equal(t, tc.wantReturn, returnClient.calls, "Return (refund) calls") + }) + } +} + +func confirmedQuote(amount float64, currency string) revokeConfirmBody { + return revokeConfirmBody{ExpectedRefundAmount: &amount, ExpectedRefundCurrency: currency} +} diff --git a/internal/api/handler_purchases_revoke_test.go b/internal/api/handler_purchases_revoke_test.go index 5c3dee2e3..193c1bdfa 100644 --- a/internal/api/handler_purchases_revoke_test.go +++ b/internal/api/handler_purchases_revoke_test.go @@ -258,7 +258,8 @@ func TestRevokePurchase_AzureSuccess(t *testing.T) { resp: armreservations.CalculateRefundClientPostResponse{ CalculateRefundResponse: armreservations.CalculateRefundResponse{ Properties: &armreservations.RefundResponseProperties{ - SessionID: &sessID, + SessionID: &sessID, + BillingRefundAmount: &armreservations.Price{Amount: toPtr(10.0), CurrencyCode: toPtr("USD")}, }, }, }, @@ -268,7 +269,7 @@ func TestRevokePurchase_AzureSuccess(t *testing.T) { h := &Handler{config: mockStore} orderID := "order-abc" resID := "res-xyz" - result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, orderID, resID, nil) + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, orderID, resID, confirmedQuote(10, "USD")) require.NoError(t, err) m, ok := result.(*revokePurchaseResult) require.True(t, ok) @@ -289,7 +290,7 @@ func TestRevokePurchase_AzureCalcRefundClientError(t *testing.T) { returnClient := &stubReturnClient{} h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -307,7 +308,8 @@ func TestRevokePurchase_AzureReturnClientError(t *testing.T) { resp: armreservations.CalculateRefundClientPostResponse{ CalculateRefundResponse: armreservations.CalculateRefundResponse{ Properties: &armreservations.RefundResponseProperties{ - SessionID: &sessID, + SessionID: &sessID, + BillingRefundAmount: &armreservations.Price{Amount: toPtr(10.0), CurrencyCode: toPtr("USD")}, }, }, }, @@ -316,7 +318,7 @@ func TestRevokePurchase_AzureReturnClientError(t *testing.T) { r := armReservationRecord() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) // 500 is not a client error -- expect wrapped error, not ClientError. _, isClient := IsClientError(err) @@ -372,7 +374,7 @@ func TestRevokePurchase_EmptyReservationIDRejected(t *testing.T) { r := armReservationRecord() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "", confirmedQuote(10, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -967,7 +969,7 @@ func TestCallAzureReturn_TOCTOUDivergenceRejectedWith422(t *testing.T) { returnClient := &stubReturnClient{} h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", &userConfirmed) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(userConfirmed, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -993,7 +995,7 @@ func TestCallAzureReturn_TOCTOUWithinEpsilonSucceeds(t *testing.T) { returnClient := &stubReturnClient{resp: armreservations.ReturnClientPostResponse{}} h := &Handler{config: mockStore} - result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", &userConfirmed) + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(userConfirmed, "USD")) require.NoError(t, err) m, ok := result.(*revokePurchaseResult) require.True(t, ok) @@ -1020,7 +1022,7 @@ func TestCallAzureReturn_AuditRowPopulatedWithQuote(t *testing.T) { ).Return(nil) h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(42.50, "EUR")) require.NoError(t, err) mockStore.AssertExpectations(t) } @@ -1094,7 +1096,7 @@ func TestCallAzureReturn_MarkPurchaseRevokedFailAllRetries(t *testing.T) { Return(errors.New("db down")).Times(4) h := &Handler{config: mockStore} - result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.NoError(t, err, "a DB failure after Azure success should not surface as an error") pending, ok := result.(*revokeReconcilePendingResult) @@ -1202,7 +1204,7 @@ func TestCallAzureReturn_JustOutsideSafetyMargin(t *testing.T) { mockStore.On("MarkPurchaseRevoked", ctx, r.PurchaseID, mock.AnythingOfType("time.Time"), "direct-api", "", mock.Anything, mock.Anything).Return(nil) h := &Handler{config: mockStore} - result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(50, "USD")) require.NoError(t, err) m, ok := result.(*revokePurchaseResult) require.True(t, ok) @@ -1267,7 +1269,7 @@ func TestCallAzureReturn_RefundPolicyViolatedReturns422WindowEdge(t *testing.T) r := armReservationRecord() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(30, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -1295,7 +1297,7 @@ func TestCallAzureReturn_TransientError_ClearsInFlight(t *testing.T) { mockStore.On("ClearRevocationInFlight", ctx, r.PurchaseID).Return(nil).Once() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) // Must not be a ClientError -- transient errors surface as a 500 from the caller. _, isClientErr := IsClientError(err) @@ -1321,7 +1323,7 @@ func TestCallAzureReturn_ClientError_ClearsInFlight(t *testing.T) { mockStore.On("ClearRevocationInFlight", ctx, r.PurchaseID).Return(nil).Once() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -1346,7 +1348,7 @@ func TestCallAzureReturn_WindowEdge_ClearsInFlight(t *testing.T) { mockStore.On("ClearRevocationInFlight", ctx, r.PurchaseID).Return(nil).Once() h := &Handler{config: mockStore} - _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok) @@ -1371,7 +1373,7 @@ func TestCallAzureReturn_Success_DoesNotClearInFlight(t *testing.T) { "direct-api", "", mock.Anything, mock.Anything).Return(nil).Once() h := &Handler{config: mockStore} - result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", nil) + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.NoError(t, err) rr, ok := result.(*revokePurchaseResult) require.True(t, ok) @@ -1604,7 +1606,7 @@ func TestRevokeAzurePurchase_Success(t *testing.T) { }, } - result, err := h.revokeAzurePurchase(ctx, r, nil) + result, err := h.revokeAzurePurchase(ctx, r, confirmedQuote(12.34, "USD")) require.NoError(t, err) m, ok := result.(*revokePurchaseResult) require.True(t, ok) @@ -1636,7 +1638,7 @@ func TestRevokeAzurePurchase_ClientFactoryError(t *testing.T) { }, } - _, err := h.revokeAzurePurchase(ctx, r, nil) + _, err := h.revokeAzurePurchase(ctx, r, confirmedQuote(10, "USD")) require.Error(t, err) assert.ErrorContains(t, err, "revoke azure:") assert.ErrorIs(t, err, factoryErr) diff --git a/internal/api/router.go b/internal/api/router.go index 03bc2552e..c7c04d9ab 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -185,7 +185,7 @@ func (r *Router) registerRoutes() { // GET /api/purchases/{id}/revoke/calculate: returns the Azure refund quote // (amount + currency) for the two-step quote-then-confirm revoke UX // (issue #290 Finding #4). No state mutation; result used to populate - // expected_refund_amount in the POST /revoke body. + // expected_refund_amount + expected_refund_currency in the POST /revoke body. {PathPrefix: "/api/purchases/", PathSuffix: "/revoke/calculate", Method: "GET", Handler: r.calculateRevokeHandler, Auth: AuthUser}, // RI Marketplace listing (issue #292). Session-authed only; the