From badc86a44cd2c26aef7bb91fa1e7ea45b6205a5c Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 16 Oct 2026 16:01:43 +0100 Subject: [PATCH 1/2] fix(approval): let email-link approvers load purchase details without view:purchases The approval deep link's confirm modal loads GET /api/purchases/{id}, which needs view:purchases and account scope. The email-link approve is authorized by the account's contact_email instead, so a contact approver without view:purchases could approve but never see the modal. Add POST /api/purchases/approve/{id}/details, authorized like the approve it precedes: session and CSRF before the lookup; then either view:purchases with the execution in scope, or a valid approval token AND a session email that is the execution's contact approver. Every final denial answers as 404 "execution not found", like a missing ID (#597). The deep link now uses this read; History is unchanged. An expired token (410) is treated as permanent in the toast. Closes #529 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/purchases-deeplink.test.ts | 21 +- frontend/src/api/index.ts | 1 + frontend/src/api/purchases.ts | 14 + frontend/src/approval-details.ts | 17 +- frontend/src/purchases-deeplink.ts | 2 +- .../history-action-confirmation.spec.ts | 4 +- .../api/handler_purchases_approval_details.go | 89 +++++++ ...handler_purchases_approval_details_test.go | 247 ++++++++++++++++++ internal/api/router.go | 11 + 9 files changed, 393 insertions(+), 13 deletions(-) create mode 100644 internal/api/handler_purchases_approval_details.go create mode 100644 internal/api/handler_purchases_approval_details_test.go diff --git a/frontend/src/__tests__/purchases-deeplink.test.ts b/frontend/src/__tests__/purchases-deeplink.test.ts index ee04a444..3bdc44a8 100644 --- a/frontend/src/__tests__/purchases-deeplink.test.ts +++ b/frontend/src/__tests__/purchases-deeplink.test.ts @@ -6,6 +6,7 @@ jest.mock('../api', () => ({ getPurchaseDetails: jest.fn(), + getApprovalDetails: jest.fn(), listAccounts: jest.fn().mockResolvedValue([]), getDeploymentInfo: jest.fn().mockResolvedValue({}), })); @@ -29,8 +30,8 @@ describe('handlePurchaseDeeplink approve (issue #247)', () => { afterEach(() => jest.restoreAllMocks()); it.each([ - ['the details fetch fails', () => (api.getPurchaseDetails as jest.Mock).mockRejectedValue(new Error('403 Forbidden'))], - ['the details carry no recommendations', () => (api.getPurchaseDetails as jest.Mock).mockResolvedValue({ + ['the details fetch fails', () => (api.getApprovalDetails as jest.Mock).mockRejectedValue(new Error('403 Forbidden'))], + ['the details carry no recommendations', () => (api.getApprovalDetails as jest.Mock).mockResolvedValue({ execution_id: EXEC_ID, status: 'pending', total_upfront_cost: 0, estimated_savings: 0, recommendations: [], })], ])('does not offer Approve when %s', async (_label, arrange) => { @@ -50,8 +51,9 @@ describe('handlePurchaseDeeplink approve (issue #247)', () => { it.each([ [403, 'forbidden', 'view:purchases'], [404, 'execution not found', 'not found or is outside your account access'], + [410, 'approval token has expired', 'The approval link has expired.'], ])('details fetch failing with %s is permanent: no retry advice, keeps server message', async (status, message, wording) => { - (api.getPurchaseDetails as jest.Mock).mockRejectedValue(Object.assign(new Error(message), { status })); + (api.getApprovalDetails as jest.Mock).mockRejectedValue(Object.assign(new Error(message), { status })); await expect(handlePurchaseDeeplink()).resolves.toBe(true); @@ -66,7 +68,7 @@ describe('handlePurchaseDeeplink approve (issue #247)', () => { [401, 'unauthorized'], [undefined, 'Failed to fetch'], ])('details fetch failing with %s keeps the retry path', async (status, message) => { - (api.getPurchaseDetails as jest.Mock).mockRejectedValue(Object.assign(new Error(message), { status })); + (api.getApprovalDetails as jest.Mock).mockRejectedValue(Object.assign(new Error(message), { status })); await expect(handlePurchaseDeeplink()).resolves.toBe(true); @@ -77,7 +79,7 @@ describe('handlePurchaseDeeplink approve (issue #247)', () => { }); it('shows the upfront amount and approves when details load', async () => { - (api.getPurchaseDetails as jest.Mock).mockResolvedValue({ + (api.getApprovalDetails as jest.Mock).mockResolvedValue({ execution_id: EXEC_ID, status: 'pending', total_upfront_cost: 1200, estimated_savings: 10, recommendations: [{ id: 'r1', provider: 'aws', service: 'ec2', region: 'us-east-1', resource_type: 'm5.large', count: 1, term: 1, payment: 'all-upfront', upfront_cost: 1200, monthly_cost: 0, savings: 10, selected: true, purchased: false }], @@ -93,6 +95,15 @@ describe('handlePurchaseDeeplink approve (issue #247)', () => { expect(apiRequest).toHaveBeenCalledWith(`/purchases/approve/${EXEC_ID}`, expect.objectContaining({ method: 'POST' })); }); + + it('loads the details through the token-authorized read, not the view:purchases route (issue #529)', async () => { + (api.getApprovalDetails as jest.Mock).mockRejectedValue(Object.assign(new Error('execution not found'), { status: 404 })); + + await handlePurchaseDeeplink(); + + expect(api.getApprovalDetails).toHaveBeenCalledWith(EXEC_ID, 'tok-xyz'); + expect(api.getPurchaseDetails).not.toHaveBeenCalled(); + }); }); describe('parsePurchaseDeeplink', () => { diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 154bbf48..20a6ebb4 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -135,6 +135,7 @@ export { export { executePurchase, getPurchaseDetails, + getApprovalDetails, cancelPurchase, retryPurchase, approvePurchase, diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts index 8cc9a4c6..7be919b5 100644 --- a/frontend/src/api/purchases.ts +++ b/frontend/src/api/purchases.ts @@ -50,6 +50,20 @@ export async function getPurchaseDetails(executionId: string): Promise(`/purchases/${executionId}`); } +/** + * Get purchase details on the approval email's deep link (issue #529). The + * backend authorizes the read like the approve it precedes: a session with + * view:purchases in scope, or the email token plus a session email that is + * the account's contact approver. The token goes in the body, not the URL + * (issue #398). + */ +export async function getApprovalDetails(executionId: string, token: string): Promise { + return apiRequest(`/purchases/approve/${encodeURIComponent(executionId)}/details`, { + method: 'POST', + body: JSON.stringify({ token }), + }); +} + /** * Cancel a scheduled purchase */ diff --git a/frontend/src/approval-details.ts b/frontend/src/approval-details.ts index 58726699..b9604ce3 100644 --- a/frontend/src/approval-details.ts +++ b/frontend/src/approval-details.ts @@ -316,19 +316,23 @@ export function computeEffectiveSavingsPct(rec: Recommendation): number | null { } /** - * Network-fetching wrapper. Fetches `/api/purchases/{id}` + the + * Network-fetching wrapper. Fetches `/api/purchases/{id}` (or, with an + * approval token, `/api/purchases/approve/{id}/details`) + the * accounts list in parallel, builds the rich body, and returns it. * Rejects when the purchase details fail to load or carry no * recommendations; callers must then refuse to offer Approve. */ -export async function buildApprovalDetailsBody(executionId: string): Promise { +export async function buildApprovalDetailsBody(executionId: string, approvalToken?: string): Promise { // Fetch purchase details, account list, and public info in parallel. // listAccounts and getPublicInfo are caught inline so the modal still // renders the full details when either endpoint is unreachable; the // per-rec table degrades gracefully. console.warn keeps failures // traceable rather than silently dropping them. const [details, accounts, deploymentInfo] = await Promise.all([ - api.getPurchaseDetails(executionId), + // The email deep link carries a token, so an approver authorized by + // the account's contact email can load the details without + // view:purchases (issue #529). + approvalToken ? api.getApprovalDetails(executionId, approvalToken) : api.getPurchaseDetails(executionId), api.listAccounts().catch((err) => { console.warn('Failed to load accounts for approval modal — falling back to UUID-prefixed labels:', err); return [] as CloudAccount[]; @@ -351,10 +355,10 @@ export async function buildApprovalDetailsBody(executionId: string): Promise { let body: string | HTMLElement; if (dl.action === 'approve') { try { - body = await buildApprovalDetailsBody(dl.id); + body = await buildApprovalDetailsBody(dl.id, dl.token); } catch (err) { console.error('Failed to load purchase details for approval:', err); showToast({ diff --git a/frontend/tests-e2e/history-action-confirmation.spec.ts b/frontend/tests-e2e/history-action-confirmation.spec.ts index dc224748..7039c235 100644 --- a/frontend/tests-e2e/history-action-confirmation.spec.ts +++ b/frontend/tests-e2e/history-action-confirmation.spec.ts @@ -137,10 +137,10 @@ test('approve is not offered when the purchase details cannot be loaded (issue # await expect.poll(() => fixture.posts.length).toBe(1); }); -for (const [status, wording] of [[403, 'view:purchases'], [404, 'outside your account access']] as const) { +for (const [status, wording] of [[403, 'view:purchases'], [404, 'outside your account access'], [410, 'approval link has expired']] as const) { test(`approval link with details ${status} shows permanent wording without retry advice (issue #529)`, async ({ page }) => { const fixture = await historyFixture(page, 'approve'); - await page.route(`**/api/purchases/${ID}`, route => route.fulfill({ status, json: { error: 'denied by server' } })); + await page.route(`**/api/purchases/approve/${ID}/details`, route => route.fulfill({ status, json: { error: 'denied by server' } })); await page.goto(`/purchases/approve/${ID}?token=tok`); const alert = page.getByRole('alert'); await expect(alert).toContainText('cannot be approved without showing the amount'); diff --git a/internal/api/handler_purchases_approval_details.go b/internal/api/handler_purchases_approval_details.go new file mode 100644 index 00000000..fce9317c --- /dev/null +++ b/internal/api/handler_purchases_approval_details.go @@ -0,0 +1,89 @@ +package api + +import ( + "context" + "fmt" + + "github.com/aws/aws-lambda-go/events" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/purchase" +) + +// getApprovalDetails answers POST /api/purchases/approve/{id}/details, the +// read the approval email's deep link makes to show the amount before the +// user confirms (issue #529). GET /api/purchases/{id} needs view:purchases +// and account scope, but the email-link approve is authorized by the +// per-account contact_email instead, so a contact approver without +// view:purchases could approve yet not load the details the modal requires. +// +// It is authorized exactly like the approve it precedes, minus the write: +// - a session (401) and CSRF (403) are required before the lookup +// (requireSessionBeforeLookup, issues #435 and #597); +// - a session holding view:purchases with the execution in scope gets the +// same response GET /api/purchases/{id} would give it; +// - otherwise the request needs BOTH a valid approval token for this +// execution AND a session email that is one of its contact approvers. +// +// Every final denial answers like a missing execution (issue #597). POST +// carries the token in the body so it stays out of access logs (issue #398). +func (h *Handler) getApprovalDetails(ctx context.Context, req *events.LambdaFunctionURLRequest, execID, token string) (any, error) { + if err := validateUUID(execID); err != nil { + return nil, err + } + if err := h.requireSessionBeforeLookup(ctx, req); err != nil { + return nil, err + } + execution, err := h.getExecutionOr404(ctx, execID) + if err != nil { + return nil, err + } + if err := h.authorizeApprovalDetailsRead(ctx, req, execution, token); err != nil { + return nil, concealExecutionDenial(err) + } + return buildPurchaseDetailsResponse(execution, h.executionPlanName(ctx, execution)), nil +} + +// authorizeApprovalDetailsRead returns nil when the caller may read the +// execution's details on the approval deep link. A session view:purchases +// denial or scope miss falls through to the token branch only when a token is +// present (fallsThroughToToken); a wrapped backend failure propagates. +func (h *Handler) authorizeApprovalDetailsRead(ctx context.Context, req *events.LambdaFunctionURLRequest, execution *config.PurchaseExecution, token string) error { + err := h.authorizeSessionViewExecution(ctx, h.tryGetSession(ctx, req), execution) + if err == nil || !fallsThroughToToken(err, token) { + return err + } + if _, err := h.authorizeApprovalAction(ctx, req, execution); err != nil { + return err + } + if err := purchase.ValidateApprovalToken(execution, token); err != nil { + return tokenActionError(err) + } + return nil +} + +// authorizeSessionViewExecution applies GET /api/purchases/{id}'s gate: +// view:purchases, then the account scope (errNotFound on a miss). +func (h *Handler) authorizeSessionViewExecution(ctx context.Context, session *Session, execution *config.PurchaseExecution) error { + if _, err := h.requireSessionPermission(ctx, session, "view", "purchases"); err != nil { + return err + } + scope, err := h.getAccountScope(ctx, session) + if err != nil { + return fmt.Errorf("failed to get allowed accounts: %w", err) + } + return h.requireExecutionInScope(ctx, scope, execution, nil) +} + +// executionPlanName returns the execution's plan name, or "" when it has no +// plan or the plan cannot be read (the name is display-only). +func (h *Handler) executionPlanName(ctx context.Context, execution *config.PurchaseExecution) string { + if execution.PlanID == "" { + return "" + } + plan, err := h.config.GetPurchasePlan(ctx, execution.PlanID) + if err != nil || plan == nil { + return "" + } + return plan.Name +} diff --git a/internal/api/handler_purchases_approval_details_test.go b/internal/api/handler_purchases_approval_details_test.go new file mode 100644 index 00000000..2a35a4ef --- /dev/null +++ b/internal/api/handler_purchases_approval_details_test.go @@ -0,0 +1,247 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// The approval deep link's confirm modal loads the purchase details before it +// offers Approve. These tests drive POST /api/purchases/approve/{id}/details +// through HandleRequest with the oracle fixture: execution in acct-1, contact +// approver contact@example.com, created by someone else (issue #529). + +func approvalDetailsRequest(method, token, csrf string) *events.LambdaFunctionURLRequest { + headers := map[string]string{"authorization": "Bearer sess-tok"} + if csrf != "" { + headers["x-csrf-token"] = csrf + } + body := "" + if token != "" { + b, _ := json.Marshal(map[string]string{"token": token}) + body = string(b) + headers["content-type"] = "application/json" + } + return &events.LambdaFunctionURLRequest{ + Headers: headers, + Body: body, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{ + Method: method, + Path: "/api/purchases/approve/" + tokenErrExecID + "/details", + }, + }, + } +} + +func viewOnlyCaller(email string) oracleCaller { + return oracleCaller{ + grant: func(m *MockAuthService) { + m.grantPermissions([]auth.Permission{{Action: auth.ActionView, Resource: auth.ResourcePurchases}}) + }, + email: email, + sendCSRF: true, + } +} + +func contactCaller() oracleCaller { + return oracleCaller{grant: func(m *MockAuthService) { m.grantPermissions(nil) }, email: "contact@example.com", sendCSRF: true} +} + +func pendingApprovalExec(expiresAt time.Time) *config.PurchaseExecution { + return &config.PurchaseExecution{ + Status: "pending", + ApprovalToken: config.HashApprovalToken(tokenErrRawToken), + ApprovalTokenExpiresAt: &expiresAt, + TotalUpfrontCost: 100, + } +} + +func callApprovalDetails(t *testing.T, h *Handler, caller oracleCaller, token string) *events.LambdaFunctionURLResponse { + t.Helper() + csrf := "" + if caller.sendCSRF { + csrf = oracleCSRF + } + resp, err := h.HandleRequest(context.Background(), approvalDetailsRequest("POST", token, csrf)) + require.NoError(t, err) + return resp +} + +// assertReadOnly fails if the details read changed execution state. +func assertReadOnly(t *testing.T, h *Handler) { + t.Helper() + store := h.config.(*MockConfigStore) + store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} + +// A contact approver without view:purchases is denied by GET +// /api/purchases/{id}, but can read the details on the approval link with +// the email token (issue #529). +func TestHandleRequest_ApprovalDetails_ContactApproverWithoutViewPurchases(t *testing.T) { + future := time.Now().Add(time.Hour) + caller := contactCaller() + + // The generic details route still refuses this caller: nothing changed there. + h := oracleHandler(t, caller, pendingApprovalExec(future), false) + getReq := approvalDetailsRequest("GET", "", "") + getReq.RequestContext.HTTP.Path = "/api/purchases/" + tokenErrExecID + getResp, err := h.HandleRequest(context.Background(), getReq) + require.NoError(t, err) + require.Equal(t, 403, getResp.StatusCode, getResp.Body) + + h = oracleHandler(t, caller, pendingApprovalExec(future), false) + resp := callApprovalDetails(t, h, caller, tokenErrRawToken) + require.Equal(t, 200, resp.StatusCode, resp.Body) + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(resp.Body), &body)) + assert.Equal(t, tokenErrExecID, body["execution_id"]) + assert.InDelta(t, 100.0, body["total_upfront_cost"], 0.001) + recs, ok := body["recommendations"].([]any) + require.True(t, ok, "recommendations missing: %s", resp.Body) + assert.Len(t, recs, 1) + assertReadOnly(t, h) +} + +// The token alone, or the contact email alone, never authorizes the read. +func TestHandleRequest_ApprovalDetails_TokenAndContactBothRequired(t *testing.T) { + future := time.Now().Add(time.Hour) + past := time.Now().Add(-time.Hour) + cases := []struct { + name string + caller oracleCaller + exec *config.PurchaseExecution + token string + wantCode int + }{ + {"contact, wrong token", contactCaller(), pendingApprovalExec(future), "wrong", 404}, + {"contact, no token", contactCaller(), pendingApprovalExec(future), "", 404}, + {"contact, execution has no token", contactCaller(), &config.PurchaseExecution{Status: "pending"}, tokenErrRawToken, 404}, + {"contact, expired token", contactCaller(), pendingApprovalExec(past), tokenErrRawToken, 410}, + {"not the contact, valid token", oracleCaller{grant: func(m *MockAuthService) { m.grantPermissions(nil) }, email: "outsider@example.com", sendCSRF: true}, pendingApprovalExec(future), tokenErrRawToken, 404}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := oracleHandler(t, tc.caller, tc.exec, false) + resp := callApprovalDetails(t, h, tc.caller, tc.token) + assert.Equal(t, tc.wantCode, resp.StatusCode, resp.Body) + assert.NotContains(t, resp.Body, "recommendations") + if tc.wantCode == 404 { + assert.Contains(t, resp.Body, "execution not found") + } + }) + } +} + +// A session with view:purchases in scope reads the details without a token, +// exactly as GET /api/purchases/{id} allows; out of scope it needs the +// contact gate like everyone else. +func TestHandleRequest_ApprovalDetails_ViewPurchasesSession(t *testing.T) { + future := time.Now().Add(time.Hour) + + t.Run("in scope, no token", func(t *testing.T) { + caller := viewOnlyCaller("viewer@example.com") + h := oracleHandler(t, caller, pendingApprovalExec(future), false) + resp := callApprovalDetails(t, h, caller, "") + assert.Equal(t, 200, resp.StatusCode, resp.Body) + }) + t.Run("scoped out, not the contact, valid token", func(t *testing.T) { + caller := oracleCaller{grant: func(m *MockAuthService) { m.grantScoped("other-acct") }, email: "outsider@example.com", sendCSRF: true} + h := oracleHandler(t, caller, pendingApprovalExec(future), false) + resp := callApprovalDetails(t, h, caller, tokenErrRawToken) + assert.Equal(t, 404, resp.StatusCode, resp.Body) + }) + t.Run("scoped out, contact, valid token", func(t *testing.T) { + caller := oracleCaller{grant: func(m *MockAuthService) { m.grantScoped("other-acct") }, email: "contact@example.com", sendCSRF: true} + h := oracleHandler(t, caller, pendingApprovalExec(future), false) + resp := callApprovalDetails(t, h, caller, tokenErrRawToken) + assert.Equal(t, 200, resp.StatusCode, resp.Body) + }) +} + +// No session is 401 and a session without CSRF is 403, both before the +// execution is read (issues #435, #597). +func TestHandleRequest_ApprovalDetails_SessionAndCSRFBeforeLookup(t *testing.T) { + future := time.Now().Add(time.Hour) + + t.Run("no session", func(t *testing.T) { + h := oracleHandler(t, contactCaller(), pendingApprovalExec(future), false) + req := approvalDetailsRequest("POST", tokenErrRawToken, oracleCSRF) + delete(req.Headers, "authorization") + resp, err := h.HandleRequest(context.Background(), req) + require.NoError(t, err) + assert.Equal(t, 401, resp.StatusCode, resp.Body) + h.config.(*MockConfigStore).AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) + }) + t.Run("contact without CSRF", func(t *testing.T) { + caller := contactCaller() + caller.sendCSRF = false + h := oracleHandler(t, caller, pendingApprovalExec(future), false) + resp := callApprovalDetails(t, h, caller, tokenErrRawToken) + assert.Equal(t, 403, resp.StatusCode, resp.Body) + assert.Contains(t, resp.Body, "CSRF") + h.config.(*MockConfigStore).AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) + }) +} + +// A signed-in caller who may not read the details gets the same status, +// headers and body for an existing and a missing execution (issue #597). +func TestHandleRequest_ApprovalDetails_SignedInCannotTellMissingFromExisting(t *testing.T) { + future := time.Now().Add(time.Hour) + for _, status := range []string{"pending", "completed", "orphan"} { + for _, token := range []string{"", tokenErrRawToken, "wrong"} { + for name, caller := range oracleCallers() { + if name == "contact/no-csrf" && token == "" { + continue + } + t.Run(status+"/token="+token+"/"+name, func(t *testing.T) { + orphan := status == "orphan" + exec := pendingApprovalExec(future) + if !orphan { + exec.Status = status + } + existingH := oracleHandler(t, caller, exec, orphan) + existing := callApprovalDetails(t, existingH, caller, token) + missing := callApprovalDetails(t, oracleHandler(t, caller, nil, false), caller, token) + + assert.Contains(t, []int{403, 404}, existing.StatusCode, existing.Body) + assert.Equal(t, missing.StatusCode, existing.StatusCode, "existing: %s / missing: %s", existing.Body, missing.Body) + assert.Equal(t, missing.Body, existing.Body) + assert.Equal(t, missing.Headers, existing.Headers) + assertReadOnly(t, existingH) + }) + } + } + } +} + +// The details route must not shadow the approve route it shares a prefix +// with: POST /api/purchases/approve/{id} still reaches approvePurchase. +func TestRouter_ApprovalDetailsDoesNotShadowApprove(t *testing.T) { + r := &Router{} + r.registerRoutes() + match := func(path string) Route { + for _, route := range r.routes { + if r.matches(route, "POST", path) { + return route + } + } + t.Fatalf("no route for %s", path) + return Route{} + } + details := match("/api/purchases/approve/" + tokenErrExecID + "/details") + assert.Equal(t, "/details", details.PathSuffix) + assert.Equal(t, tokenErrExecID, r.extractParams(details, "/api/purchases/approve/"+tokenErrExecID+"/details")["id"]) + approve := match("/api/purchases/approve/" + tokenErrExecID) + assert.Empty(t, approve.PathSuffix) +} diff --git a/internal/api/router.go b/internal/api/router.go index cf255b70..f325c9fe 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -162,6 +162,10 @@ func (r *Router) registerRoutes() { // cancel it. The emails link to the SPA deep links // /purchases/{approve,cancel}/{id}?token=..., which POST the token // after the user confirms (issues #549, #578). + // Read-only details for the approval deep link's confirm modal + // (issue #529). Must precede the approve route, which shares its + // prefix. POST so the token travels in the body (issue #398). + {PathPrefix: "/api/purchases/approve/", PathSuffix: "/details", Method: "POST", Handler: r.approvalDetailsHandler, Auth: AuthPublic}, {PathPrefix: "/api/purchases/approve/", Method: "POST", Handler: r.approvePurchaseHandler, Auth: AuthPublic}, {PathPrefix: "/api/purchases/cancel/", Method: "POST", Handler: r.cancelPurchaseHandler, Auth: AuthPublic}, // Revoke a completed purchase via email one-click link (issue #291). @@ -605,6 +609,13 @@ func (r *Router) approvePurchaseHandler(ctx context.Context, req *events.LambdaF return r.h.approvePurchase(ctx, req, params["id"], token) } +func (r *Router) approvalDetailsHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + if err := r.h.checkRateLimit(ctx, req, "approve_cancel_public"); err != nil { + return nil, err + } + return r.h.getApprovalDetails(ctx, req, params["id"], resolveApprovalToken(req)) +} + func (r *Router) cancelPurchaseHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { if err := r.h.checkRateLimit(ctx, req, "approve_cancel_public"); err != nil { return nil, err From 1366784f51bc9d642ac5da50b941a606759e430d Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 16 Oct 2026 16:50:42 +0100 Subject: [PATCH 2/2] test(api): bind the mock store to a local before AssertNotCalled TestNoUnfailableMockAssertions rejects AssertNotCalled on a non-plain receiver (a type assertion), since it can't prove the assertion can fail. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_purchases_approval_details_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/api/handler_purchases_approval_details_test.go b/internal/api/handler_purchases_approval_details_test.go index 2a35a4ef..10eaa6dc 100644 --- a/internal/api/handler_purchases_approval_details_test.go +++ b/internal/api/handler_purchases_approval_details_test.go @@ -181,7 +181,8 @@ func TestHandleRequest_ApprovalDetails_SessionAndCSRFBeforeLookup(t *testing.T) resp, err := h.HandleRequest(context.Background(), req) require.NoError(t, err) assert.Equal(t, 401, resp.StatusCode, resp.Body) - h.config.(*MockConfigStore).AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) + store := h.config.(*MockConfigStore) + store.AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) }) t.Run("contact without CSRF", func(t *testing.T) { caller := contactCaller() @@ -190,7 +191,8 @@ func TestHandleRequest_ApprovalDetails_SessionAndCSRFBeforeLookup(t *testing.T) resp := callApprovalDetails(t, h, caller, tokenErrRawToken) assert.Equal(t, 403, resp.StatusCode, resp.Body) assert.Contains(t, resp.Body, "CSRF") - h.config.(*MockConfigStore).AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) + store := h.config.(*MockConfigStore) + store.AssertNotCalled(t, "GetExecutionByID", mock.Anything, mock.Anything) }) }