fix(auth): enforce four-eyes principle on purchase approve

frontend/src/__tests__/history-approve-button.test.ts

@@ -386,6 +386,48 @@ describe('History inline Approve button (issue #286)', () => {
     expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
   });
 
+  test('user without any approve permission sees NO Approve button even on their own rows (issue #1407 four-eyes)', async () => {
+    // Regression guard for issue #1407. Before the fix, canApprovePendingRow
+    // returned true based on ownership alone (created_by_user_id === user.id)
+    // without verifying that the session holds approve-own or approve-any.
+    // This caused Viewer-role users (no approve-* in effectivePermissions) to
+    // see the Approve button on purchases they submitted.
+    //
+    // After the fix, approve-own must be checked explicitly before ownership is
+    // evaluated; a session without it sees no Approve buttons at all.
+    const VIEWER_USER = {
+      id: 'viewer-uuid',
+      email: 'viewer@example.com',
+      groups: [],
+      effectivePermissions: [
+        { action: 'view', resource: 'recommendations' },
+        { action: 'view', resource: 'plans' },
+        { action: 'view', resource: 'history' },
+        // cancel-own / retry-own / revoke-own present; approve-own intentionally absent.
+        { action: 'cancel-own', resource: 'purchases' },
+      ],
+    };
+
+    (getCurrentUser as jest.Mock).mockReturnValue(VIEWER_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        // Own row: before fix this showed Approve; after fix it must NOT.
+        makeRow({ purchase_id: 'exec-mine', created_by_user_id: VIEWER_USER.id }),
+        makeRow({ purchase_id: 'exec-other', created_by_user_id: OTHER_UUID }),
+      ],
+    });
+
+    await loadHistory();
+
+    // Scope to the history list (not the approval queue card).
+    const list = document.getElementById('history-list')!;
+    const buttons = list.querySelectorAll<HTMLButtonElement>('.history-approve-btn');
+    // No Approve buttons must appear for any row when the session has no
+    // approve-own or approve-any permission.
+    expect(buttons).toHaveLength(0);
+  });
+
   test('admin WITHOUT Purchaser membership does not see Approve on rows they did not create (CR #924 F5)', async () => {
     // Issue #923 + CR #924 F5: approve-any:purchases is carved out of
     // admin:*. canApprovePendingRow must gate on

frontend/src/__tests__/permissions.test.ts

@@ -56,6 +56,9 @@ describe('permissions', () => {
     });
 
     test('user role grants the standard non-admin verbs', () => {
+      // Issue #1407 (four-eyes): approve-own is intentionally absent from the
+      // standard user permission set. Self-approval requires an explicit custom
+      // group grant; ownership alone does not confer the right to approve.
       const perms = getRolePermissions('user');
       const expected = [
         'view:recommendations',
@@ -68,12 +71,13 @@ describe('permissions', () => {
         'update:purchases',
         'cancel-own:purchases',
         'retry-own:purchases',
-        'approve-own:purchases',
         // Added by PR #804: revoke-own gates the History inline Revoke button
         // for completed Azure purchases within the free-cancel window.
         'revoke-own:purchases',
       ];
       expected.forEach((p) => expect(perms.has(p)).toBe(true));
+      // approve-own must NOT be in the standard user permission set (issue #1407).
+      expect(perms.has('approve-own:purchases')).toBe(false);
       expect(perms.size).toBe(expected.length);
     });
 

frontend/src/history.ts

@@ -477,13 +477,14 @@ function canCancelPendingRow(p: HistoryPurchase): boolean {
 // false-positive here surfaces as a 403 toast on click rather than a
 // successful approve.
 //
-// Heuristic:
+// Heuristic (four-eyes — issue #1407):
 //   * status must be "pending" or "notified";
 //   * any session with approve-any:purchases (carved-out admin verb,
 //     seeded on Purchaser group; can also come from a custom group via
-//     effectivePermissions) → approve-any;
-//   * otherwise the row's created_by_user_id must match the current
-//     user (approve-own);
+//     effectivePermissions) → approve-any; shows Approve on every pending row;
+//   * session must also hold approve-own:purchases before ownership is
+//     even evaluated (four-eyes: ownership alone does NOT grant approve);
+//   * only then: the row's created_by_user_id must match the current user;
 //   * legacy rows with NULL created_by_user_id → no (the email-token
 //     path remains the escape hatch).
 function canApprovePendingRow(p: HistoryPurchase): boolean {
@@ -497,6 +498,9 @@ function canApprovePendingRow(p: HistoryPurchase): boolean {
   // verb directly so a non-seeded role with the same grant still
   // approves rows the backend would also let through.
   if (canAccess('approve-any', 'purchases')) return true;
+  // Four-eyes (issue #1407): the session must hold an explicit approve-own
+  // grant before ownership is consulted. Ownership alone never grants approve.
+  if (!canAccess('approve-own', 'purchases')) return false;
   if (!p.created_by_user_id) return false;
   return p.created_by_user_id === user.id;
 }

frontend/src/permissions.generated.ts

@@ -20,7 +20,6 @@ export const ADMIN_PERMS: ReadonlySet<string> = new Set([
 ]);
 
 export const USER_PERMS: ReadonlySet<string> = new Set([
-  'approve-own:purchases',
   'cancel-own:purchases',
   'create:plans',
   'delete:plans',

internal/api/handler_purchases_test.go

@@ -580,6 +580,70 @@ func TestHandler_approvePurchase_RejectsGlobalNotifyWhenContactSet(t *testing.T)
 	mockPurchase.AssertNotCalled(t, "ApproveExecution")
 }
 
+// TestHandler_approvePurchase_RejectsCreatorWithoutApprovePermission is the
+// security regression guard for issue #1407 (four-eyes). Before the fix,
+// DefaultUserPermissions granted approve-own to all standard users, meaning
+// any creator could silently approve their own purchase. After the fix,
+// approve-own is removed from DefaultUserPermissions; this test confirms that
+// a session whose user IS the execution creator but holds NEITHER approve-any
+// nor approve-own is denied at the authorizeSessionApprove gate (403).
+//
+// Fail-before scenario: with approve-own in DefaultUserPermissions this user
+// would reach approvePurchaseViaSession and succeed.
+// Pass-after scenario: without approve-own the session is denied with 403 and
+// the handler falls through to the token branch (which also rejects: no token).
+func TestHandler_approvePurchase_RejectsCreatorWithoutApprovePermission(t *testing.T) {
+	ctx := context.Background()
+	execID := "12345678-1234-1234-1234-123456789abc"
+	creatorUserID := "creator-user-uuid"
+	creatorEmail := "creator@example.com"
+
+	mockConfig := new(MockConfigStore)
+	exec := &config.PurchaseExecution{
+		ExecutionID:     execID,
+		ApprovalToken:   "valid-token",
+		Status:          "pending",
+		CreatedByUserID: &creatorUserID,
+	}
+	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
+
+	mockAuth := new(MockAuthService)
+	// Session belongs to the creator themselves but holds no approve verb.
+	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{
+		UserID: creatorUserID,
+		Email:  creatorEmail,
+	}, nil)
+	// Four-eyes enforcement: neither approve-any nor approve-own is granted.
+	mockAuth.On("HasPermissionAPI", ctx, creatorUserID, "approve-any", "purchases").Return(false, nil)
+	mockAuth.On("HasPermissionAPI", ctx, creatorUserID, "approve-own", "purchases").Return(false, nil)
+	// approvePurchase: when token is empty and the session has 403, execution
+	// falls through to the final approvePurchaseViaSession call (line 506),
+	// which starts with a CSRF check. Stub it (.Maybe) so the mock doesn't
+	// panic; the CSRF error returns a 403 before the purchase manager is
+	// reached, which is equivalent to the authorizeSessionApprove 403 for the
+	// purpose of this regression guard.
+	mockAuth.On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).
+		Return(errors.New("csrf invalid")).Maybe()
+
+	mockPurchase := new(MockPurchaseManager)
+
+	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
+
+	// No approval token: the token branch is unavailable, so the 403 from
+	// authorizeSessionApprove (or CSRF fallback) surfaces to the caller.
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer sess-tok"},
+	}
+	_, err := handler.approvePurchase(ctx, req, execID, "")
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a ClientError, got %T: %v", err, err)
+	assert.Equal(t, 403, ce.code, "creator without approve permission must be denied 403")
+	// Purchase manager must never be reached.
+	mockPurchase.AssertNotCalled(t, "ApproveExecution")
+	mockPurchase.AssertNotCalled(t, "ApproveAndExecute")
+}
+
 // TestRouter_approvePurchaseHandler_RateLimited is a regression test for issue #400.
 // The approve endpoint is AuthPublic (token-only); without rate limiting any
 // attacker can flood approve attempts to brute-force a valid token.

internal/auth/service_group_test.go

@@ -279,12 +279,13 @@ func TestService_GetUserPermissions(t *testing.T) {
 
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
-		// 12 = 6 read/plan-author + delete:plans (PR-A #660)
+		// 11 = 6 read/plan-author + delete:plans (PR-A #660)
 		// + update:purchases (PR-A #660)
 		// + cancel-own:purchases (issue #46)
-		// + retry-own:purchases (issue #47) + approve-own:purchases (issue #286)
+		// + retry-own:purchases (issue #47)
 		// + revoke-own:purchases (issue #290).
-		assert.Len(t, permissions, 12)
+		// NOTE: approve-own removed (issue #1407, four-eyes).
+		assert.Len(t, permissions, 11)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -353,11 +354,11 @@ func TestService_GetUserPermissions(t *testing.T) {
 
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
-		// 12 standard-group (incl. delete:plans (PR-A #660) + update:purchases (PR-A #660)
-		// + cancel-own (#46) + retry-own (#47) + approve-own (#286)
-		// + revoke-own (#290):purchases)
-		// + 1 group1 + 1 group2 = 14
-		assert.Len(t, permissions, 14)
+		// 11 standard-group (incl. delete:plans (PR-A #660) + update:purchases (PR-A #660)
+		// + cancel-own (#46) + retry-own (#47)
+		// + revoke-own (#290):purchases; approve-own removed per #1407 four-eyes)
+		// + 1 group1 + 1 group2 = 13
+		assert.Len(t, permissions, 13)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -400,12 +401,13 @@ func TestService_GetUserPermissions(t *testing.T) {
 		require.NoError(t, err)
 		// Should have only the resolvable group's permissions; the missing
 		// group is skipped.
-		// 12 = 6 read/plan-author + delete:plans (PR-A #660)
+		// 11 = 6 read/plan-author + delete:plans (PR-A #660)
 		// + update:purchases (PR-A #660)
 		// + cancel-own:purchases (issue #46)
-		// + retry-own:purchases (issue #47) + approve-own:purchases (issue #286)
+		// + retry-own:purchases (issue #47)
 		// + revoke-own:purchases (issue #290).
-		assert.Len(t, permissions, 12)
+		// NOTE: approve-own removed (issue #1407, four-eyes).
+		assert.Len(t, permissions, 11)
 
 		mockStore.AssertExpectations(t)
 	})

internal/auth/types.go

@@ -368,22 +368,24 @@ const (
 	ActionRetryOwn = "retry-own"
 	ActionRetryAny = "retry-any"
 	// ActionApproveOwn / ActionApproveAny gate the session-authed Approve
-	// button on pending Purchase History rows (issue #286). Mirror image
-	// of the cancel-{own,any} verbs above:
+	// button on pending Purchase History rows (issue #286).
 	//
+	// Default grants (four-eyes policy, issue #1407):
 	//   * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers
 	//     both verbs.
-	//   * RoleUser — DefaultUserPermissions() adds approve-own:purchases.
-	//     Allows approving pending executions whose created_by_user_id
-	//     matches the session user. Legacy rows with NULL creator are
-	//     out of reach for non-admins via this verb; admins still
-	//     approve them via approve-any.
+	//   * RoleUser — NO default grant (issue #1407). Four-eyes principle:
+	//     the submitter of a purchase must not approve it by default.
+	//     approve-own must be explicitly added to a custom group for
+	//     organizations that deliberately permit self-approval.
 	//   * RoleReadOnly — neither verb. Read-only users cannot approve.
 	//
-	// approve-any has no default non-admin grant; the constant exists so
-	// future operator roles can be granted broad approve rights without
-	// escalating to admin. Add it to a custom group's Permissions to
-	// enable that path.
+	// approve-any: seeded by the Purchaser group (migration 000059);
+	//   not a default non-admin grant. Add it to a custom group's
+	//   Permissions to enable broad approve rights without escalating
+	//   to admin.
+	// approve-own: no system-managed group holds this by default
+	//   (issue #1407); add to a custom group when self-approval is an
+	//   explicit policy for that group.
 	//
 	// The legacy email-token approve path stays unchanged as an escape
 	// hatch and is gated by token possession + the per-account
@@ -524,14 +526,13 @@ func DefaultUserPermissions() []Permission {
 		// and the retry-attempt counter on the chain to be below the
 		// soft-block threshold (overridable with ?force=true).
 		{Action: ActionRetryOwn, Resource: ResourcePurchases},
-		// approve-own:purchases — every authenticated user can approve
-		// pending purchase executions they created themselves (issue #286).
-		// The handler still requires the execution to be in an approvable
-		// state (pending/notified) and the creator UUID to match the
-		// session UserID before honoring the request. The legacy email-
-		// token approve path stays as an escape hatch for non-session
-		// approvers.
-		{Action: ActionApproveOwn, Resource: ResourcePurchases},
+		// approve-own:purchases is intentionally NOT granted here (issue #1407).
+		// Four-eyes principle: the same user who submits a purchase must NOT
+		// be able to approve it by default. The approve-own verb must be
+		// explicitly granted to a custom group when self-approval is a
+		// deliberate policy choice for that group. Roles without an explicit
+		// approve-own or approve-any grant cannot approve any purchase,
+		// including their own.
 		// revoke-own:purchases — every authenticated user can revoke completed
 		// purchases they created themselves while still within the provider's
 		// free-cancel window (issue #290). The handler verifies the window has

internal/auth/types_test.go

@@ -20,9 +20,9 @@ func TestDefaultPermissions(t *testing.T) {
 		// + update:purchases (PR-A #660)
 		// + cancel-own:purchases (issue #46)
 		// + retry-own:purchases (issue #47)
-		// + approve-own:purchases (issue #286)
-		// + revoke-own:purchases (issue #290) = 12.
-		assert.Len(t, perms, 12)
+		// + revoke-own:purchases (issue #290)
+		// NOTE: approve-own was removed (issue #1407, four-eyes) = 11.
+		assert.Len(t, perms, 11)
 
 		actions := make(map[string]bool)
 		for _, p := range perms {
@@ -39,8 +39,11 @@ func TestDefaultPermissions(t *testing.T) {
 		assert.True(t, actions[ActionUpdate+":"+ResourcePurchases])
 		assert.True(t, actions[ActionCancelOwn+":"+ResourcePurchases])
 		assert.True(t, actions[ActionRetryOwn+":"+ResourcePurchases])
-		assert.True(t, actions[ActionApproveOwn+":"+ResourcePurchases])
 		assert.True(t, actions[ActionRevokeOwn+":"+ResourcePurchases])
+		// Four-eyes guard (issue #1407): approve-own must NOT be a default
+		// user permission. Self-approval requires an explicit custom-group grant.
+		assert.False(t, actions[ActionApproveOwn+":"+ResourcePurchases],
+			"approve-own must not be in DefaultUserPermissions (four-eyes, issue #1407)")
 	})
 
 	t.Run("DefaultReadOnlyPermissions returns readonly access", func(t *testing.T) {

internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users.down.sql

@@ -0,0 +1,13 @@
+-- Restore approve-own:purchases to the Standard Users group (down migration
+-- for 000086_remove_approve_own_from_standard_users).
+--
+-- NOTE: this down migration re-enables the self-approval path for all
+-- Standard Users members, which violates the four-eyes principle patched
+-- by issue #1407. Apply only when explicitly rolling back.
+
+UPDATE groups
+SET
+    permissions = permissions || '[{"action":"approve-own","resource":"purchases"}]'::jsonb,
+    updated_at = NOW()
+WHERE id = '00000000-0000-5000-8000-000000000005'  -- Standard Users
+  AND NOT (permissions @> '[{"action":"approve-own","resource":"purchases"}]');

internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users.up.sql

@@ -0,0 +1,35 @@
+-- Remove approve-own:purchases from the Standard Users group (issue #1407).
+--
+-- Four-eyes principle: the same user who submits a purchase must not be
+-- able to approve it by default. The approve-own verb was seeded in
+-- migration 000057 alongside cancel-own and retry-own, but unlike those
+-- verbs it creates a self-approval path that violates four-eyes.
+--
+-- After this migration:
+--   * Standard Users (Plan Authors, Viewer-equivalent custom groups that
+--     inherited the Standard Users permission set) cannot approve any
+--     purchase, including ones they created themselves.
+--   * The Purchaser group (approve-any:purchases, migration 000059) is
+--     unchanged: Purchaser members can still approve any pending purchase.
+--   * approve-own can be added to a CUSTOM group for organisations that
+--     deliberately permit self-approval as an explicit policy choice.
+--
+-- The permissions column is a JSONB array; this statement uses
+-- jsonb_agg + jsonb_array_elements to filter out the target element
+-- without touching any other permission entries in any other group.
+
+UPDATE groups
+SET
+    permissions = (
+        SELECT COALESCE(
+            jsonb_agg(elem ORDER BY elem->>'action', elem->>'resource'),
+            '[]'::jsonb
+        )
+        FROM jsonb_array_elements(permissions) AS elem
+        WHERE NOT (
+            elem->>'action' = 'approve-own'
+            AND elem->>'resource' = 'purchases'
+        )
+    ),
+    updated_at = NOW()
+WHERE id = '00000000-0000-5000-8000-000000000005';  -- Standard Users