fix(api): return session authz denials when no email token is present

internal/api/handler_purchases.go

@@ -612,15 +612,14 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 	//      session. authorizeApprovalAction enforces the per-account
 	//      contact_email gate from PR #101; the purchase service
 	//      validates the token itself before mutating state.
-	//   3. token == "" → session-authed dashboard Approve button.
-	//      approvePurchaseViaSession runs the approve-any /
-	//      approve-own RBAC matrix and rejects sessions without it.
+	//   3. token == "" → a session denial above is final (issue #173);
+	//      with no session, approvePurchaseViaSession returns 401.
 	if session := h.tryGetSession(ctx, req); session != nil {
 		switch err := h.authorizeScopedSessionApprove(ctx, session, execution); {
 		case err == nil:
 			return h.approvePurchaseViaSession(ctx, req, execution)
 		case fallsThroughToToken(err, token):
-			// Explicit 403, or an out-of-scope 404 with a token → fall
+			// Explicit 403 or out-of-scope 404, with a token → fall
 			// through to the token branch so the contact_email gate gets a
 			// chance (a logged-in user without approve-* or outside the
 			// account scope may still be the per-account contact recipient).
@@ -1215,18 +1214,16 @@ func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunction
 	//      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.
+	//   3. token == "" → a session denial above is final (issue #173);
+	//      with no session, cancelPurchaseViaSession returns 401.
 	if session := h.tryGetSession(ctx, req); session != nil {
 		switch err := h.authorizeCancelSession(ctx, session, execution); {
 		case err == nil:
 			// Session is RBAC-authorized → run the session-authed cancel.
 			return h.cancelPurchaseViaSession(ctx, req, execution)
 		case fallsThroughToToken(err, token):
-			// Explicit "permission denied" (403), or an out-of-scope 404
-			// with a token → fall through to the token branch so the
+			// Explicit "permission denied" (403) or out-of-scope 404, with
+			// a token → 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).
@@ -2264,15 +2261,15 @@ func wrapConstraintDenied(err error) error {
 }
 
 // fallsThroughToToken reports whether a session-authorization failure should
-// hand the request to the email-token branch: an RBAC denial (403), or an
-// account-scope miss (404) when a token is present. The token branch's
-// contact_email gate is deliberately not account-scoped (issue #92). Without
-// a token the 404 is returned as-is, so the token branch's status guards
-// cannot reveal an out-of-scope execution's existence or status. Both
-// matches are strict, like isPermissionDenied, so a wrapped error from
-// deeper in the chain propagates.
+// hand the request to the email-token branch: an RBAC denial (403) or an
+// account-scope miss (404), and only when a token is present. The token
+// branch's contact_email gate is deliberately not account-scoped (issue #92).
+// Without a token the denial is final (issue #173): nothing else can authorize
+// the request, and the 404 must not let later status guards reveal an
+// out-of-scope execution. Both matches are strict, like isPermissionDenied,
+// so a wrapped error from deeper in the chain propagates.
 func fallsThroughToToken(err error, token string) bool {
-	return isPermissionDenied(err) || (token != "" && err == errNotFound) //nolint:errorlint // strict sentinel identity is deliberate
+	return token != "" && (isPermissionDenied(err) || err == errNotFound) //nolint:errorlint // strict sentinel identity is deliberate
 }
 
 // isPermissionDenied reports whether err is *directly* a 403 ClientError

internal/api/handler_purchases_test.go

@@ -3245,7 +3245,7 @@ func TestHandler_cancelPurchase_Session_RejectsTerminalStatus(t *testing.T) {
 	}
 	session := &Session{UserID: cancelCallerID}
 
-	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false, false)
+	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, true, false)
 
 	_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "")
 	require.Error(t, err)
@@ -3274,7 +3274,7 @@ func TestHandler_cancelPurchase_Session_RejectsEachNonCancelableStatus(t *testin
 			}
 			session := &Session{UserID: cancelCallerID}
 
-			handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false, false)
+			handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, true, false)
 
 			_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "")
 			require.Error(t, err)
@@ -5869,9 +5869,9 @@ func TestHandler_revokePurchase_SessionOwnerCancelOwn(t *testing.T) {
 }
 
 // TestHandler_revokePurchase_SessionNoPermissionNoToken verifies that a session
-// lacking both cancel-any and cancel-own, with no token supplied, is denied:
-// the session branch returns permission-denied and falls through to the
-// tokenless 401. Covers the session-auth revoke branch CR finding (PR #889).
+// lacking both cancel-any and cancel-own, with no token supplied, gets the
+// session branch's 403: without a token the denial is final (issue #173).
+// Covers the session-auth revoke branch CR finding (PR #889).
 func TestHandler_revokePurchase_SessionNoPermissionNoToken(t *testing.T) {
 	ctx := context.Background()
 	execID := "88888888-8888-8888-8888-888888888888"
@@ -5899,7 +5899,7 @@ func TestHandler_revokePurchase_SessionNoPermissionNoToken(t *testing.T) {
 	require.Error(t, err, "no permission and no token must be denied")
 	ce, ok := IsClientError(err)
 	require.True(t, ok, "expected a client error")
-	assert.Equal(t, 401, ce.code)
+	assert.Equal(t, 403, ce.code)
 	mockStore.AssertExpectations(t)
 	mockAuth.AssertExpectations(t)
 }

internal/api/handler_ri_exchange.go

@@ -2166,8 +2166,8 @@ func (h *Handler) getRIExchangeHistory(ctx context.Context, req *events.LambdaFu
 //  2. token != "" -> legacy email-link flow. validateExchangeApproval enforces
 //     the token-equality check; the permission-denied fall-through ensures a
 //     logged-in user without approve-* can still use an email link they hold.
-//  3. token == "" AND no qualifying session -> 403 via
-//     approveRIExchangeViaSession's requireSession gate.
+//  3. token == "": a session denial is returned as-is (issue #173); with no
+//     session, approveRIExchangeViaSession's requireSession gate returns 401.
 func (h *Handler) approveRIExchange(ctx context.Context, req *events.LambdaFunctionURLRequest, id, token string) (any, error) {
 	if session := h.tryGetSession(ctx, req); session != nil {
 		// Quick RBAC pre-check (no record fetch needed): does this session hold
@@ -2193,7 +2193,7 @@ func (h *Handler) approveRIExchange(ctx context.Context, req *events.LambdaFunct
 			if errors.Is(sessErr, errCSRFRejected) || token == "" || !isPermissionDenied(sessErr) {
 				return nil, sessErr
 			}
-		case isPermissionDenied(err):
+		case token != "" && isPermissionDenied(err):
 			// Logged-in user without approve-* may still hold a valid email token.
 		default:
 			return nil, err

internal/api/middleware_test.go

@@ -306,10 +306,14 @@ func TestApproveViaSession_RequiresCSRF(t *testing.T) {
 
 	mockConfig := new(MockConfigStore)
 	exec := &config.PurchaseExecution{
-		ExecutionID:     execID,
-		ApprovalToken:   "email-tok",
-		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{{ID: "r1"}},
+		ExecutionID:   execID,
+		ApprovalToken: "email-tok",
+		Status:        "pending",
+		// Non-zero UpfrontCost so the approve-any Constraints check passes and
+		// the dispatcher hands off to approvePurchaseViaSession (issue #173).
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 
@@ -319,7 +323,7 @@ func TestApproveViaSession_RequiresCSRF(t *testing.T) {
 	// Authorization is group-membership-only after issue #907: the session must
 	// pass the approve-* HasPermissionAPI check to reach the CSRF guard, since
 	// the dispatcher authorizes before invoking approvePurchaseViaSession.
-	mockAuth.grantAdmin()
+	mockAuth.grantAdminPurchaser()
 	// CSRF token is empty → ValidateCSRFToken must return an error so the
 	// request is rejected. This is the critical regression assertion for #404.
 	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(errors.New("csrf mismatch"))

internal/api/session_denial_final_test.go

@@ -0,0 +1,143 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"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/config"
+)
+
+// Issue #173: without a token a session denial is final. The context principal is an authorized
+// admin distinct from the denied bearer session, so a fall-through reaches the mutation.
+
+const (
+	denialTestExecID = "17317317-3173-1731-7317-317317317317"
+	denialTestUserID = "denied-uid"
+)
+
+var errDenialTestReached = errors.New("mutation reached")
+
+func newDenialTestHandler(t *testing.T, store *MockConfigStore, mockPurchase *MockPurchaseManager) (*Handler, context.Context) {
+	t.Helper()
+	mockAuth := new(MockAuthService)
+
+	mockAuth.On("ValidateSession", mock.Anything, "sess-tok").Return(&Session{UserID: denialTestUserID, Email: "denied@example.com"}, nil)
+	mockAuth.On("HasPermissionAPI", mock.Anything, denialTestUserID, mock.Anything, mock.Anything).Return(false, nil)
+	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, denialTestUserID).Return(nil, nil).Maybe()
+	mockAuth.On("ValidateCSRFToken", mock.Anything, "sess-tok", "csrf-ok").Return(nil).Maybe()
+
+	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe()
+	store.On("CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(false, "", errDenialTestReached).Maybe()
+	store.On("TransitionRIExchangeStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, errDenialTestReached).Maybe()
+	mockPurchase.On("ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return("", errDenialTestReached).Maybe()
+
+	ctx := contextWithPrincipal(context.Background(), &Principal{
+		Kind:    PrincipalSession,
+		Session: &Session{UserID: apiKeyAdminUserID, Email: "admin@example.com"},
+		UserID:  apiKeyAdminUserID,
+	})
+	return &Handler{auth: mockAuth, config: store, purchase: mockPurchase}, ctx
+}
+
+func denialTestRequest() *events.LambdaFunctionURLRequest {
+	return &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer sess-tok", "x-csrf-token": "csrf-ok"},
+		RequestContext: events.LambdaFunctionURLRequestContext{
+			HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "POST"},
+		},
+	}
+}
+
+func requireForbidden(t *testing.T, err error) {
+	t.Helper()
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a ClientError, got: %v", err)
+	assert.Equal(t, 403, ce.code, "got: %v", err)
+}
+
+func TestApprovePurchase_SessionDenialWithoutTokenIsFinal(t *testing.T) {
+	store, mockPurchase := new(MockConfigStore), new(MockPurchaseManager)
+	h, ctx := newDenialTestHandler(t, store, mockPurchase)
+	store.On("GetExecutionByID", mock.Anything, denialTestExecID).Return(&config.PurchaseExecution{
+		ExecutionID:     denialTestExecID,
+		Status:          "pending",
+		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "aws", UpfrontCost: 100}},
+	}, nil)
+
+	_, err := h.approvePurchase(ctx, denialTestRequest(), denialTestExecID, "")
+
+	requireForbidden(t, err)
+	assert.Contains(t, err.Error(), "requires approve-any or approve-own")
+	mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+func TestCancelPurchase_SessionDenialWithoutTokenIsFinal(t *testing.T) {
+	store := new(MockConfigStore)
+	h, ctx := newDenialTestHandler(t, store, new(MockPurchaseManager))
+	store.On("GetExecutionByID", mock.Anything, denialTestExecID).Return(&config.PurchaseExecution{
+		ExecutionID: denialTestExecID,
+		Status:      "pending",
+	}, nil)
+
+	_, err := h.cancelPurchase(ctx, denialTestRequest(), denialTestExecID, "")
+
+	requireForbidden(t, err)
+	assert.Contains(t, err.Error(), "requires cancel-any or cancel-own")
+	store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+func TestRevokePurchase_SessionDenialWithoutTokenIsFinal(t *testing.T) {
+	store := new(MockConfigStore)
+	h, ctx := newDenialTestHandler(t, store, new(MockPurchaseManager))
+	store.On("GetExecutionByID", mock.Anything, denialTestExecID).Return(&config.PurchaseExecution{
+		ExecutionID: denialTestExecID,
+		Status:      "completed",
+	}, nil)
+
+	_, err := h.revokeViaEmailToken(ctx, denialTestRequest(), denialTestExecID, "")
+
+	requireForbidden(t, err)
+	assert.Contains(t, err.Error(), "requires cancel-any or cancel-own")
+}
+
+func TestApproveRIExchange_SessionDenialWithoutTokenIsFinal(t *testing.T) {
+	store := new(MockConfigStore)
+	h, ctx := newDenialTestHandler(t, store, new(MockPurchaseManager))
+	store.On("GetRIExchangeRecord", mock.Anything, denialTestExecID).Return(&config.RIExchangeRecord{
+		ID:            denialTestExecID,
+		Status:        "pending",
+		ApprovalToken: config.HashApprovalToken("tok"),
+		SourceRIIDs:   []string{"ri-1"},
+		PaymentDue:    "10.00",
+	}, nil).Maybe()
+
+	_, err := h.approveRIExchange(ctx, denialTestRequest(), denialTestExecID, "")
+
+	requireForbidden(t, err)
+	assert.Contains(t, err.Error(), "requires approve-any or approve-own")
+	store.AssertNotCalled(t, "TransitionRIExchangeStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+func TestApproveRIExchange_SessionDenialWithTokenUsesToken(t *testing.T) {
+	store := new(MockConfigStore)
+	h, ctx := newDenialTestHandler(t, store, new(MockPurchaseManager))
+	store.On("GetRIExchangeRecord", mock.Anything, denialTestExecID).Return(&config.RIExchangeRecord{
+		ID:            denialTestExecID,
+		Status:        "pending",
+		ApprovalToken: config.HashApprovalToken("tok"),
+		SourceRIIDs:   []string{"ri-1"},
+		PaymentDue:    "10.00",
+	}, nil)
+
+	_, err := h.approveRIExchange(ctx, denialTestRequest(), denialTestExecID, "tok")
+
+	require.ErrorIs(t, err, errDenialTestReached)
+	store.AssertCalled(t, "TransitionRIExchangeStatus", mock.Anything, denialTestExecID, "pending", "processing", (*string)(nil))
+}