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