Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions frontend/src/__tests__/purchases-deeplink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

jest.mock('../api', () => ({
getPurchaseDetails: jest.fn(),
getApprovalDetails: jest.fn(),
listAccounts: jest.fn().mockResolvedValue([]),
getDeploymentInfo: jest.fn().mockResolvedValue({}),
}));
Expand All @@ -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) => {
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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 }],
Expand All @@ -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', () => {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export {
export {
executePurchase,
getPurchaseDetails,
getApprovalDetails,
cancelPurchase,
retryPurchase,
approvePurchase,
Expand Down
14 changes: 14 additions & 0 deletions frontend/src/api/purchases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,20 @@ export async function getPurchaseDetails(executionId: string): Promise<PurchaseD
return apiRequest<PurchaseDetails>(`/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<PurchaseDetails> {
return apiRequest<PurchaseDetails>(`/purchases/approve/${encodeURIComponent(executionId)}/details`, {
method: 'POST',
body: JSON.stringify({ token }),
});
}

/**
* Cancel a scheduled purchase
*/
Expand Down
17 changes: 12 additions & 5 deletions frontend/src/approval-details.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement> {
export async function buildApprovalDetailsBody(executionId: string, approvalToken?: string): Promise<HTMLElement> {
// 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[];
Expand All @@ -351,10 +355,10 @@ export async function buildApprovalDetailsBody(executionId: string): Promise<HTM
return renderApprovalDetailsBody(details, accountsById, hostAWSAccountID);
}

/** True for a 403/404 on the details fetch: retrying cannot succeed. */
/** True for a 403/404/410 on the details fetch: retrying cannot succeed. */
export function isPermanentDetailsError(err: unknown): boolean {
const status = (err as { status?: number } | null)?.status;
return status === 403 || status === 404;
return status === 403 || status === 404 || status === 410;
}

export function approvalDetailsUnavailableMessage(err: unknown): string {
Expand All @@ -367,5 +371,8 @@ export function approvalDetailsUnavailableMessage(err: unknown): string {
if (status === 404) {
return `${base} The purchase was not found or is outside your account access.`;
}
if (status === 410) {
return `${base} The approval link has expired.`;
}
return base;
}
2 changes: 1 addition & 1 deletion frontend/src/purchases-deeplink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export async function handlePurchaseDeeplink(): Promise<boolean> {
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({
Expand Down
4 changes: 2 additions & 2 deletions frontend/tests-e2e/history-action-confirmation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
89 changes: 89 additions & 0 deletions internal/api/handler_purchases_approval_details.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading
Loading