test(purchase): pin case-insensitive email matching in 4-eyes and approver checks

internal/purchase/approvals_test.go

@@ -663,30 +663,51 @@ func TestManager_ApproveAndExecute_FourEyesOff_AllowsSelfApprove(t *testing.T) {
 // awareness of 4-eyes mode at all, so a replayed or forwarded token+actor
 // matching the creator's own email would approve the row despite mode being
 // on.
+//
+// The case-differing rows (issue #307) pin the case-insensitive comparison:
+// email addresses are matched case-insensitively everywhere else in the
+// approval flow, so a creator whose stored email differs from the actor's
+// only in letter case is the same person and must still be denied. A
+// regression that swaps strings.EqualFold for == fails those rows.
 func TestManager_ApproveExecution_FourEyesOn_SQSActorEqualsCreator_Denied(t *testing.T) {
-	ctx := context.Background()
-	manager, store, _ := newApproveManager(t)
-
-	creatorID := "user-creator"
-	creatorEmail := "creator@example.com"
-	execution := &config.PurchaseExecution{
-		ExecutionID:     "exec-sqs-self",
-		PlanID:          "plan-fourEyes",
-		Status:          "pending",
-		ApprovalToken:   config.HashApprovalToken("valid-token"),
-		CreatedByUserID: &creatorID,
+	cases := []struct {
+		name         string
+		creatorEmail string
+		actorEmail   string
+	}{
+		{name: "byte-identical emails", creatorEmail: "creator@example.com", actorEmail: "creator@example.com"},
+		{name: "creator stored mixed case", creatorEmail: "Creator@Example.COM", actorEmail: "creator@example.com"},
+		{name: "actor mixed case", creatorEmail: "creator@example.com", actorEmail: "CREATOR@Example.com"},
 	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			manager, store, _ := newApproveManager(t)
+			// Fail this row on an unexpected call instead of panicking, so
+			// one regressed row doesn't abort the remaining rows.
+			store.Test(t)
 
-	store.On("GetExecutionByID", ctx, "exec-sqs-self").Return(execution, nil)
-	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil)
-	store.On("GetUserEmailByID", ctx, creatorID).Return(creatorEmail, nil)
+			creatorID := "user-creator"
+			execution := &config.PurchaseExecution{
+				ExecutionID:     "exec-sqs-self",
+				PlanID:          "plan-fourEyes",
+				Status:          "pending",
+				ApprovalToken:   config.HashApprovalToken("valid-token"),
+				CreatedByUserID: &creatorID,
+			}
 
-	_, err := manager.ApproveExecution(ctx, "exec-sqs-self", "valid-token", creatorEmail, nil)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
-	store.AssertNotCalled(t, "TransitionExecutionStatus",
-		mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
-	store.AssertExpectations(t)
+			store.On("GetExecutionByID", ctx, "exec-sqs-self").Return(execution, nil)
+			store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil)
+			store.On("GetUserEmailByID", ctx, creatorID).Return(tc.creatorEmail, nil)
+
+			_, err := manager.ApproveExecution(ctx, "exec-sqs-self", "valid-token", tc.actorEmail, nil)
+			require.ErrorIs(t, err, ErrFourEyesDenied)
+			assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
+			store.AssertNotCalled(t, "TransitionExecutionStatus",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			store.AssertExpectations(t)
+		})
+	}
 }
 
 // TestManager_ApproveExecution_FourEyesOn_DifferentActor_Allowed is the

internal/purchase/coverage_extra_test.go

@@ -3,6 +3,7 @@ package purchase
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"
 
@@ -437,45 +438,109 @@ func TestProcessMessage_ApproveHappyPath(t *testing.T) {
 // enabled. Pre-fix this test proceeds straight through to
 // TransitionExecutionStatus (proven by stashing the fix and re-running);
 // post-fix ProcessMessage returns the 4-eyes denial before any state change.
+//
+// The case-differing row (issue #307) runs both email folds on the SQS path
+// end to end: the actor must match the mixed-case per-account contact_email
+// case-insensitively to get past verifyAsyncApprovalActor, and must then be
+// recognized as the creator despite the case difference so 4-eyes denies.
 func TestProcessMessage_ApproveFourEyesOn_SelfApproveDenied(t *testing.T) {
-	ctx := context.Background()
-	mockStore := new(MockConfigStore)
-	mockEmail := new(MockEmailSender)
-
-	accountID := "acct-1"
-	creatorID := "user-creator"
-	creatorEmail := "owner@example.com"
-	exec := &config.PurchaseExecution{
-		ExecutionID:     "exec-appv-self",
-		Status:          "pending",
-		ApprovalToken:   config.HashApprovalToken("correct-token"),
-		CreatedByUserID: &creatorID,
-		Recommendations: []config.RecommendationRecord{
-			{CloudAccountID: &accountID},
-		},
+	cases := []struct {
+		name         string
+		storedEmail  string // creator's user email and the account contact_email
+		messageActor string
+	}{
+		{name: "byte-identical emails", storedEmail: "owner@example.com", messageActor: "owner@example.com"},
+		{name: "stored mixed case, actor lowercase", storedEmail: "Owner@Example.COM", messageActor: "owner@example.com"},
 	}
-	account := &config.CloudAccount{ID: accountID, ContactEmail: creatorEmail}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			mockStore := new(MockConfigStore)
+			mockStore.Test(t) // fail this row on an unexpected call, don't panic
+			mockEmail := new(MockEmailSender)
 
-	// verifyAsyncApprovalActor + the manager's own fetch inside ApproveExecution
-	// both load the execution; enforceFourEyesPolicy loads it again.
-	mockStore.On("GetExecutionByID", ctx, "exec-appv-self").Return(exec, nil)
-	mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil)
-	mockStore.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil)
-	mockStore.On("GetUserEmailByID", ctx, creatorID).Return(creatorEmail, nil)
+			accountID := "acct-1"
+			creatorID := "user-creator"
+			exec := &config.PurchaseExecution{
+				ExecutionID:     "exec-appv-self",
+				Status:          "pending",
+				ApprovalToken:   config.HashApprovalToken("correct-token"),
+				CreatedByUserID: &creatorID,
+				Recommendations: []config.RecommendationRecord{
+					{CloudAccountID: &accountID},
+				},
+			}
+			account := &config.CloudAccount{ID: accountID, ContactEmail: tc.storedEmail}
 
-	manager := &Manager{
-		config:       mockStore,
-		email:        mockEmail,
-		dashboardURL: "https://dashboard.example.com",
+			// verifyAsyncApprovalActor + the manager's own fetch inside
+			// ApproveExecution both load the execution; enforceFourEyesPolicy
+			// loads it again.
+			mockStore.On("GetExecutionByID", ctx, "exec-appv-self").Return(exec, nil)
+			mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil)
+			mockStore.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil)
+			mockStore.On("GetUserEmailByID", ctx, creatorID).Return(tc.storedEmail, nil)
+
+			manager := &Manager{
+				config:       mockStore,
+				email:        mockEmail,
+				dashboardURL: "https://dashboard.example.com",
+			}
+
+			msg := fmt.Sprintf(`{"type":"approve","execution_id":"exec-appv-self","token":"correct-token","actor_email":%q}`, tc.messageActor)
+			err := manager.ProcessMessage(ctx, msg)
+			require.ErrorIs(t, err, ErrFourEyesDenied)
+			assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
+			mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			mockStore.AssertExpectations(t)
+			mockEmail.AssertExpectations(t)
+		})
 	}
+}
 
-	err := manager.ProcessMessage(ctx, `{"type":"approve","execution_id":"exec-appv-self","token":"correct-token","actor_email":"owner@example.com"}`)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
-	mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
-		mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
-	mockStore.AssertExpectations(t)
-	mockEmail.AssertExpectations(t)
+// TestProcessMessage_ApproverMatchIgnoresEmailCase pins the case-insensitive
+// comparison in matchActorAgainstApprovers (issue #307): an SQS approve or
+// cancel message whose actor_email differs from the account's stored
+// contact_email only in letter case is the authorized approver and must get
+// past verifyAsyncApprovalActor. One row per side of the comparison, so
+// dropping the strings.ToLower on either the actor or the stored address
+// fails a row. The non-matching control
+// (TestProcessMessage_ApproveRejectsNonMatchingActor) keeps an "accept
+// everyone" regression from passing.
+func TestProcessMessage_ApproverMatchIgnoresEmailCase(t *testing.T) {
+	cases := []struct {
+		name         string
+		contactEmail string
+		messageActor string
+	}{
+		{name: "contact_email mixed case, actor lowercase", contactEmail: "Owner@Example.COM", messageActor: "owner@example.com"},
+		{name: "contact_email lowercase, actor mixed case", contactEmail: "owner@example.com", messageActor: "OWNER@Example.com"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			mockStore := new(MockConfigStore)
+			mockStore.Test(t) // fail this row on an unexpected call, don't panic
+
+			accountID := "acct-1"
+			exec := &config.PurchaseExecution{
+				ExecutionID:   "exec-case",
+				Status:        "pending",
+				ApprovalToken: config.HashApprovalToken("correct-token"),
+				Recommendations: []config.RecommendationRecord{
+					{CloudAccountID: &accountID},
+				},
+			}
+			mockStore.On("GetExecutionByID", ctx, "exec-case").Return(exec, nil)
+			mockStore.On("GetCloudAccount", ctx, accountID).
+				Return(&config.CloudAccount{ID: accountID, ContactEmail: tc.contactEmail}, nil)
+
+			manager := &Manager{config: mockStore, email: new(MockEmailSender)}
+			msg := &AsyncMessage{ExecutionID: "exec-case", Token: "correct-token", ActorEmail: tc.messageActor}
+			require.NoError(t, manager.verifyAsyncApprovalActor(ctx, msg))
+			mockStore.AssertExpectations(t)
+		})
+	}
 }
 
 // TestProcessMessage_ApproveFourEyesOn_DifferentApproverSucceeds is the