fix(api): probe the API key, not its owner, for remaining purchase *-any/*-own verbs

internal/api/handler_purchases.go

@@ -1055,7 +1055,7 @@ func (h *Handler) authorizeScopedSessionApprove(ctx context.Context, session *Se
 func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
 	// The stateless admin API key has full access and no user row to resolve
 	// permissions from. Administrators-group users fall through and pass via
-	// the approve-any HasPermissionAPI check below, since {admin, *} matches
+	// the approve-any check below, since {admin, *} matches
 	// any requested permission.
 	if session.UserID == apiKeyAdminUserID {
 		return nil
@@ -1064,17 +1064,17 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session,
 		return NewClientError(500, "authentication service not configured")
 	}
 
-	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveAny, auth.ResourcePurchases)
+	hasAny, err := h.sessionHoldsVerb(ctx, session, auth.ActionApproveAny)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if hasAny {
 		return wrapConstraintDenied(h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveAny, execution.Recommendations))
 	}
 
-	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
+	hasOwn, err := h.sessionHoldsVerb(ctx, session, auth.ActionApproveOwn)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if !hasOwn {
 		return NewClientError(403, "permission denied: requires approve-any or approve-own on purchases")
@@ -1113,16 +1113,16 @@ func (h *Handler) approveConstraintsForSession(ctx context.Context, session *Ses
 // session holds (approve-any first, then approve-own). bound is false when no
 // verb applies to this row, so the caller falls back to the plan cap.
 func (h *Handler) enforceHeldApproveConstraints(ctx context.Context, session *Session, execution *config.PurchaseExecution) (bound bool, err error) {
-	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveAny, auth.ResourcePurchases)
+	hasAny, err := h.sessionHoldsVerb(ctx, session, auth.ActionApproveAny)
 	if err != nil {
-		return false, fmt.Errorf("permission check failed: %w", err)
+		return false, err
 	}
 	if hasAny {
 		return true, h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveAny, execution.Recommendations)
 	}
-	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
+	hasOwn, err := h.sessionHoldsVerb(ctx, session, auth.ActionApproveOwn)
 	if err != nil {
-		return false, fmt.Errorf("permission check failed: %w", err)
+		return false, err
 	}
 	// A plan-step row has no creator (plan_step_attach.go), so its approve-own
 	// holder is bound by their Constraints regardless of it.
@@ -1401,7 +1401,7 @@ func (h *Handler) requireDifferentApprover(ctx context.Context, session *Session
 //     component is a 500 as per feedback_fail_closed_middleware.md.
 func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Session, creatorID string, recs []config.RecommendationRecord) error {
 	// Stateless admin API key: full access, no user row. Administrators-group
-	// users pass via the execute-any HasPermissionAPI check below, since
+	// users pass via the execute-any check below, since
 	// {admin, *} matches any requested permission.
 	if session.UserID == apiKeyAdminUserID {
 		return nil
@@ -1410,17 +1410,21 @@ func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Se
 		return NewClientError(500, "authentication service not configured")
 	}
 
-	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteAny, auth.ResourcePurchases)
+	// Both verbs are probed on the principal enforcePurchaseConstraints
+	// judges: the key (intersected with its owner) for a user API key, so a
+	// key holding execute-own is not routed to its owner's execute-any and
+	// then refused on the key's (issue #1005).
+	hasAny, err := h.sessionHoldsVerb(ctx, session, auth.ActionExecuteAny)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if hasAny {
 		return h.enforcePurchaseConstraints(ctx, session, auth.ActionExecuteAny, recs)
 	}
 
-	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteOwn, auth.ResourcePurchases)
+	hasOwn, err := h.sessionHoldsVerb(ctx, session, auth.ActionExecuteOwn)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if !hasOwn {
 		return NewClientError(403, "permission denied: requires execute-any or execute-own on purchases")
@@ -1935,17 +1939,17 @@ func (h *Handler) authorizeCancelSession(ctx context.Context, session *Session,
 // 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.
+	// users pass via the cancel-any check below.
 	if session.UserID == apiKeyAdminUserID {
 		return nil
 	}
 	if h.auth == nil {
 		return NewClientError(500, "authentication service not configured")
 	}
 
-	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionCancelAny, auth.ResourcePurchases)
+	hasAny, err := h.sessionHoldsVerb(ctx, session, auth.ActionCancelAny)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	var anyErr error
 	if hasAny {
@@ -1962,9 +1966,9 @@ func (h *Handler) authorizeSessionCancel(ctx context.Context, session *Session,
 // 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)
+	hasOwn, err := h.sessionHoldsVerb(ctx, session, auth.ActionCancelOwn)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if !hasOwn {
 		if anyErr != nil {
@@ -2519,25 +2523,25 @@ var errRetryLadderTrancheSettled = errors.New("ladder tranche is settled")
 // non-admins, same as the cancel path).
 func (h *Handler) authorizeSessionRetry(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
 	// Stateless admin API key: full access, no user row. Administrators-group
-	// users pass via the retry-any HasPermissionAPI check below.
+	// users pass via the retry-any check below.
 	if session.UserID == apiKeyAdminUserID {
 		return nil
 	}
 	if h.auth == nil {
 		return NewClientError(500, "authentication service not configured")
 	}
 
-	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionRetryAny, auth.ResourcePurchases)
+	hasAny, err := h.sessionHoldsVerb(ctx, session, auth.ActionRetryAny)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if hasAny {
 		return nil
 	}
 
-	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionRetryOwn, auth.ResourcePurchases)
+	hasOwn, err := h.sessionHoldsVerb(ctx, session, auth.ActionRetryOwn)
 	if err != nil {
-		return fmt.Errorf("permission check failed: %w", err)
+		return err
 	}
 	if !hasOwn {
 		return NewClientError(403, "permission denied: requires retry-any or retry-own on purchases")

internal/api/handler_purchases_cancel_constraints_test.go

@@ -161,8 +161,10 @@ func TestAuthorizeSessionCancel_AdminAndAPIKeys(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()
+			// The verbs are probed on the key, not the owner (issue #1005).
+			probe := []auth.PermissionConstraints{{}}
+			mockAuth.On("HasAPIKeyPermissionForConstraintsAPI", ctx, "key-1", revokeConstraintsUser, auth.ActionCancelAny, auth.ResourcePurchases, probe).Return(true, nil)
+			mockAuth.On("HasAPIKeyPermissionForConstraintsAPI", ctx, "key-1", revokeConstraintsUser, auth.ActionCancelOwn, auth.ResourcePurchases, probe).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)

internal/api/handler_purchases_owner_key_probe_helpers_test.go

@@ -0,0 +1,181 @@
+package api //nolint:revive // var-naming: package name "api" is intentional for handler package
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"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"
+)
+
+// Issue #1005, helper level: each purchase authorization helper must decide
+// a user-API-key session's full-scope *-any verb on the KEY (intersected
+// with its owner), never on the owner alone, so that the verb check and the
+// Constraints check that follows judge the same principal. Apart from
+// direct execute, no route hands these helpers a key session today
+// (TestSessionOnlyPurchaseRoutes_RefuseAUserAPIKey), so this pins the
+// helpers themselves.
+
+var errOKPUnbound = errors.New("no held approve verb binds this row")
+
+func okpRecs() []config.RecommendationRecord {
+	return []config.RecommendationRecord{{
+		ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", Count: 1, UpfrontCost: 100,
+	}}
+}
+
+func okpExecution(creator string) *config.PurchaseExecution {
+	return &config.PurchaseExecution{
+		ExecutionID: okpRowID, Status: "failed", CreatedByUserID: &creator, Recommendations: okpRecs(),
+	}
+}
+
+type okpHelperCase struct {
+	name, anyVerb, ownVerb string
+	// narrow is a key scope holding no full-scope verb; defaults to ownVerb.
+	narrow []string
+	call   func(h *Handler, s *Session, creator string) error
+}
+
+var okpHelperCases = []okpHelperCase{
+	{name: "approve", anyVerb: auth.ActionApproveAny, ownVerb: auth.ActionApproveOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionApprove(context.Background(), s, okpExecution(creator))
+		}},
+	{name: "approve token constraints", anyVerb: auth.ActionApproveAny, ownVerb: auth.ActionApproveOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			bound, err := h.enforceHeldApproveConstraints(context.Background(), s, okpExecution(creator))
+			if err == nil && !bound {
+				return errOKPUnbound
+			}
+			return err
+		}},
+	{name: "execute direct", anyVerb: auth.ActionExecuteAny, ownVerb: auth.ActionExecuteOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionExecuteDirect(context.Background(), s, creator, okpRecs())
+		}},
+	{name: "cancel", anyVerb: auth.ActionCancelAny, ownVerb: auth.ActionCancelOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionCancel(context.Background(), s, okpExecution(creator))
+		}},
+	{name: "retry", anyVerb: auth.ActionRetryAny, ownVerb: auth.ActionRetryOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionRetry(context.Background(), s, okpExecution(creator))
+		}},
+	{name: "revoke scheduled", anyVerb: auth.ActionRevokeAny, ownVerb: auth.ActionRevokeOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionRevokeExecution(context.Background(), s, okpExecution(creator))
+		}},
+	// History rows carry no creator (ownership is account access, and the
+	// owner's scope is unrestricted here), so the narrow key holds no revoke
+	// verb at all and creator is ignored.
+	{name: "revoke history", anyVerb: auth.ActionRevokeAny, ownVerb: auth.ActionRevokeOwn, narrow: []string{auth.ActionExecute},
+		call: func(h *Handler, s *Session, _ string) error {
+			acct := "acct-1005"
+			return h.authorizeSessionRevoke(context.Background(), s, &config.PurchaseHistoryRecord{
+				PurchaseID: okpRowID, CloudAccountID: &acct, Provider: "azure", Service: "compute", Region: "westeurope",
+			})
+		}},
+	// The pre-fetch probe has no row, so the narrow key holds no approve verb.
+	{name: "ri exchange approve right", anyVerb: auth.ActionApproveAny, ownVerb: auth.ActionApproveOwn, narrow: []string{auth.ActionExecute},
+		call: func(h *Handler, s *Session, _ string) error {
+			return h.sessionHasApproveRight(context.Background(), s)
+		}},
+	{name: "ri exchange approve", anyVerb: auth.ActionApproveAny, ownVerb: auth.ActionApproveOwn,
+		call: func(h *Handler, s *Session, creator string) error {
+			return h.authorizeSessionApproveRIExchange(context.Background(), s, &config.RIExchangeRecord{
+				ID: okpRowID, Status: "pending", CreatedByUserID: &creator,
+			})
+		}},
+}
+
+func okpHelperHandler(a AuthServiceInterface) *Handler {
+	store := new(MockConfigStore)
+	store.On("ListCloudAccounts", mock.Anything, mock.Anything).Return([]config.CloudAccount{}, nil).Maybe()
+	return &Handler{auth: a, config: store}
+}
+
+func okpKeySession() *Session { return &Session{UserID: okpOwnerID, UserAPIKeyID: okpKeyID} }
+
+// TestPurchaseAuthzHelpers_FullScopeFollowsTheAPIKey: the owner holds every
+// *-any verb throughout, so a helper that probed the owner would let the
+// narrow key through (or wrongly route its own-row request to *-any).
+func TestPurchaseAuthzHelpers_FullScopeFollowsTheAPIKey(t *testing.T) {
+	for _, tc := range okpHelperCases {
+		narrow := tc.narrow
+		if narrow == nil {
+			narrow = []string{tc.ownVerb}
+		}
+		t.Run(tc.name+"/narrow key on another user's row is refused", func(t *testing.T) {
+			err := tc.call(okpHelperHandler(newOKPAuth(narrow...)), okpKeySession(), okpVictimID)
+			assert.Error(t, err)
+		})
+		t.Run(tc.name+"/key with neither verb on its owner's row is refused", func(t *testing.T) {
+			err := tc.call(okpHelperHandler(newOKPAuth(auth.ActionExecute)), okpKeySession(), okpOwnerID)
+			assert.Error(t, err)
+		})
+		t.Run(tc.name+"/key "+tc.ownVerb+" on its owner's row is allowed", func(t *testing.T) {
+			err := tc.call(okpHelperHandler(newOKPAuth(tc.ownVerb)), okpKeySession(), okpOwnerID)
+			assert.NoError(t, err)
+		})
+		t.Run(tc.name+"/key "+tc.anyVerb+" on another user's row is allowed", func(t *testing.T) {
+			err := tc.call(okpHelperHandler(newOKPAuth(tc.anyVerb)), okpKeySession(), okpVictimID)
+			assert.NoError(t, err)
+		})
+		t.Run(tc.name+"/owner session on another user's row is allowed", func(t *testing.T) {
+			err := tc.call(okpHelperHandler(newOKPAuth()), &Session{UserID: okpOwnerID, Email: "owner@example.com"}, okpVictimID)
+			assert.NoError(t, err)
+		})
+	}
+}
+
+// TestPurchaseAuthzHelpers_APIKeyVerbProbeErrorRefuses: an error probing
+// the key's *-any verb, or its *-own verb once *-any is not held, is
+// returned, never read as "not held". Every other answer allows, including
+// the owner's own grants, so a helper that swallowed the error or probed the
+// owner would succeed instead.
+func TestPurchaseAuthzHelpers_APIKeyVerbProbeErrorRefuses(t *testing.T) {
+	probe := []auth.PermissionConstraints{{}}
+	for _, tc := range okpHelperCases {
+		for _, failing := range []string{tc.anyVerb, tc.ownVerb} {
+			t.Run(tc.name+"/"+failing, func(t *testing.T) {
+				boom := errors.New("key store down")
+				m := new(MockAuthService)
+				if failing == tc.ownVerb {
+					m.On("HasAPIKeyPermissionForConstraintsAPI", mock.Anything, okpKeyID, okpOwnerID, tc.anyVerb, auth.ResourcePurchases, probe).
+						Return(false, nil)
+				}
+				m.On("HasAPIKeyPermissionForConstraintsAPI", mock.Anything, okpKeyID, okpOwnerID, failing, auth.ResourcePurchases, probe).
+					Return(false, boom)
+				m.On("HasAPIKeyPermissionForConstraintsAPI", mock.Anything, okpKeyID, okpOwnerID, mock.Anything, auth.ResourcePurchases,
+					mock.Anything).Return(true, nil).Maybe()
+				m.On("HasPermissionAPI", mock.Anything, okpOwnerID, mock.Anything, auth.ResourcePurchases).Return(true, nil).Maybe()
+				m.On("GetAllowedAccountsAPI", mock.Anything, okpOwnerID).Return([]string{}, nil).Maybe()
+
+				err := tc.call(okpHelperHandler(m), okpKeySession(), okpOwnerID)
+				require.ErrorIs(t, err, boom)
+			})
+		}
+	}
+}
+
+// TestPurchaseAuthzHelpers_AdminAPIKeyUnchanged: the stateless admin API key
+// still short-circuits every helper before any permission probe.
+func TestPurchaseAuthzHelpers_AdminAPIKeyUnchanged(t *testing.T) {
+	for _, tc := range okpHelperCases {
+		if tc.name == "approve token constraints" {
+			continue // approveConstraintsForSession exempts the admin key before this helper runs
+		}
+		t.Run(tc.name, func(t *testing.T) {
+			m := new(MockAuthService) // no expectations: any probe would panic
+			err := tc.call(okpHelperHandler(m), &Session{UserID: apiKeyAdminUserID}, okpVictimID)
+			assert.NoError(t, err)
+			assert.Empty(t, m.Calls)
+		})
+	}
+}