fix(api): match history rows to scoped accounts by provider and external id

internal/api/handler_history.go

@@ -997,7 +997,10 @@ func (h *Handler) filterPurchaseHistoryByAllowedAccounts(ctx context.Context, se
 	if allowed.AllowsAll() {
 		return purchases, nil
 	}
-	nameByID := h.resolveAccountNamesByID(ctx)
+	accounts, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list cloud accounts: %w", err)
+	}
 	filtered := make([]config.PurchaseHistoryRecord, 0, len(purchases))
 	for _rvc := range purchases {
 		p := purchases[_rvc]
@@ -1012,13 +1015,43 @@ func (h *Handler) filterPurchaseHistoryByAllowedAccounts(ctx context.Context, se
 			}
 			continue
 		}
-		if allowed.Allows(p.AccountID, nameByID[p.AccountID]) {
+		if allowed.Allows(p.AccountID, historyRowAccountName(accounts, p)) {
 			filtered = append(filtered, p)
 		}
 	}
 	return filtered, nil
 }
 
+// historyRowAccountName returns the registered account name for a history row,
+// or "" when it cannot be determined unambiguously. Without CloudAccountID it
+// tries the account UUID first (in-flight rows store the UUID in AccountID),
+// then provider plus external id, since external ids are unique only per
+// provider. A legacy row without a provider matches only when exactly one
+// account across all providers has that external id.
+func historyRowAccountName(accounts []config.CloudAccount, p config.PurchaseHistoryRecord) string {
+	if p.CloudAccountID != nil {
+		return accountNameByID(accounts, *p.CloudAccountID)
+	}
+	if name := accountNameByID(accounts, p.AccountID); name != "" {
+		return name
+	}
+	if p.Provider != "" {
+		return accountNameByExternalID(accounts, p.Provider, p.AccountID)
+	}
+	var name string
+	matches := 0
+	for i := range accounts {
+		if accounts[i].ExternalID == p.AccountID {
+			name = accounts[i].Name
+			matches++
+		}
+	}
+	if matches != 1 {
+		return ""
+	}
+	return name
+}
+
 func summarizePurchaseHistory(purchases []config.PurchaseHistoryRecord) HistorySummary {
 	summary := HistorySummary{TotalPurchases: len(purchases), TotalUpfront: new(float64)}
 	for _rvc := range purchases {

internal/api/handler_history_scope_provider_test.go

@@ -0,0 +1,146 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"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"
+)
+
+// historyScopeIDs drives GET /api/history through HandleRequest as a session
+// user whose allow-list is `scope`, with the given accounts registered, and
+// returns the purchase ids that come back.
+func historyScopeIDs(t *testing.T, scope []string, accounts []config.CloudAccount, rows []config.PurchaseHistoryRecord) []string {
+	t.Helper()
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	mockAuth.On("ValidateSession", mock.Anything, "tok").Return(&Session{UserID: "u-1", Email: "u@example.com"}, nil)
+	mockAuth.On("HasPermissionAPI", mock.Anything, "u-1", mock.Anything, mock.Anything).Return(true, nil)
+	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, "u-1").Return(scope, nil)
+
+	store := new(MockConfigStore)
+	store.On("GetAllPurchaseHistory", mock.Anything, mock.Anything).Return(append([]config.PurchaseHistoryRecord(nil), rows...), nil)
+	store.On("GetExecutionsByStatuses", mock.Anything, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil)
+	store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) {
+		return accounts, nil
+	}
+
+	h := &Handler{auth: mockAuth, config: store}
+	resp, err := h.HandleRequest(ctx, &events.LambdaFunctionURLRequest{
+		Headers:               map[string]string{"Authorization": "Bearer tok"},
+		QueryStringParameters: map[string]string{},
+		RequestContext: events.LambdaFunctionURLRequestContext{
+			HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/history"},
+		},
+	})
+	require.NoError(t, err)
+	require.Equal(t, 200, resp.StatusCode, "body: %s", resp.Body)
+
+	var body HistoryResponse
+	require.NoError(t, json.Unmarshal([]byte(resp.Body), &body))
+	ids := make([]string, 0, len(body.Purchases))
+	for _, p := range body.Purchases {
+		ids = append(ids, p.PurchaseID)
+	}
+	return ids
+}
+
+// Issue #550: external ids are unique per (provider, external_id) only. A
+// name-scoped History user must see rows of the account in their scope and
+// none of the same-external-id account under another provider, whatever the
+// order the accounts are listed in.
+func TestHandleRequest_History_SameExternalIDAcrossProviders(t *testing.T) {
+	t.Parallel()
+	awsDev := config.CloudAccount{ID: "u-aws", Name: "dev", Provider: "aws", ExternalID: "123"}
+	azProd := config.CloudAccount{ID: "u-az", Name: "prod", Provider: "azure", ExternalID: "123"}
+	awsRow := config.PurchaseHistoryRecord{PurchaseID: "aws-row", Provider: "aws", AccountID: "123", Status: "completed"}
+	azRow := config.PurchaseHistoryRecord{PurchaseID: "az-row", Provider: "azure", AccountID: "123", Status: "completed"}
+	rows := []config.PurchaseHistoryRecord{awsRow, azRow}
+	bothOrders := map[string][]config.CloudAccount{
+		"aws listed first":   {awsDev, azProd},
+		"azure listed first": {azProd, awsDev},
+	}
+
+	for order, accounts := range bothOrders {
+		t.Run("scope names azure account, "+order, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, []string{"az-row"}, historyScopeIDs(t, []string{"prod"}, accounts, rows))
+		})
+		t.Run("scope names aws account, "+order, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, []string{"aws-row"}, historyScopeIDs(t, []string{"dev"}, accounts, rows))
+		})
+	}
+
+	t.Run("legacy row without provider on an ambiguous external id is not name-matched", func(t *testing.T) {
+		t.Parallel()
+		legacy := []config.PurchaseHistoryRecord{{PurchaseID: "legacy", AccountID: "123", Status: "completed"}}
+		assert.Empty(t, historyScopeIDs(t, []string{"prod"}, []config.CloudAccount{awsDev, azProd}, legacy))
+	})
+
+	t.Run("a set cloud account id stays authoritative", func(t *testing.T) {
+		t.Parallel()
+		uuid := "u-az"
+		row := []config.PurchaseHistoryRecord{{PurchaseID: "by-uuid", Provider: "aws", AccountID: "123", CloudAccountID: &uuid, Status: "completed"}}
+		assert.Equal(t, []string{"by-uuid"}, historyScopeIDs(t, []string{"prod"}, []config.CloudAccount{awsDev, azProd}, row))
+		assert.Empty(t, historyScopeIDs(t, []string{"dev"}, []config.CloudAccount{awsDev, azProd}, row))
+	})
+}
+
+// Unambiguous accounts keep matching by name or id, with and without a
+// provider on the row.
+func TestHandleRequest_History_UniqueExternalIDStillMatches(t *testing.T) {
+	t.Parallel()
+	in := config.CloudAccount{ID: "u-in", Name: "in", Provider: "aws", ExternalID: "111"}
+	out := config.CloudAccount{ID: "u-out", Name: "out", Provider: "aws", ExternalID: "222"}
+	rows := []config.PurchaseHistoryRecord{
+		{PurchaseID: "in-prov", Provider: "aws", AccountID: "111", Status: "completed"},
+		{PurchaseID: "in-legacy", AccountID: "111", Status: "completed"},
+		{PurchaseID: "out", Provider: "aws", AccountID: "222", Status: "completed"},
+	}
+	got := historyScopeIDs(t, []string{"in"}, []config.CloudAccount{in, out}, rows)
+	assert.ElementsMatch(t, []string{"in-prov", "in-legacy"}, got)
+	got = historyScopeIDs(t, []string{"111"}, []config.CloudAccount{in, out}, rows)
+	assert.ElementsMatch(t, []string{"in-prov", "in-legacy"}, got)
+}
+
+// In-flight history rows carry the account UUID in AccountID with no
+// CloudAccountID, so a name-scoped user must still see their own rows.
+func TestHandleRequest_History_UUIDInAccountID(t *testing.T) {
+	t.Parallel()
+	dev := config.CloudAccount{ID: "u-aws", Name: "dev", Provider: "aws", ExternalID: "123"}
+	prod := config.CloudAccount{ID: "u-az", Name: "prod", Provider: "azure", ExternalID: "123"}
+	accounts := []config.CloudAccount{dev, prod}
+
+	t.Run("pending row with provider", func(t *testing.T) {
+		t.Parallel()
+		rows := []config.PurchaseHistoryRecord{{PurchaseID: "p1", Provider: "aws", AccountID: "u-aws", Status: "pending"}}
+		assert.Equal(t, []string{"p1"}, historyScopeIDs(t, []string{"dev"}, accounts, rows))
+		assert.Empty(t, historyScopeIDs(t, []string{"prod"}, accounts, rows))
+	})
+	t.Run("pending row without provider", func(t *testing.T) {
+		t.Parallel()
+		rows := []config.PurchaseHistoryRecord{{PurchaseID: "p2", AccountID: "u-aws", Status: "pending"}}
+		assert.Equal(t, []string{"p2"}, historyScopeIDs(t, []string{"dev"}, accounts, rows))
+		assert.Empty(t, historyScopeIDs(t, []string{"prod"}, accounts, rows))
+	})
+}
+
+// Two accounts of one provider sharing an external id are ambiguous: the row
+// is hidden from both name scopes.
+func TestHandleRequest_History_SameProviderAmbiguousExternalID(t *testing.T) {
+	t.Parallel()
+	a := config.CloudAccount{ID: "u-a", Name: "dev", Provider: "aws", ExternalID: "123"}
+	b := config.CloudAccount{ID: "u-b", Name: "prod", Provider: "aws", ExternalID: "123"}
+	rows := []config.PurchaseHistoryRecord{{PurchaseID: "amb", Provider: "aws", AccountID: "123", Status: "completed"}}
+	for _, scope := range []string{"dev", "prod"} {
+		assert.Empty(t, historyScopeIDs(t, []string{scope}, []config.CloudAccount{a, b}, rows), scope)
+		assert.Empty(t, historyScopeIDs(t, []string{scope}, []config.CloudAccount{b, a}, rows), scope)
+	}
+}