fix(purchases): admin / cancel-* sessions bypass contact_email gate on email-link cancel

internal/api/coverage_extras_test.go

@@ -121,6 +121,12 @@ func TestHandler_cancelPurchase_PurchaseError(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// Session has no admin role / cancel permissions → cancelPurchase's
+	// session-authed pre-check falls through to authorizeApprovalAction →
+	// purchase manager invocation, which is what this test's
+	// "cancel failed" assertion exercises.
+	mockAuth.On("HasPermissionAPI", ctx, "", "cancel-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "cancel-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("CancelExecution", ctx, execID, "tok", approver).

internal/api/handler_purchases.go

@@ -304,19 +304,52 @@ func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunction
 		return nil, NewClientError(404, "execution not found")
 	}
 
-	// Two-mode dispatch (issue #46):
-	//   * token != ""  → legacy email-link flow. Token possession proves
-	//     intent; the handler still requires the session email (when one
-	//     exists) to be on the authorised-approver list, then delegates
-	//     to the purchase service which validates the token itself.
-	//   * token == ""  → session-authed dashboard Cancel button. The
-	//     session must carry a permission allowing the cancel:
-	//       - cancel-any:purchases (admin) → any pending execution; or
-	//       - cancel-own:purchases (default user) AND the execution's
-	//         created_by_user_id matches the session UserID.
-	//     Legacy rows with NULL created_by_user_id are reachable only
-	//     via cancel-any (admin) or via the email token already in the
-	//     inbox — they do not become orphaned by this change.
+	// Three-mode dispatch:
+	//   1. Session present AND RBAC-authorized (admin / cancel-any /
+	//      cancel-own match) → session-authed cancel, regardless of
+	//      whether a token is in the URL. Restores parity with the
+	//      History page Cancel button: a user who can cancel from the
+	//      dashboard should also be able to cancel from the email-link
+	//      flow, since the same email-link flow already requires a
+	//      logged-in session before reaching this endpoint (see
+	//      frontend purchases-deeplink.ts). Without this branch, an
+	//      admin clicking Cancel from a notification email about an
+	//      ambient-credentials execution (CloudAccountID == nil → no
+	//      per-account contact_email available) is locked out of an
+	//      action they can perform from the dashboard. The token in
+	//      the URL is informational at that point — the session has
+	//      already authenticated the actor.
+	//   2. token != "" → legacy email-link flow without a qualifying
+	//      session. authorizeApprovalAction enforces the per-account
+	//      contact_email gate from PR #101; the purchase service
+	//      validates the token itself before mutating state. This
+	//      preserves the security model for forwarded email / shared
+	//      inbox / stolen link cases — a non-privileged session falls
+	//      through to this branch.
+	//   3. token == "" → session-authed dashboard Cancel button (issue
+	//      #46). Same path as branch (1) but reached without a URL
+	//      token. cancelPurchaseViaSession runs the cancel-any /
+	//      cancel-own RBAC matrix and rejects sessions without it.
+	if session := h.tryGetSession(ctx, req); session != nil {
+		switch err := h.authorizeSessionCancel(ctx, session, execution); {
+		case err == nil:
+			// Session is RBAC-authorized → run the session-authed cancel.
+			return h.cancelPurchaseViaSession(ctx, req, execution)
+		case isPermissionDenied(err):
+			// Explicit "permission denied" (403) → fall through to the
+			// token branch so the contact_email gate still gets a chance
+			// (a logged-in user without admin / cancel-* may still be
+			// the per-account contact email recipient).
+		default:
+			// Transient failure (auth-service down, HasPermissionAPI
+			// returning a wrapped error, h.auth==nil 500). Propagate
+			// instead of silently widening to the contact_email gate —
+			// a stale auth backend should not mask itself as a 403 about
+			// missing contact emails. CR feedback on PR #216.
+			return nil, err
+		}
+	}
+
 	if token != "" {
 		actor, err := h.authorizeApprovalAction(ctx, req, execution)
 		if err != nil {
@@ -759,18 +792,49 @@ func (h *Handler) authorizeSessionRetry(ctx context.Context, session *Session, e
 // → session-authed call with ?token=…) can record per-user attribution
 // without changing the token-only fallback path used by message workers.
 func (h *Handler) tryResolveActorEmail(ctx context.Context, req *events.LambdaFunctionURLRequest) string {
+	if s := h.tryGetSession(ctx, req); s != nil {
+		return s.Email
+	}
+	return ""
+}
+
+// isPermissionDenied reports whether err is *directly* a 403 ClientError
+// (not merely something that wraps one). Used by the cancel-from-email
+// session pre-check to distinguish a legitimate "your session lacks
+// cancel-* permission" answer (fall through to the token branch's
+// contact_email gate) from a transient auth-service failure (propagate so
+// the caller sees the real cause). CR feedback on PR #216.
+//
+// Strict (un-wrapped) type assertion is deliberate: if a future caller
+// wraps a 403 to add context (fmt.Errorf("permission check failed: %w",
+// ...)), that wrapped error represents the *outer* failure mode (the
+// wrapper's intent), not "this is still a 403". errors.As-style unwrapping
+// would erase that distinction and silently route wrapped backend
+// failures into the contact_email gate — exactly the misclassification
+// the propagate-vs-fall-through split is meant to prevent.
+func isPermissionDenied(err error) bool {
+	ce, ok := err.(*clientError)
+	return ok && ce.code == 403
+}
+
+// tryGetSession returns the validated session for the request, or nil when
+// the request carries no Bearer token, the auth service isn't configured,
+// or session validation fails. Mirrors tryResolveActorEmail's silent
+// best-effort semantics so AuthPublic callers can opt into session-aware
+// behaviour without forcing a 401 on tokenless flows.
+func (h *Handler) tryGetSession(ctx context.Context, req *events.LambdaFunctionURLRequest) *Session {
 	if req == nil || h.auth == nil {
-		return ""
+		return nil
 	}
 	bearer := h.extractBearerToken(req)
 	if bearer == "" {
-		return ""
+		return nil
 	}
 	session, err := h.auth.ValidateSession(ctx, bearer)
 	if err != nil || session == nil {
-		return ""
+		return nil
 	}
-	return session.Email
+	return session
 }
 
 // buildPurchaseDetailsResponse builds the response map for a purchase execution.

internal/api/handler_purchases_test.go

@@ -80,6 +80,12 @@ func TestHandler_cancelPurchase(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// Session has no admin role and no cancel permissions, so cancelPurchase's
+	// session-authed pre-check (added to fix the deep-link contact_email gate)
+	// falls through to authorizeApprovalAction. The contact_email + globalNotify
+	// fixture above is what proves the legacy email-link path still works.
+	mockAuth.On("HasPermissionAPI", ctx, "", "cancel-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "cancel-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("CancelExecution", ctx, execID, "valid-token", approver).Return(nil)
@@ -1418,6 +1424,201 @@ func TestHandler_cancelPurchase_Session_RejectsMissingSession(t *testing.T) {
 	assert.Contains(t, err.Error(), "no authorization token provided")
 }
 
+// TestHandler_cancelPurchase_DeepLink_AdminBypassesContactEmailGate is the
+// regression for the cancel-from-email contact_email-gate UX bug:
+//
+//	"Failed to cancel purchase: no per-account contact email configured for
+//	 this execution; set the cloud account's contact_email before approving"
+//
+// The deep-link cancel flow (frontend purchases-deeplink.ts) always POSTs
+// /api/purchases/cancel/:id with both an X-Authorization Bearer session AND
+// the email-link's URL token. Before this fix, cancelPurchase always took
+// the token branch → authorizeApprovalAction → 403 when the execution's
+// recs had no per-account contact_email (e.g. AWS ambient-credentials
+// rows where CloudAccountID is nil). The same admin could already cancel
+// the same execution from the History page Cancel button (session-authed,
+// no contact_email gate); the email-link path was the only place they
+// were locked out.
+//
+// Fix: when the caller has a valid session AND authorizeSessionCancel
+// passes (admin / cancel-any / cancel-own match), use the session-authed
+// cancel path, regardless of whether a token is present. Token-only
+// callers (no session, e.g. forwarded email or shared inbox) still hit
+// the contact_email gate from PR #101.
+func TestHandler_cancelPurchase_DeepLink_AdminBypassesContactEmailGate(t *testing.T) {
+	// Ambient-credentials execution: CloudAccountID is nil, so
+	// gatherAccountContactEmails returns []. Pre-fix this would 403 even
+	// for admin sessions. Post-fix the session-authed pre-check fires and
+	// the cancel succeeds.
+	exec := &config.PurchaseExecution{
+		ExecutionID:     cancelExecID,
+		Status:          "notified",
+		CreatedByUserID: nil, // no creator binding either; admin can still cancel
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r-ambient", CloudAccountID: nil},
+		},
+	}
+	session := &Session{UserID: cancelCallerID, Role: "admin", Email: "admin@example.com"}
+
+	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false, false)
+	var saved *config.PurchaseExecution
+	mockConfig.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).
+		Run(func(args mock.Arguments) {
+			saved = args.Get(1).(*config.PurchaseExecution)
+		}).
+		Return(nil)
+
+	// Token IS present in the URL — the deep-link flow always sends one.
+	// The fix's whole point is that the admin session takes the
+	// session-authed branch instead of routing through the token path.
+	result, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "deep-link-token")
+	require.NoError(t, err, "admin clicking Cancel from notification email must succeed even when no contact_email is configured")
+	assert.Equal(t, "cancelled", result.(map[string]string)["status"])
+
+	require.NotNil(t, saved, "session-authed branch must commit the status flip")
+	assert.Equal(t, "cancelled", saved.Status)
+	require.NotNil(t, saved.CancelledBy, "session-authed branch must stamp CancelledBy")
+	assert.Equal(t, session.Email, *saved.CancelledBy)
+
+	// Critical security assertion: the token branch's contact_email gate
+	// (authorizeApprovalAction → GetGlobalConfig → resolveApprovalRecipients)
+	// was NOT consulted. If a regression re-routed admins through the
+	// token path, GetGlobalConfig would fire because the gate fetches
+	// the global notification email; asserting it didn't is the cleanest
+	// way to pin the new branch behaviour.
+	mockConfig.AssertNotCalled(t, "GetGlobalConfig", mock.Anything)
+	mockAuth.AssertExpectations(t)
+}
+
+// TestHandler_cancelPurchase_DeepLink_CancelOwnBypassesContactEmailGate is
+// the cancel-own variant of the regression above: a non-admin user with
+// cancel-own on a purchase they themselves created should be able to
+// cancel it from the email link, bypassing the contact_email gate that
+// would otherwise lock them out for ambient-credentials executions.
+func TestHandler_cancelPurchase_DeepLink_CancelOwnBypassesContactEmailGate(t *testing.T) {
+	creator := cancelCallerID // session user is the creator
+	exec := &config.PurchaseExecution{
+		ExecutionID:     cancelExecID,
+		Status:          "notified",
+		CreatedByUserID: &creator,
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r-ambient", CloudAccountID: nil},
+		},
+	}
+	session := &Session{UserID: cancelCallerID, Role: "user", Email: "u1@example.com"}
+
+	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false /*hasAny*/, true /*hasOwn*/)
+	mockConfig.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil)
+
+	result, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "deep-link-token")
+	require.NoError(t, err)
+	assert.Equal(t, "cancelled", result.(map[string]string)["status"])
+	mockConfig.AssertNotCalled(t, "GetGlobalConfig", mock.Anything)
+	mockAuth.AssertExpectations(t)
+}
+
+// TestIsPermissionDenied pins the strict (un-wrapped) type-assertion
+// invariant from CR pass 2 on PR #216: a wrapped 403 ClientError must
+// NOT count as permission-denied because the wrapper changes the
+// failure's outer category. Only an *exact* *clientError with code 403
+// triggers the fall-through to the contact_email gate.
+func TestIsPermissionDenied(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{"nil error is not denial", nil, false},
+		{"plain 403 ClientError is denial", NewClientError(403, "permission denied"), true},
+		{"500 ClientError is not denial", NewClientError(500, "auth service down"), false},
+		{"401 ClientError is not denial", NewClientError(401, "no session"), false},
+		{"non-ClientError is not denial", errors.New("auth backend timeout"), false},
+		{
+			name: "wrapped 403 is NOT denial (errors.As-style unwrap is rejected)",
+			err:  fmt.Errorf("permission check failed: %w", NewClientError(403, "denied")),
+			want: false,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, isPermissionDenied(tc.err))
+		})
+	}
+}
+
+// TestHandler_cancelPurchase_DeepLink_TransientAuthErrorPropagates pins the
+// CR-feedback hardening on PR #216: when authorizeSessionCancel returns a
+// non-403 error (auth service down, HasPermissionAPI wrapped error,
+// h.auth nil), the pre-check MUST surface it instead of silently falling
+// through to the contact_email gate. A stale auth backend should not
+// disguise itself as a "set the contact_email" 403, which would mislead
+// operators investigating the failure.
+func TestHandler_cancelPurchase_DeepLink_TransientAuthErrorPropagates(t *testing.T) {
+	creator := cancelOtherID
+	exec := &config.PurchaseExecution{
+		ExecutionID:     cancelExecID,
+		Status:          "notified",
+		CreatedByUserID: &creator,
+	}
+	session := &Session{UserID: cancelCallerID, Role: "user", Email: "u1@example.com"}
+
+	mockConfig := new(MockConfigStore)
+	mockConfig.On("GetExecutionByID", mock.Anything, exec.ExecutionID).Return(exec, nil)
+
+	mockAuth := new(MockAuthService)
+	mockAuth.On("ValidateSession", mock.Anything, "sess-tok").Return(session, nil)
+	// Simulate a transient auth-backend failure on the cancel-any check.
+	// authorizeSessionCancel wraps this as "permission check failed: …"
+	// — NOT a 403 ClientError. The pre-check must propagate.
+	mockAuth.On("HasPermissionAPI", mock.Anything, session.UserID, "cancel-any", "purchases").
+		Return(false, errors.New("auth backend timeout"))
+
+	handler := &Handler{config: mockConfig, auth: mockAuth}
+
+	_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "deep-link-token")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "permission check failed",
+		"transient auth-service errors must surface, not silently route to the contact_email gate")
+	assert.NotContains(t, err.Error(), "contact email",
+		"the contact_email message would mislead operators about the actual failure cause")
+
+	// Token branch must NOT have been reached — GetGlobalConfig is the
+	// signature first call inside authorizeApprovalAction.
+	mockConfig.AssertNotCalled(t, "GetGlobalConfig", mock.Anything)
+	mockAuth.AssertExpectations(t)
+}
+
+// TestHandler_cancelPurchase_DeepLink_NonPrivilegedSessionStillHitsContactGate
+// pins the security-model invariant from PR #101: a logged-in user
+// without admin / cancel-any / cancel-own permission MUST still go
+// through authorizeApprovalAction and hit the contact_email gate when
+// clicking a forwarded cancel link. The session-authed pre-check is
+// strictly additive — it widens for privileged users only.
+func TestHandler_cancelPurchase_DeepLink_NonPrivilegedSessionStillHitsContactGate(t *testing.T) {
+	creator := cancelOtherID // someone else created it; no cancel-own match
+	exec := &config.PurchaseExecution{
+		ExecutionID:     cancelExecID,
+		ApprovalToken:   "deep-link-token",
+		Status:          "notified",
+		CreatedByUserID: &creator,
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r-ambient", CloudAccountID: nil},
+		},
+	}
+	session := &Session{UserID: cancelCallerID, Role: "user", Email: "u1@example.com"}
+
+	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false /*hasAny*/, false /*hasOwn*/)
+	// Token branch fetches the global config to populate the Cc list.
+	mockConfig.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil)
+
+	_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "deep-link-token")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "no per-account contact email configured for this execution",
+		"non-privileged session must fall through to the contact_email gate from PR #101")
+	mockConfig.AssertNotCalled(t, "SavePurchaseExecution")
+	mockAuth.AssertExpectations(t)
+}
+
 // ─── Session-authed Retry (issue #47) ──────────────────────────────────────
 //
 // Mirror image of the cancel matrix above. retryPurchase creates a NEW

internal/api/handler_test.go

@@ -764,6 +764,12 @@ func TestHandler_HandleRequest_CancelPurchase(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", mock.Anything, "sess-tok").Return(&Session{Email: approver}, nil)
+	// Session has no admin role / cancel permissions → cancelPurchase's
+	// session-authed pre-check falls through to authorizeApprovalAction →
+	// MockPurchaseManager.CancelExecution, exercising the legacy
+	// email-link path that this end-to-end test was always covering.
+	mockAuth.On("HasPermissionAPI", mock.Anything, "", "cancel-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", mock.Anything, "", "cancel-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("CancelExecution", mock.Anything, execID, "token456", approver).Return(nil)