From 1c450dbf674bc62bf34e6ed2292925e2d10f7ee9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 6 Oct 2026 19:50:50 +0200 Subject: [PATCH 1/2] fix(api): resolve analytics and inventory accounts by provider and external id Analytics scope checks and the inventory account name read a name map keyed by external id alone, so two accounts of different providers sharing an external id made the result depend on account list order: a raw account_id was allowed or refused by whichever account was listed last and then queried every provider's rows, and inventory rows showed another account's name. Analytics now attributes a raw external id only when exactly one account has it and refuses an ambiguous one; inventory resolves the name with the provider-qualified helper History uses (UUID, then provider plus external id, then a unique legacy match). Does not change AccountScope.Allows or name matching (#433). Refs #520 --- internal/api/handler_analytics.go | 36 ++++- .../handler_analytics_scope_provider_test.go | 134 +++++++++++++++++ internal/api/handler_inventory.go | 6 +- .../handler_inventory_scope_provider_test.go | 139 ++++++++++++++++++ internal/api/handler_inventory_test.go | 8 +- 5 files changed, 315 insertions(+), 8 deletions(-) create mode 100644 internal/api/handler_analytics_scope_provider_test.go create mode 100644 internal/api/handler_inventory_scope_provider_test.go diff --git a/internal/api/handler_analytics.go b/internal/api/handler_analytics.go index 4778b0311..93f8c11e7 100644 --- a/internal/api/handler_analytics.go +++ b/internal/api/handler_analytics.go @@ -7,6 +7,8 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/analytics" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-lambda-go/events" ) @@ -241,13 +243,43 @@ func (h *Handler) validateAnalyticsAccountScope(ctx context.Context, session *Se if accountID == "" { return NewClientError(400, "account_id is required for scoped users") } - nameByID := h.resolveAccountNamesByID(ctx) - if !allowed.Allows(accountID, nameByID[accountID]) { + // A failed account load leaves no names, so only a literal id match in the + // allow-list passes. + accounts, _ := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) + if !analyticsAccountAllowed(allowed, accounts, accountID) { return errNotFound } return nil } +// analyticsAccountAllowed reports whether the scope allows the requested +// account_id. A registered account UUID is authoritative. Otherwise the value +// is a raw external id, which is unique only per provider, so it is attributed +// to an account only when exactly one registered account has it; an id shared +// across providers (or within one) cannot be attributed and is refused, rather +// than read through a name that depends on account list order. +func analyticsAccountAllowed(allowed auth.AccountScope, accounts []config.CloudAccount, accountID string) bool { + for i := range accounts { + if accounts[i].ID == accountID { + return allowed.Allows(accountID, accounts[i].Name) + } + } + var match *config.CloudAccount + for i := range accounts { + if accounts[i].ExternalID != accountID { + continue + } + if match != nil { + return false + } + match = &accounts[i] + } + if match == nil { + return allowed.Allows(accountID, "") + } + return allowed.Allows(accountID, match.Name) +} + // triggerAnalyticsCollection handles POST /analytics/collect (admin only) // This can be used to manually trigger the hourly collection. func (h *Handler) triggerAnalyticsCollection(ctx context.Context, req *events.LambdaFunctionURLRequest, _ map[string]string) (any, error) { diff --git a/internal/api/handler_analytics_scope_provider_test.go b/internal/api/handler_analytics_scope_provider_test.go new file mode 100644 index 000000000..ddb21530f --- /dev/null +++ b/internal/api/handler_analytics_scope_provider_test.go @@ -0,0 +1,134 @@ +package api + +import ( + "context" + "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" +) + +// analyticsScopeCall drives GET /api/history/analytics through HandleRequest +// as a session user whose allow-list is `scope`, requesting `accountID`. It +// returns the HTTP status and the (uuid, provider-bucketed external id) filter +// that reached the analytics query, or nil filters when no query ran. +func analyticsScopeCall(t *testing.T, scope []string, accountID string, accounts []config.CloudAccount) (status int, uuids []string, externals map[string][]string) { + t.Helper() + 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) + + client := new(MockAnalyticsClient) + client.On("QueryHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]HistoryDataPoint{}, &HistorySummary{}, nil) + + store := new(MockConfigStore) + store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return accounts, nil + } + + h := &Handler{auth: mockAuth, analyticsClient: client, config: store} + resp, err := h.HandleRequest(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{"account_id": accountID}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/history/analytics"}, + }, + }) + require.NoError(t, err) + for _, c := range client.Calls { + if c.Method == "QueryHistory" { + uuids, _ = c.Arguments.Get(1).([]string) + externals, _ = c.Arguments.Get(2).(map[string][]string) + } + } + return resp.StatusCode, uuids, externals +} + +// Issue #520: a raw external id shared by accounts of different providers +// cannot be attributed to one of them. The scope decision used to read the +// account name from a map keyed by external id alone (last listed account +// wins), so it depended on list order, and an allowed request then queried +// every provider's rows for that id. +func TestHandleRequest_Analytics_RawExternalIDSharedAcrossProviders(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"} + bothOrders := map[string][]config.CloudAccount{ + "aws listed first": {awsDev, azProd}, + "azure listed first": {azProd, awsDev}, + } + + for order, accounts := range bothOrders { + for _, scope := range [][]string{{"dev"}, {"prod"}, {"123"}} { + t.Run(scope[0]+" scope, "+order, func(t *testing.T) { + t.Parallel() + status, uuids, externals := analyticsScopeCall(t, scope, "123", accounts) + assert.Equal(t, 404, status, "ambiguous raw id must fail closed") + assert.Nil(t, uuids) + assert.Nil(t, externals, "no query may run for an ambiguous raw id") + }) + } + + t.Run("account UUID resolves to its own provider, "+order, func(t *testing.T) { + t.Parallel() + status, uuids, externals := analyticsScopeCall(t, []string{"dev"}, "u-aws", accounts) + assert.Equal(t, 200, status) + assert.Equal(t, []string{"u-aws"}, uuids) + assert.Equal(t, map[string][]string{"aws": {"123"}}, externals) + + status, _, _ = analyticsScopeCall(t, []string{"dev"}, "u-az", accounts) + assert.Equal(t, 404, status, "the other provider's account stays out of scope") + }) + } +} + +// A raw external id that exactly one account owns still works for a scoped +// user, by name or by id, and an out-of-scope account stays hidden. +func TestHandleRequest_Analytics_UniqueRawExternalID(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"} + accounts := []config.CloudAccount{in, out} + + status, _, externals := analyticsScopeCall(t, []string{"in"}, "111", accounts) + assert.Equal(t, 200, status) + assert.Equal(t, map[string][]string{"": {"111"}}, externals, "legacy raw-id filter shape is unchanged") + + status, _, _ = analyticsScopeCall(t, []string{"111"}, "111", accounts) + assert.Equal(t, 200, status) + + status, _, _ = analyticsScopeCall(t, []string{"in"}, "222", accounts) + assert.Equal(t, 404, status) +} + +// Two accounts of one provider sharing an external id are ambiguous too. +func TestHandleRequest_Analytics_SameProviderAmbiguousRawExternalID(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"} + for _, accounts := range [][]config.CloudAccount{{a, b}, {b, a}} { + for _, scope := range []string{"dev", "prod"} { + status, _, externals := analyticsScopeCall(t, []string{scope}, "123", accounts) + assert.Equal(t, 404, status, scope) + assert.Nil(t, externals) + } + } +} + +// Unrestricted sessions are not narrowed: any account_id passes through. +func TestHandleRequest_Analytics_UnrestrictedRawExternalID(t *testing.T) { + t.Parallel() + accounts := []config.CloudAccount{ + {ID: "u-aws", Name: "dev", Provider: "aws", ExternalID: "123"}, + {ID: "u-az", Name: "prod", Provider: "azure", ExternalID: "123"}, + } + status, _, externals := analyticsScopeCall(t, nil, "123", accounts) + assert.Equal(t, 200, status) + assert.Equal(t, map[string][]string{"": {"123"}}, externals) +} diff --git a/internal/api/handler_inventory.go b/internal/api/handler_inventory.go index 7a2bba928..4b0fce226 100644 --- a/internal/api/handler_inventory.go +++ b/internal/api/handler_inventory.go @@ -49,7 +49,9 @@ func (h *Handler) listActiveCommitments(ctx context.Context, req *events.LambdaF return nil, err } - nameByID := h.resolveAccountNamesByID(ctx) + // A failed account load leaves the name blank rather than failing a read + // that is already scoped. + accounts, _ := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) commitments := make([]InventoryCommitment, 0, len(purchases)) for _rvc := range purchases { @@ -57,7 +59,7 @@ func (h *Handler) listActiveCommitments(ctx context.Context, req *events.LambdaF if !isActiveCommitment(p, now) { continue } - commitments = append(commitments, buildInventoryCommitment(p, nameByID[p.AccountID])) + commitments = append(commitments, buildInventoryCommitment(p, historyRowAccountName(accounts, p))) } // Soonest-expiring first. The dashboard framing is "what do I need to diff --git a/internal/api/handler_inventory_scope_provider_test.go b/internal/api/handler_inventory_scope_provider_test.go new file mode 100644 index 000000000..19ec5a10d --- /dev/null +++ b/internal/api/handler_inventory_scope_provider_test.go @@ -0,0 +1,139 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + "time" + + "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" +) + +// inventoryNames drives GET /api/inventory/commitments through HandleRequest +// as a session user whose allow-list is `scope` (nil means unrestricted) and +// returns purchase id -> account name for the rows that come back. +func inventoryNames(t *testing.T, scope []string, accounts []config.CloudAccount, rows []config.PurchaseHistoryRecord) map[string]string { + t.Helper() + 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("GetActivePurchaseHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(append([]config.PurchaseHistoryRecord(nil), rows...), nil) + store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return accounts, nil + } + + h := &Handler{auth: mockAuth, config: store} + resp, err := h.HandleRequest(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/inventory/commitments"}, + }, + }) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode, "body: %s", resp.Body) + + var body InventoryCommitmentsResponse + require.NoError(t, json.Unmarshal([]byte(resp.Body), &body)) + names := make(map[string]string, len(body.Commitments)) + for _, c := range body.Commitments { + names[c.ID] = c.AccountName + } + return names +} + +func activeRow(purchaseID, provider, accountID string) config.PurchaseHistoryRecord { + return config.PurchaseHistoryRecord{ + PurchaseID: purchaseID, + Provider: provider, + AccountID: accountID, + Service: "ec2", + Count: 1, + Term: 3, + Timestamp: time.Now().AddDate(0, -1, 0), + Status: "completed", + } +} + +// Issue #520: external ids are unique per (provider, external_id) only. The +// inventory account name was keyed by external id alone, so with two accounts +// sharing an id the name on a row (and the name-scope decision made with it) +// depended on the order the accounts were listed in. +func TestHandleRequest_Inventory_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"} + rows := []config.PurchaseHistoryRecord{activeRow("aws-row", "aws", "123"), activeRow("az-row", "azure", "123")} + bothOrders := map[string][]config.CloudAccount{ + "aws listed first": {awsDev, azProd}, + "azure listed first": {azProd, awsDev}, + } + + for order, accounts := range bothOrders { + t.Run("scope names aws account, "+order, func(t *testing.T) { + t.Parallel() + got := inventoryNames(t, []string{"dev"}, accounts, rows) + assert.Equal(t, map[string]string{"123:aws-row": "dev"}, got) + }) + t.Run("scope names azure account, "+order, func(t *testing.T) { + t.Parallel() + got := inventoryNames(t, []string{"prod"}, accounts, rows) + assert.Equal(t, map[string]string{"123:az-row": "prod"}, got) + }) + t.Run("unrestricted sees each row under its own account name, "+order, func(t *testing.T) { + t.Parallel() + got := inventoryNames(t, nil, accounts, rows) + assert.Equal(t, map[string]string{"123:aws-row": "dev", "123:az-row": "prod"}, got) + }) + } +} + +// A set CloudAccountID and an account UUID in AccountID stay authoritative, +// and an external id that is unique still resolves. +func TestHandleRequest_Inventory_ResolutionOrder(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"} + solo := config.CloudAccount{ID: "u-solo", Name: "solo", Provider: "gcp", ExternalID: "777"} + accounts := []config.CloudAccount{awsDev, azProd, solo} + + uuid := "u-az" + byCloudID := activeRow("by-cloud-id", "aws", "123") + byCloudID.CloudAccountID = &uuid + rows := []config.PurchaseHistoryRecord{ + byCloudID, + activeRow("by-uuid", "aws", "u-aws"), + activeRow("unique", "gcp", "777"), + } + got := inventoryNames(t, nil, accounts, rows) + assert.Equal(t, map[string]string{ + "123:by-cloud-id": "prod", + "u-aws:by-uuid": "dev", + "777:unique": "solo", + }, got) +} + +// Two accounts of one provider sharing an external id are ambiguous: the row +// is attributed to neither, so a name-scoped user does not see it and an +// unrestricted user sees it without a name. +func TestHandleRequest_Inventory_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{activeRow("amb", "aws", "123")} + for _, accounts := range [][]config.CloudAccount{{a, b}, {b, a}} { + for _, scope := range []string{"dev", "prod"} { + assert.Empty(t, inventoryNames(t, []string{scope}, accounts, rows), scope) + } + assert.Equal(t, map[string]string{"123:amb": ""}, inventoryNames(t, nil, accounts, rows)) + } +} diff --git a/internal/api/handler_inventory_test.go b/internal/api/handler_inventory_test.go index 81ccdfe90..2fb885170 100644 --- a/internal/api/handler_inventory_test.go +++ b/internal/api/handler_inventory_test.go @@ -97,13 +97,13 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) { mockStore.On("GetActivePurchaseHistory", ctx, mock.AnythingOfType("time.Time"), []string(nil), map[string][]string(nil)).Return(purchases, nil) // Use realistic fixtures: CloudAccount.ID is a UUID; CloudAccount.ExternalID is // the provider external ID (e.g. AWS account number) that matches - // PurchaseHistoryRecord.AccountID. The name lookup in resolveAccountNamesByID - // must key by ExternalID, not UUID, to find the name — this is the regression + // PurchaseHistoryRecord.AccountID. The name lookup must resolve by provider plus + // ExternalID, not UUID, to find the name; this is the regression // guard for issue #952. mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { return []config.CloudAccount{ - {ID: "11111111-0000-0000-0000-000000000001", ExternalID: "acc-active", Name: "Active Account"}, - {ID: "11111111-0000-0000-0000-000000000002", ExternalID: "acc-expired", Name: "Expired Account"}, + {ID: "11111111-0000-0000-0000-000000000001", Provider: "aws", ExternalID: "acc-active", Name: "Active Account"}, + {ID: "11111111-0000-0000-0000-000000000002", Provider: "aws", ExternalID: "acc-expired", Name: "Expired Account"}, }, nil } From 387c180454c559fd7b3588778bb937c907e14ce3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 6 Oct 2026 20:03:37 +0200 Subject: [PATCH 2/2] fix(api): fail loudly when the account list cannot be loaded Analytics scope checks and the inventory handler ignored a ListCloudAccounts error. Return it instead, and cover the unregistered raw id branch. Refs #520 --- internal/api/handler_analytics.go | 7 ++-- .../handler_analytics_scope_provider_test.go | 38 +++++++++++++++++++ internal/api/handler_inventory.go | 8 ++-- .../handler_inventory_scope_provider_test.go | 25 ++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/internal/api/handler_analytics.go b/internal/api/handler_analytics.go index 93f8c11e7..770878b04 100644 --- a/internal/api/handler_analytics.go +++ b/internal/api/handler_analytics.go @@ -243,9 +243,10 @@ func (h *Handler) validateAnalyticsAccountScope(ctx context.Context, session *Se if accountID == "" { return NewClientError(400, "account_id is required for scoped users") } - // A failed account load leaves no names, so only a literal id match in the - // allow-list passes. - accounts, _ := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) + accounts, listErr := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) + if listErr != nil { + return fmt.Errorf("failed to list cloud accounts: %w", listErr) + } if !analyticsAccountAllowed(allowed, accounts, accountID) { return errNotFound } diff --git a/internal/api/handler_analytics_scope_provider_test.go b/internal/api/handler_analytics_scope_provider_test.go index ddb21530f..7655b752e 100644 --- a/internal/api/handler_analytics_scope_provider_test.go +++ b/internal/api/handler_analytics_scope_provider_test.go @@ -132,3 +132,41 @@ func TestHandleRequest_Analytics_UnrestrictedRawExternalID(t *testing.T) { assert.Equal(t, 200, status) assert.Equal(t, map[string][]string{"": {"123"}}, externals) } + +// An id that matches no registered account is judged on the literal allow-list +// entry alone. +func TestHandleRequest_Analytics_UnregisteredRawID(t *testing.T) { + t.Parallel() + accounts := []config.CloudAccount{{ID: "u-solo", Name: "solo", Provider: "aws", ExternalID: "111"}} + + status, _, _ := analyticsScopeCall(t, []string{"999"}, "999", accounts) + assert.Equal(t, 200, status) + + status, _, _ = analyticsScopeCall(t, []string{"solo"}, "999", accounts) + assert.Equal(t, 404, status) +} + +// A failed account load must not fall back to literal-id matching. +func TestHandleRequest_Analytics_AccountListFailureIsAnError(t *testing.T) { + t.Parallel() + 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([]string{"999"}, nil) + store := new(MockConfigStore) + store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return nil, assert.AnError + } + client := new(MockAnalyticsClient) + + h := &Handler{auth: mockAuth, analyticsClient: client, config: store} + resp, _ := h.HandleRequest(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{"account_id": "999"}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/history/analytics"}, + }, + }) + assert.GreaterOrEqual(t, resp.StatusCode, 500) + client.AssertNotCalled(t, "QueryHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} diff --git a/internal/api/handler_inventory.go b/internal/api/handler_inventory.go index 4b0fce226..d2435bcce 100644 --- a/internal/api/handler_inventory.go +++ b/internal/api/handler_inventory.go @@ -3,6 +3,7 @@ package api import ( "context" + "fmt" "sort" "time" @@ -49,9 +50,10 @@ func (h *Handler) listActiveCommitments(ctx context.Context, req *events.LambdaF return nil, err } - // A failed account load leaves the name blank rather than failing a read - // that is already scoped. - accounts, _ := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) + accounts, listErr := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{}) + if listErr != nil { + return nil, fmt.Errorf("failed to list cloud accounts: %w", listErr) + } commitments := make([]InventoryCommitment, 0, len(purchases)) for _rvc := range purchases { diff --git a/internal/api/handler_inventory_scope_provider_test.go b/internal/api/handler_inventory_scope_provider_test.go index 19ec5a10d..cb005663d 100644 --- a/internal/api/handler_inventory_scope_provider_test.go +++ b/internal/api/handler_inventory_scope_provider_test.go @@ -137,3 +137,28 @@ func TestHandleRequest_Inventory_SameProviderAmbiguousExternalID(t *testing.T) { assert.Equal(t, map[string]string{"123:amb": ""}, inventoryNames(t, nil, accounts, rows)) } } + +// A failed account load is an error, not a blank account name. +func TestHandleRequest_Inventory_AccountListFailureIsAnError(t *testing.T) { + t.Parallel() + 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([]string(nil), nil) + store := new(MockConfigStore) + store.On("GetActivePurchaseHistory", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]config.PurchaseHistoryRecord{activeRow("r", "aws", "123")}, nil) + store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return nil, assert.AnError + } + + h := &Handler{auth: mockAuth, config: store} + resp, _ := h.HandleRequest(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer tok"}, + QueryStringParameters: map[string]string{}, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "GET", Path: "/api/inventory/commitments"}, + }, + }) + assert.GreaterOrEqual(t, resp.StatusCode, 500) +}