fix(api): enforce cancel permission constraints on session cancels

internal/api/handler_purchases.go

@@ -1399,6 +1399,8 @@ func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunction
 	//      action they can perform from the dashboard. The token in
 	//      the URL is informational at that point — the session has
 	//      already authenticated the actor.
+	//      A session whose held cancel verb is denied by its Constraints
+	//      (issue #678) is final: it does not fall through to the token.
 	//   2. token != "" → legacy email-link flow without a qualifying
 	//      session. authorizeApprovalAction enforces the per-account
 	//      contact_email gate from PR #101; the purchase service
@@ -1860,7 +1862,13 @@ func (h *Handler) authorizeCancelSession(ctx context.Context, session *Session,
 
 // authorizeSessionCancel returns nil when the session is permitted to cancel
 // the given execution under the cancel-any / cancel-own RBAC rules added in
-// issue #46. Returns a 403 ClientError otherwise.
+// issue #46, and the Constraints of the permission that grants the verb cover
+// every recommendation of the execution (issue #678, same strict sets as the
+// #404 revoke check). A constrained cancel-any falls back to cancel-own only on
+// a 403; a Constraints denial is terminal for the email-token fall-through (it
+// is wrapped so fallsThroughToToken does not hand it to the contact_email
+// branch). The token branch itself is a separate capability and is not
+// Constraints-checked. Returns a 403 ClientError otherwise.
 func (h *Handler) authorizeSessionCancel(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
 	// Stateless admin API key: full access, no user row. Administrators-group
 	// users pass via the cancel-any HasPermissionAPI check below.
@@ -1875,22 +1883,38 @@ func (h *Handler) authorizeSessionCancel(ctx context.Context, session *Session,
 	if err != nil {
 		return fmt.Errorf("permission check failed: %w", err)
 	}
+	var anyErr error
 	if hasAny {
-		return nil
+		anyErr = wrapConstraintDenied(h.requirePermissionConstraints(ctx, session, auth.ActionCancelAny, auth.ResourcePurchases, revokeExecutionConstraintSets(execution)))
+		if anyErr == nil || !isForbidden(anyErr) {
+			return anyErr
+		}
 	}
+	return h.authorizeCancelOwn(ctx, session, execution, anyErr)
+}
 
+// authorizeCancelOwn requires cancel-own, a matching creator and the cancel-own
+// Constraints. anyErr is the cancel-any Constraints denial (nil when the
+// session has no cancel-any); it is returned instead of the generic denials so
+// a constraint refusal stays terminal.
+func (h *Handler) authorizeCancelOwn(ctx context.Context, session *Session, execution *config.PurchaseExecution, anyErr error) error {
 	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionCancelOwn, auth.ResourcePurchases)
 	if err != nil {
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 	if !hasOwn {
+		if anyErr != nil {
+			return anyErr
+		}
 		return NewClientError(403, "permission denied: requires cancel-any or cancel-own on purchases")
 	}
-
 	if execution.CreatedByUserID == nil || *execution.CreatedByUserID != session.UserID {
+		if anyErr != nil {
+			return anyErr
+		}
 		return NewClientError(403, "permission denied: cannot cancel another user's pending purchase")
 	}
-	return nil
+	return wrapConstraintDenied(h.requirePermissionConstraints(ctx, session, auth.ActionCancelOwn, auth.ResourcePurchases, revokeExecutionConstraintSets(execution)))
 }
 
 // requireSession validates the request's session token and returns the

internal/api/handler_purchases_cancel_constraints_test.go

@@ -0,0 +1,278 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/purchase"
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// Issue #678: cancel-any / cancel-own Constraints must bound what a holder can
+// cancel, on the session cancel, the revoke-token session branch and the
+// planned-purchase DELETE. Answers come from the real matcher via
+// grantPermissionsScoped; helpers are shared with the #404 revoke tests.
+
+var cancelSess = &Session{UserID: revokeConstraintsUser}
+
+func azureRec() config.RecommendationRecord {
+	return recFor("acct-uuid", "azure", "compute", "westeurope")
+}
+
+func TestAuthorizeSessionCancel_ConstraintDimensions(t *testing.T) {
+	t.Parallel()
+	denying := map[string]*auth.PermissionConstraints{
+		"providers":   {Providers: []string{"aws"}},
+		"services":    {Services: []string{"rds"}},
+		"regions":     {Regions: []string{"eastus"}},
+		"account ids": {AccountIDs: []string{"other-acct"}},
+	}
+	allowing := map[string]*auth.PermissionConstraints{
+		"providers":   {Providers: []string{"azure"}},
+		"services":    {Services: []string{"compute"}},
+		"regions":     {Regions: []string{"westeurope"}},
+		"account ids": {AccountIDs: []string{"acct-uuid"}},
+	}
+	for _, verb := range []string{auth.ActionCancelAny, auth.ActionCancelOwn} {
+		for dim, c := range denying {
+			t.Run(verb+"/deny/"+dim, func(t *testing.T) {
+				t.Parallel()
+				h := newRevokeHandler(revokePerm(verb, c))
+				err := h.authorizeSessionCancel(context.Background(), cancelSess, execWithRecs(revokeConstraintsUser, azureRec()))
+				requireRevokeForbidden(t, err, constraintDenied)
+			})
+			t.Run(verb+"/allow/"+dim, func(t *testing.T) {
+				t.Parallel()
+				h := newRevokeHandler(revokePerm(verb, allowing[dim]))
+				require.NoError(t, h.authorizeSessionCancel(context.Background(), cancelSess, execWithRecs(revokeConstraintsUser, azureRec())))
+			})
+		}
+	}
+}
+
+func TestAuthorizeSessionCancel_StrictScopeAndRecommendations(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	regions := revokePerm(auth.ActionCancelAny, &auth.PermissionConstraints{Regions: []string{"westeurope"}})
+	inside, outside := azureRec(), recFor("acct-uuid", "azure", "compute", "eastus")
+
+	t.Run("blank region denies a Regions grant", func(t *testing.T) {
+		t.Parallel()
+		err := newRevokeHandler(regions).authorizeSessionCancel(ctx, cancelSess, execWithRecs("", recFor("a", "azure", "compute", "")))
+		requireRevokeForbidden(t, err, constraintDenied)
+	})
+	t.Run("unattributed account denies an AccountIDs grant", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(revokePerm(auth.ActionCancelAny, &auth.PermissionConstraints{AccountIDs: []string{"acct-uuid"}}))
+		requireRevokeForbidden(t, h.authorizeSessionCancel(ctx, cancelSess, execWithRecs("", recFor("", "azure", "compute", "westeurope"))), constraintDenied)
+	})
+	t.Run("unconstrained grant allows blank dimensions", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(revokePerm(auth.ActionCancelAny, nil))
+		require.NoError(t, h.authorizeSessionCancel(ctx, cancelSess, execWithRecs("", recFor("", "", "", ""))))
+	})
+	t.Run("every recommendation must be covered", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(regions)
+		require.NoError(t, h.authorizeSessionCancel(ctx, cancelSess, execWithRecs("", inside, inside)))
+		requireRevokeForbidden(t, h.authorizeSessionCancel(ctx, cancelSess, execWithRecs("", inside, outside)), constraintDenied)
+	})
+	t.Run("zero recommendations: constrained denies, unconstrained allows", func(t *testing.T) {
+		t.Parallel()
+		requireRevokeForbidden(t, newRevokeHandler(regions).authorizeSessionCancel(ctx, cancelSess, execWithRecs("")), constraintDenied)
+		require.NoError(t, newRevokeHandler(revokePerm(auth.ActionCancelAny, nil)).authorizeSessionCancel(ctx, cancelSess, execWithRecs("")))
+	})
+}
+
+func TestAuthorizeSessionCancel_ConstrainedAnyFallsBackToOwn(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	constrained := &auth.PermissionConstraints{Providers: []string{"aws"}}
+	any, own := revokePerm(auth.ActionCancelAny, constrained), revokePerm(auth.ActionCancelOwn, nil)
+
+	t.Run("creator with unconstrained own is allowed", func(t *testing.T) {
+		t.Parallel()
+		require.NoError(t, newRevokeHandler(any, own).authorizeSessionCancel(ctx, cancelSess, execWithRecs(revokeConstraintsUser, azureRec())))
+	})
+	t.Run("non-creator gets the constraint message, not the ownership one", func(t *testing.T) {
+		t.Parallel()
+		err := newRevokeHandler(any, own).authorizeSessionCancel(ctx, cancelSess, execWithRecs("someone-else", azureRec()))
+		requireRevokeForbidden(t, err, constraintDenied)
+	})
+	t.Run("constrained any without own is denied", func(t *testing.T) {
+		t.Parallel()
+		requireRevokeForbidden(t, newRevokeHandler(any).authorizeSessionCancel(ctx, cancelSess, execWithRecs(revokeConstraintsUser, azureRec())), constraintDenied)
+	})
+	t.Run("both constrained and outside is denied", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(any, revokePerm(auth.ActionCancelOwn, constrained))
+		requireRevokeForbidden(t, h.authorizeSessionCancel(ctx, cancelSess, execWithRecs(revokeConstraintsUser, azureRec())), constraintDenied)
+	})
+	t.Run("plain ownership and missing-verb denials are unchanged", func(t *testing.T) {
+		t.Parallel()
+		requireRevokeForbidden(t, newRevokeHandler(own).authorizeSessionCancel(ctx, cancelSess, execWithRecs("someone-else", azureRec())), "cannot cancel another user's pending purchase")
+		requireRevokeForbidden(t, newRevokeHandler().authorizeSessionCancel(ctx, cancelSess, execWithRecs("", azureRec())), "requires cancel-any or cancel-own")
+	})
+}
+
+func TestAuthorizeSessionCancel_ConstraintStoreErrorDoesNotFallBack(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	boom := errors.New("constraint store down")
+	mockAuth := new(MockAuthService)
+	mockAuth.On("HasPermissionAPI", ctx, revokeConstraintsUser, auth.ActionCancelAny, auth.ResourcePurchases).Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, revokeConstraintsUser, auth.ActionCancelAny, auth.ResourcePurchases, mock.Anything).Return(false, boom)
+	// cancel-own is deliberately unregistered: querying it fails the test.
+
+	h := &Handler{auth: mockAuth, config: &MockConfigStore{}}
+	err := h.authorizeSessionCancel(ctx, cancelSess, execWithRecs(revokeConstraintsUser, azureRec()))
+	require.ErrorIs(t, err, boom)
+	_, isClient := IsClientError(err)
+	assert.False(t, isClient)
+}
+
+func TestAuthorizeSessionCancel_AdminAndAPIKeys(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	exec := execWithRecs("someone-else", azureRec())
+
+	t.Run("stateless admin API key passes", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(revokePerm(auth.ActionCancelAny, &auth.PermissionConstraints{Providers: []string{"aws"}}))
+		require.NoError(t, h.authorizeSessionCancel(ctx, &Session{UserID: apiKeyAdminUserID}, exec))
+	})
+	t.Run("admin wildcard carrying Constraints is enforced", func(t *testing.T) {
+		t.Parallel()
+		h := newRevokeHandler(auth.Permission{Action: auth.ActionAdmin, Resource: auth.ResourceAll, Constraints: &auth.PermissionConstraints{Providers: []string{"aws"}}})
+		requireRevokeForbidden(t, h.authorizeSessionCancel(ctx, cancelSess, exec), constraintDenied)
+	})
+	t.Run("unconstrained admin wildcard passes", func(t *testing.T) {
+		t.Parallel()
+		require.NoError(t, newRevokeHandler(auth.Permission{Action: auth.ActionAdmin, Resource: auth.ResourceAll}).authorizeSessionCancel(ctx, cancelSess, exec))
+	})
+	t.Run("user API key sessions use the key's constraints", func(t *testing.T) {
+		t.Parallel()
+		for _, allowed := range []bool{false, true} {
+			mockAuth := new(MockAuthService)
+			mockAuth.On("HasPermissionAPI", ctx, revokeConstraintsUser, auth.ActionCancelAny, auth.ResourcePurchases).Return(true, nil)
+			mockAuth.On("HasPermissionAPI", ctx, revokeConstraintsUser, auth.ActionCancelOwn, auth.ResourcePurchases).Return(false, nil).Maybe()
+			mockAuth.On("HasAPIKeyPermissionForConstraintsAPI", ctx, "key-1", revokeConstraintsUser, auth.ActionCancelAny, auth.ResourcePurchases, mock.Anything).Return(allowed, nil).Once()
+			h := &Handler{auth: mockAuth, config: &MockConfigStore{}}
+			err := h.authorizeSessionCancel(ctx, &Session{UserID: revokeConstraintsUser, UserAPIKeyID: "key-1"}, exec)
+			if allowed {
+				require.NoError(t, err)
+			} else {
+				requireRevokeForbidden(t, err, constraintDenied)
+			}
+			mockAuth.AssertExpectations(t)
+		}
+	})
+}
+
+// cancelRouteExec is a pending azure execution with a contact approver, so the
+// token branch is reachable when a test asks for it.
+func cancelRouteExec(status string) *config.PurchaseExecution {
+	acct := "acct-1"
+	future := time.Now().Add(time.Hour)
+	return &config.PurchaseExecution{
+		ExecutionID: cancelExecID, Status: status, CloudAccountID: &acct,
+		ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future,
+		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "azure", Service: "compute", Region: "westeurope", CloudAccountID: &acct}},
+	}
+}
+
+// cancelRouteHandler answers authorization from the real matcher for a caller
+// whose session email is the execution's contact approver.
+func cancelRouteHandler(exec *config.PurchaseExecution, perms ...auth.Permission) (*Handler, *MockConfigStore) {
+	store := new(MockConfigStore)
+	store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) {
+		return &config.CloudAccount{ID: id, Name: id, ContactEmail: "contact@example.com"}, nil
+	}
+	store.On("GetExecutionByID", mock.Anything, cancelExecID).Return(exec, nil)
+	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe()
+	store.On("CancelExecutionAtomic", mock.Anything, mock.Anything, cancelExecID, mock.Anything).Return(true, "canceled", nil).Maybe()
+	store.On("DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, cancelExecID).Return(nil).Maybe()
+	store.On("TransitionExecutionStatus", mock.Anything, cancelExecID, mock.Anything, mock.Anything, mock.Anything).
+		Return(&config.PurchaseExecution{ExecutionID: cancelExecID, Status: "canceled"}, nil).Maybe()
+	store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe()
+	store.On("SetCancelledBy", mock.Anything, cancelExecID, mock.Anything).Return(nil).Maybe()
+
+	m := new(MockAuthService)
+	m.On("ValidateSession", mock.Anything, "sess-tok").Return(&Session{UserID: revokeConstraintsUser, Email: "contact@example.com"}, nil)
+	m.On("ValidateCSRFToken", mock.Anything, "sess-tok", "").Return(nil).Maybe()
+	m.grantPermissions(perms)
+	return &Handler{config: store, auth: m, purchase: purchase.NewManager(purchase.ManagerConfig{ConfigStore: store})}, store
+}
+
+func awsOnly(verb string) auth.Permission {
+	return revokePerm(verb, &auth.PermissionConstraints{Providers: []string{"aws"}})
+}
+
+func TestCancelPurchase_ConstrainedCancelAnyIsDeniedWithAndWithoutToken(t *testing.T) {
+	t.Parallel()
+	for _, token := range []string{"", tokenErrRawToken} {
+		t.Run("token="+token, func(t *testing.T) {
+			t.Parallel()
+			h, store := cancelRouteHandler(cancelRouteExec("pending"), awsOnly(auth.ActionCancelAny))
+			_, err := h.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, token)
+			requireRevokeForbidden(t, err, constraintDenied)
+			store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		})
+	}
+}
+
+func TestCancelPurchase_ContactApproverWithoutCancelVerbStillUsesToken(t *testing.T) {
+	t.Parallel()
+	h, store := cancelRouteHandler(cancelRouteExec("pending"))
+	res, err := h.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, tokenErrRawToken)
+	require.NoError(t, err)
+	assert.Equal(t, "canceled", res.(map[string]string)["status"])
+	store.AssertCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, cancelExecID, mock.Anything)
+}
+
+func TestCancelPurchase_InScopeCancelAnyStillCancels(t *testing.T) {
+	t.Parallel()
+	h, _ := cancelRouteHandler(cancelRouteExec("pending"), revokePerm(auth.ActionCancelAny, &auth.PermissionConstraints{Providers: []string{"azure"}}))
+	res, err := h.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "")
+	require.NoError(t, err)
+	assert.Equal(t, "canceled", res.(map[string]string)["status"])
+}
+
+func revokeTokenReq(token string) *events.LambdaFunctionURLRequest {
+	req := tokenErrRequest("revoke", token)
+	req.RequestContext.HTTP.Path = "/api/purchases/revoke/" + cancelExecID
+	return req
+}
+
+func TestRevokeViaEmailToken_SessionBranchEnforcesCancelConstraints(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	t.Run("constrained out is denied and nothing is written", func(t *testing.T) {
+		t.Parallel()
+		h, store := cancelRouteHandler(cancelRouteExec("completed"), awsOnly(auth.ActionCancelAny))
+		_, err := h.revokeViaEmailToken(ctx, revokeTokenReq(""), cancelExecID, "")
+		requireRevokeForbidden(t, err, constraintDenied)
+		store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+	t.Run("constrained out is terminal even with a token", func(t *testing.T) {
+		t.Parallel()
+		h, store := cancelRouteHandler(cancelRouteExec("completed"), awsOnly(auth.ActionCancelAny))
+		_, err := h.revokeViaEmailToken(ctx, revokeTokenReq(tokenErrRawToken), cancelExecID, tokenErrRawToken)
+		requireRevokeForbidden(t, err, constraintDenied)
+		store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+	t.Run("constrained in proceeds", func(t *testing.T) {
+		t.Parallel()
+		h, store := cancelRouteHandler(cancelRouteExec("completed"), revokePerm(auth.ActionCancelAny, &auth.PermissionConstraints{Providers: []string{"azure"}}))
+		_, err := h.revokeViaEmailToken(ctx, revokeTokenReq(""), cancelExecID, "")
+		require.NoError(t, err)
+		store.AssertCalled(t, "TransitionExecutionStatus", mock.Anything, cancelExecID, mock.Anything, "revocation_requested", mock.Anything)
+	})
+}

internal/api/handler_purchases_revoke.go

@@ -362,6 +362,7 @@ func revokeConstraintSet(accountID, provider, service, region string) auth.Permi
 // revokeExecutionConstraintSets builds one set per recommendation. An
 // execution with none gets a single empty strict set so a constrained grant
 // still denies instead of the empty slice failing the permission check loudly.
+// The session cancel paths (issue #678) reuse it.
 func revokeExecutionConstraintSets(execution *config.PurchaseExecution) []auth.PermissionConstraints {
 	if len(execution.Recommendations) == 0 {
 		return []auth.PermissionConstraints{revokeConstraintSet("", "", "", "")}

internal/api/handler_purchases_test.go

@@ -3127,6 +3127,7 @@ func buildSessionCancelHandler(exec *config.PurchaseExecution, session *Session,
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", mock.Anything, "sess-tok").Return(session, nil)
+	mockAuth.allowConstraintChecks()
 	// cancelPurchaseViaSession enforces CSRF on the session-authed path (issue
 	// #404). sessionCancelReq carries no CSRF header, so ValidateCSRFToken is
 	// called with an empty csrfToken. Tests exercising the session-authed cancel
@@ -5956,6 +5957,7 @@ func TestHandler_revokePurchase_SessionAdminCancelAny(t *testing.T) {
 	mockAuth.On("ValidateSession", ctx, "admin-token").
 		Return(&Session{UserID: adminUserID, Email: adminEmail}, nil)
 	mockAuth.On("HasPermissionAPI", ctx, adminUserID, "cancel-any", "purchases").Return(true, nil)
+	mockAuth.allowConstraintChecks()
 	// requireExecutionAccess (issue #92) runs before RBAC; unrestricted
 	// here since scope is not under test.
 	mockAuth.On("GetAllowedAccountsAPI", ctx, adminUserID).Return([]string{}, nil)
@@ -6002,6 +6004,7 @@ func TestHandler_revokePurchase_SessionOwnerCancelOwn(t *testing.T) {
 		Return(&Session{UserID: ownerUserID, Email: ownerEmail}, nil)
 	mockAuth.On("HasPermissionAPI", ctx, ownerUserID, "cancel-any", "purchases").Return(false, nil)
 	mockAuth.On("HasPermissionAPI", ctx, ownerUserID, "cancel-own", "purchases").Return(true, nil)
+	mockAuth.allowConstraintChecks()
 	// requireExecutionAccess (issue #92) runs before RBAC; unrestricted
 	// here since scope is not under test.
 	mockAuth.On("GetAllowedAccountsAPI", ctx, ownerUserID).Return([]string{}, nil)