From 7f3ccad7aac7c6e2ae016f0a392f5f0143366b01 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 6 Oct 2026 11:51:12 +0200 Subject: [PATCH 1/2] fix(api): return 422 from the Azure revoke quote when the refund amount or currency is missing calculateAzureRevoke filled 0 and an empty currency when Azure omitted BillingRefundAmount, so the confirm dialog showed a fabricated refund. extractAzureRefundQuote now returns a nil amount instead of 0, the quote endpoint answers 422, and azureCalculateRefund reuses the same extractor instead of duplicating the parsing. --- internal/api/handler_purchases_revoke.go | 53 ++++--- .../handler_purchases_revoke_quote_test.go | 131 ++++++++++++++++++ 2 files changed, 155 insertions(+), 29 deletions(-) create mode 100644 internal/api/handler_purchases_revoke_quote_test.go diff --git a/internal/api/handler_purchases_revoke.go b/internal/api/handler_purchases_revoke.go index eceb43431..125bc536e 100644 --- a/internal/api/handler_purchases_revoke.go +++ b/internal/api/handler_purchases_revoke.go @@ -496,8 +496,11 @@ func (h *Handler) calculateAzureRevoke(ctx context.Context, req *events.LambdaFu } refundAmount, refundCurrency := extractAzureRefundQuote(calcResp) + if refundAmount == nil || strings.TrimSpace(refundCurrency) == "" { + return nil, NewClientError(422, "Azure returned no refund amount or currency for this reservation; cannot quote a refund, contact Azure Support to request one") + } return &revokeQuoteResult{ - RefundAmount: refundAmount, + RefundAmount: *refundAmount, RefundCurrency: refundCurrency, QuotedAt: time.Now().UTC().Format(time.RFC3339), }, nil @@ -575,20 +578,24 @@ func azureRevokeWindowAndIDs(record *config.PurchaseHistoryRecord) (string, stri } // extractAzureRefundQuote pulls the refund amount and currency out of a -// CalculateRefund response, guarding every nil pointer in the chain. Returns -// zero values when the response carries no billing-refund amount. -func extractAzureRefundQuote(resp armreservations.CalculateRefundClientPostResponse) (float64, string) { //nolint:gocritic // unnamedResult: return names would conflict with body locals - var refundAmount float64 - var refundCurrency string - if resp.Properties != nil && resp.Properties.BillingRefundAmount != nil { - if resp.Properties.BillingRefundAmount.Amount != nil { - refundAmount = *resp.Properties.BillingRefundAmount.Amount - } - if resp.Properties.BillingRefundAmount.CurrencyCode != nil { - refundCurrency = *resp.Properties.BillingRefundAmount.CurrencyCode - } +// CalculateRefund response, guarding every nil pointer in the chain. A missing +// amount is returned as nil and a missing currency as "", never as zero, so +// callers can tell "Azure quoted nothing" from "Azure quoted 0". +func extractAzureRefundQuote(resp armreservations.CalculateRefundClientPostResponse) (*float64, string) { //nolint:gocritic // unnamedResult: return names would conflict with body locals + if resp.Properties == nil || resp.Properties.BillingRefundAmount == nil { + return nil, "" + } + price := resp.Properties.BillingRefundAmount + var amount *float64 + if price.Amount != nil { + v := *price.Amount + amount = &v } - return refundAmount, refundCurrency + var currency string + if price.CurrencyCode != nil { + currency = *price.CurrencyCode + } + return amount, currency } // revokeAzurePurchase handles Azure reservation returns via the Azure @@ -787,22 +794,10 @@ func (h *Handler) azureCalculateRefund(ctx context.Context, calcClient azureCalc } var sessionID string - var calcRefundAmount *float64 - var calcRefundCurrency string - if calcResp.Properties != nil { - if calcResp.Properties.SessionID != nil { - sessionID = *calcResp.Properties.SessionID - } - if calcResp.Properties.BillingRefundAmount != nil { - if calcResp.Properties.BillingRefundAmount.Amount != nil { - v := *calcResp.Properties.BillingRefundAmount.Amount - calcRefundAmount = &v - } - if calcResp.Properties.BillingRefundAmount.CurrencyCode != nil { - calcRefundCurrency = *calcResp.Properties.BillingRefundAmount.CurrencyCode - } - } + if calcResp.Properties != nil && calcResp.Properties.SessionID != nil { + sessionID = *calcResp.Properties.SessionID } + calcRefundAmount, calcRefundCurrency := extractAzureRefundQuote(calcResp) return sessionID, calcRefundAmount, calcRefundCurrency, nil } diff --git a/internal/api/handler_purchases_revoke_quote_test.go b/internal/api/handler_purchases_revoke_quote_test.go new file mode 100644 index 000000000..d8f211fa7 --- /dev/null +++ b/internal/api/handler_purchases_revoke_quote_test.go @@ -0,0 +1,131 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Issue #447: the quote endpoint must not fabricate a 0 / empty refund when +// Azure omits the amount or currency; it must fail with a 422 instead. + +func quoteResponse(sessionID string, price *armreservations.Price) armreservations.CalculateRefundClientPostResponse { + return armreservations.CalculateRefundClientPostResponse{ + CalculateRefundResponse: armreservations.CalculateRefundResponse{ + Properties: &armreservations.RefundResponseProperties{ + SessionID: &sessionID, + BillingRefundAmount: price, + }, + }, + } +} + +func quoteHandlerRequest(purchaseID string) *events.LambdaFunctionURLRequest { + return &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer tok"}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{ + Method: "GET", + Path: "/api/purchases/" + purchaseID + "/revoke/calculate", + }, + }, + } +} + +func TestHandleRequest_RevokeCalculate_AzureQuote(t *testing.T) { + amount := 75.25 + currency := "USD" + empty := "" + + cases := []struct { + name string + resp armreservations.CalculateRefundClientPostResponse + wantStatus int + wantErr string + }{ + { + name: "complete quote", + resp: quoteResponse("s1", &armreservations.Price{Amount: &amount, CurrencyCode: ¤cy}), + wantStatus: 200, + }, + { + name: "zero amount is a real quote", + resp: quoteResponse("s1", &armreservations.Price{Amount: new(float64), CurrencyCode: ¤cy}), + wantStatus: 200, + }, + { + name: "no billing refund amount", + resp: quoteResponse("s1", nil), + wantStatus: 422, + wantErr: "no refund amount or currency", + }, + { + name: "no properties", + resp: armreservations.CalculateRefundClientPostResponse{}, + wantStatus: 422, + wantErr: "no refund amount or currency", + }, + { + name: "amount missing", + resp: quoteResponse("s1", &armreservations.Price{CurrencyCode: ¤cy}), + wantStatus: 422, + wantErr: "no refund amount or currency", + }, + { + name: "currency missing", + resp: quoteResponse("s1", &armreservations.Price{Amount: &amount}), + wantStatus: 422, + wantErr: "no refund amount or currency", + }, + { + name: "currency blank", + resp: quoteResponse("s1", &armreservations.Price{Amount: &amount, CurrencyCode: &empty}), + wantStatus: 422, + wantErr: "no refund amount or currency", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", mock.Anything, "tok").Return(revokeAdminSession(), nil) + r := armReservationRecord() + mockStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, r.PurchaseID).Return(r, nil) + + h := &Handler{ + config: mockStore, + auth: mockAuth, + azureRevokeFactory: &azureRevokeClientFactory{ + newCredential: func() (azcore.TokenCredential, error) { return nil, nil }, + newCalculateRefundClient: func(azcore.TokenCredential) (azureCalculateRefundClient, error) { + return &stubCalcRefundClient{resp: tc.resp}, nil + }, + newReturnClient: func(azcore.TokenCredential) (azureReturnClient, error) { + return &stubReturnClient{}, nil + }, + }, + } + + resp, err := h.HandleRequest(ctx, quoteHandlerRequest(r.PurchaseID)) + require.NoError(t, err) + require.Equal(t, tc.wantStatus, resp.StatusCode, resp.Body) + if tc.wantErr != "" { + assert.Contains(t, resp.Body, tc.wantErr) + assert.NotContains(t, resp.Body, "refund_amount") + return + } + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(resp.Body), &body)) + assert.Equal(t, "USD", body["refund_currency"]) + }) + } +} From 63e177c03d2253c67896949e03d727c2c3b76203 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 6 Oct 2026 12:00:27 +0200 Subject: [PATCH 2/2] test(api): drop the constant session id parameter from the quote test helper --- .../api/handler_purchases_revoke_quote_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/api/handler_purchases_revoke_quote_test.go b/internal/api/handler_purchases_revoke_quote_test.go index d8f211fa7..b0527fb7b 100644 --- a/internal/api/handler_purchases_revoke_quote_test.go +++ b/internal/api/handler_purchases_revoke_quote_test.go @@ -16,7 +16,8 @@ import ( // Issue #447: the quote endpoint must not fabricate a 0 / empty refund when // Azure omits the amount or currency; it must fail with a 422 instead. -func quoteResponse(sessionID string, price *armreservations.Price) armreservations.CalculateRefundClientPostResponse { +func quoteResponse(price *armreservations.Price) armreservations.CalculateRefundClientPostResponse { + sessionID := "s1" return armreservations.CalculateRefundClientPostResponse{ CalculateRefundResponse: armreservations.CalculateRefundResponse{ Properties: &armreservations.RefundResponseProperties{ @@ -52,17 +53,17 @@ func TestHandleRequest_RevokeCalculate_AzureQuote(t *testing.T) { }{ { name: "complete quote", - resp: quoteResponse("s1", &armreservations.Price{Amount: &amount, CurrencyCode: ¤cy}), + resp: quoteResponse(&armreservations.Price{Amount: &amount, CurrencyCode: ¤cy}), wantStatus: 200, }, { name: "zero amount is a real quote", - resp: quoteResponse("s1", &armreservations.Price{Amount: new(float64), CurrencyCode: ¤cy}), + resp: quoteResponse(&armreservations.Price{Amount: new(float64), CurrencyCode: ¤cy}), wantStatus: 200, }, { name: "no billing refund amount", - resp: quoteResponse("s1", nil), + resp: quoteResponse(nil), wantStatus: 422, wantErr: "no refund amount or currency", }, @@ -74,19 +75,19 @@ func TestHandleRequest_RevokeCalculate_AzureQuote(t *testing.T) { }, { name: "amount missing", - resp: quoteResponse("s1", &armreservations.Price{CurrencyCode: ¤cy}), + resp: quoteResponse(&armreservations.Price{CurrencyCode: ¤cy}), wantStatus: 422, wantErr: "no refund amount or currency", }, { name: "currency missing", - resp: quoteResponse("s1", &armreservations.Price{Amount: &amount}), + resp: quoteResponse(&armreservations.Price{Amount: &amount}), wantStatus: 422, wantErr: "no refund amount or currency", }, { name: "currency blank", - resp: quoteResponse("s1", &armreservations.Price{Amount: &amount, CurrencyCode: &empty}), + resp: quoteResponse(&armreservations.Price{Amount: &amount, CurrencyCode: &empty}), wantStatus: 422, wantErr: "no refund amount or currency", },