refactor(auth): represent account scope as a type whose zero value denies

internal/api/account_scope_fail_closed_test.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -31,10 +30,11 @@ func TestGetAllowedAccounts_FailsClosedWhenAuthMissing(t *testing.T) {
 	ctx := context.Background()
 	h := &Handler{auth: nil}
 
-	got, err := h.getAllowedAccounts(ctx, &Session{UserID: scopeSessionUser})
+	got, err := h.getAccountScope(ctx, &Session{UserID: scopeSessionUser})
 
 	require.Error(t, err, "a nil auth service must refuse, not grant unrestricted access")
-	assert.Nil(t, got)
+	assert.False(t, got.AllowsAll(), "the scope returned alongside the error must not be unrestricted")
+	assert.False(t, got.Allows("any-account", ""), "and must grant no account at all")
 	assert.Contains(t, err.Error(), "cannot establish account scope")
 }
 
@@ -49,10 +49,11 @@ func TestGetAllowedAccounts_PropagatesResolverFailure(t *testing.T) {
 	m.On("GetAllowedAccountsAPI", ctx, scopeSessionUser).Return([]string(nil), boom)
 
 	h := &Handler{auth: m}
-	got, err := h.getAllowedAccounts(ctx, &Session{UserID: scopeSessionUser})
+	got, err := h.getAccountScope(ctx, &Session{UserID: scopeSessionUser})
 
 	require.Error(t, err)
-	assert.Nil(t, got)
+	assert.False(t, got.AllowsAll(), "a failed resolution must not yield an unrestricted scope")
+	assert.False(t, got.Allows("any-account", ""))
 }
 
 // End-to-end through the shared scoping seam every scoped handler uses: an
@@ -84,10 +85,10 @@ func TestGetAllowedAccounts_AdminAPIKeyStillUnrestricted(t *testing.T) {
 	ctx := context.Background()
 	h := &Handler{auth: new(MockAuthService)}
 
-	got, err := h.getAllowedAccounts(ctx, &Session{UserID: apiKeyAdminUserID})
+	got, err := h.getAccountScope(ctx, &Session{UserID: apiKeyAdminUserID})
 
 	require.NoError(t, err, "the admin API key must remain unrestricted")
-	assert.True(t, auth.IsUnrestrictedAccess(got))
+	assert.True(t, got.AllowsAll(), "expressed as a SET flag, never as an empty list")
 }
 
 // The other two legitimate unrestricted principals, resolved through the auth
@@ -111,10 +112,10 @@ func TestGetAllowedAccounts_LegitimateUnrestrictedPrincipalsPass(t *testing.T) {
 			m.On("GetAllowedAccountsAPI", ctx, scopeSessionUser).Return(tc.resolved, nil)
 
 			h := &Handler{auth: m}
-			got, err := h.getAllowedAccounts(ctx, &Session{UserID: scopeSessionUser})
+			got, err := h.getAccountScope(ctx, &Session{UserID: scopeSessionUser})
 
 			require.NoError(t, err, "a successfully resolved scope must not be refused")
-			assert.True(t, auth.IsUnrestrictedAccess(got),
+			assert.True(t, got.AllowsAll(),
 				"a successful resolution to an empty/wildcard scope still means all accounts")
 		})
 	}

internal/api/grantscoped_test.go

@@ -19,7 +19,7 @@ import (
 // are green": the suite had no restriction to enforce.
 //
 // These pin the shared seam every scoped handler funnels through
-// (getAllowedAccounts -> requireAccountAccess / requirePlanAccess), so a
+// (getAccountScope -> requireAccountAccess / requirePlanAccess), so a
 // regression in the seam fails here rather than silently in production.
 
 const (

internal/api/handler.go

@@ -221,7 +221,7 @@ func NewHandler(cfg HandlerConfig) *Handler {
 // API-key session. It has no backing user row in the auth store, so any code
 // that would otherwise resolve group-derived permissions for it must treat it
 // as full-access up front (the API key is an infrastructure credential, not a
-// user). See requirePermission / requireAdmin / getAllowedAccounts.
+// user). See requirePermission / requireAdmin / getAccountScope.
 // It is also the actor identity threaded into the group-write grant ceiling,
 // which recognizes it and measures the key against a bare {admin, *} holding
 // (auth.AdminAPIKeyActorID) rather than failing the user lookup.
@@ -523,22 +523,41 @@ func (h *Handler) requirePermissionConstraints(ctx context.Context, session *Ses
 	return nil
 }
 
-// getAllowedAccounts returns the list of account IDs the user is allowed to
-// access. Empty slice means all access (Administrators-group members carry the
-// "*" wildcard, which GetAllowedAccountsAPI surfaces as unrestricted). The
-// stateless admin API key has no user row, so it short-circuits to all access.
-func (h *Handler) getAllowedAccounts(ctx context.Context, session *Session) ([]string, error) {
+// getAccountScope returns the auth.AccountScope describing which cloud
+// accounts the session may reach.
+//
+// The scope is a value, not a list, and its ZERO FORM DENIES: an
+// AccountScope{} grants no account at all. That is the inverse of the
+// []string it replaced, where an empty slice meant ALL access -- the
+// representation confusion behind issue #1748. Unrestricted access is carried
+// as an explicit flag and is only ever set positively, here for the stateless
+// admin API key (an infrastructure credential with no user row) and by
+// auth.ScopeFromLegacyList for a resolved list that is empty or carries "*".
+//
+// Fails closed: every path that cannot establish the scope returns an error
+// alongside a zero AccountScope, so a caller that ignores the error still
+// denies.
+func (h *Handler) getAccountScope(ctx context.Context, session *Session) (auth.AccountScope, error) {
 	if session.UserID == apiKeyAdminUserID {
-		return nil, nil // stateless admin API key = all access
+		// Positively unrestricted: the stateless admin API key is an
+		// infrastructure credential with no user row. Expressed as a set flag,
+		// never as an empty list (issue #1748).
+		return auth.UnrestrictedScope(), nil
 	}
 	if h.auth == nil {
-		// Fail closed. Returning an empty list here meant "all accounts", so a
-		// handler running without an auth service granted every caller access
-		// to every cloud account (issue #1748). Auth components must fail
-		// closed when nil, never fall through.
-		return nil, fmt.Errorf("authentication service not configured: cannot establish account scope")
+		// Fail closed. Auth components must fail closed when nil, never fall
+		// through. The zero AccountScope returned alongside this error denies
+		// everything even if a caller ignores the error.
+		return auth.AccountScope{}, fmt.Errorf("authentication service not configured: cannot establish account scope")
+	}
+	resolved, err := h.auth.GetAllowedAccountsAPI(ctx, session.UserID)
+	if err != nil {
+		return auth.AccountScope{}, err
 	}
-	return h.auth.GetAllowedAccountsAPI(ctx, session.UserID)
+	// Safe here and only here: GetAllowedAccountsAPI has already failed closed
+	// on an unestablishable scope, so an empty list at this point genuinely
+	// means "no restriction configured".
+	return auth.ScopeFromLegacyList(resolved), nil
 }
 
 // setSecurityHeaders adds comprehensive security headers to the response.

internal/api/handler_accounts.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/oauth2"
 
 	"github.com/LeanerCloud/CUDly/internal/accounts"
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/oidc"
@@ -105,15 +104,15 @@ func (h *Handler) listAccounts(ctx context.Context, req *events.LambdaFunctionUR
 	// Filter by allowed accts if the user has restricted access.
 	// An empty list or one containing "*" grants unrestricted access.
 	// Otherwise each entry is matched against the account's ID or Name.
-	allowedAccounts, err := h.getAllowedAccounts(ctx, session)
+	allowedAccounts, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if !auth.IsUnrestrictedAccess(allowedAccounts) {
+	if !allowedAccounts.AllowsAll() {
 		filtered := accts[:0]
 		for _rvc := range accts {
 			acct := accts[_rvc]
-			if auth.MatchesAccount(allowedAccounts, acct.ID, acct.Name) {
+			if allowedAccounts.Allows(acct.ID, acct.Name) {
 				filtered = append(filtered, acct)
 			}
 		}
@@ -162,18 +161,18 @@ func (h *Handler) listAccountsMinimal(ctx context.Context, req *events.LambdaFun
 		return nil, fmt.Errorf("accounts: %w", err)
 	}
 
-	allowedAccounts, err := h.getAllowedAccounts(ctx, session)
+	allowedAccounts, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	unrestricted := auth.IsUnrestrictedAccess(allowedAccounts)
+	unrestricted := allowedAccounts.AllowsAll()
 
 	// Build the minimal projection in place, applying allowed_accounts scoping
 	// during the copy so a restricted user only ever sees their entitled rows.
 	summaries := make([]AccountSummary, 0, len(accts))
 	for i := range accts {
 		acct := &accts[i]
-		if !unrestricted && !auth.MatchesAccount(allowedAccounts, acct.ID, acct.Name) {
+		if !unrestricted && !allowedAccounts.Allows(acct.ID, acct.Name) {
 			continue
 		}
 		summaries = append(summaries, AccountSummary{

internal/api/handler_analytics.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/LeanerCloud/CUDly/internal/analytics"
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/aws/aws-lambda-go/events"
 )
 
@@ -232,18 +231,18 @@ func (h *Handler) getHistoryBreakdown(ctx context.Context, req *events.LambdaFun
 // without account_id or for an account outside their allowed_accounts list.
 // Admin/unrestricted sessions pass through (account_id may be empty).
 func (h *Handler) validateAnalyticsAccountScope(ctx context.Context, session *Session, accountID string) error {
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if auth.IsUnrestrictedAccess(allowed) {
+	if allowed.AllowsAll() {
 		return nil
 	}
 	if accountID == "" {
 		return NewClientError(400, "account_id is required for scoped users")
 	}
 	nameByID := h.resolveAccountNamesByID(ctx)
-	if !auth.MatchesAccount(allowed, accountID, nameByID[accountID]) {
+	if !allowed.Allows(accountID, nameByID[accountID]) {
 		return errNotFound
 	}
 	return nil

internal/api/handler_dashboard.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/aws/aws-lambda-go/events"
@@ -129,7 +128,7 @@ func (h *Handler) resolveDashboardAccountScope(ctx context.Context, params map[s
 // into the dual-column purchase-history filter inputs so dashboard commitment
 // metrics and the inventory endpoints (fetchCommitmentRecords) never include
 // accounts the session can't access (issue #956). It lists
-// the cloud accounts, keeps those the session matches via auth.MatchesAccount,
+// the cloud accounts, keeps those the session scope allows (AccountScope.Allows),
 // and resolves their UUIDs through resolveAccountFilterIDs (same code path as an
 // explicit filter).
 //
@@ -139,11 +138,11 @@ func (h *Handler) resolveDashboardAccountScope(ctx context.Context, params map[s
 // so a scoped user with zero accessible accounts sees zeroed KPIs rather than
 // everyone's data.
 func (h *Handler) resolveAllowedAccountScope(ctx context.Context, session *Session) (uuids []string, externalIDsByProvider map[string][]string, err error) {
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if auth.IsUnrestrictedAccess(allowed) {
+	if allowed.AllowsAll() {
 		return nil, nil, nil
 	}
 	accounts, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{})
@@ -155,7 +154,7 @@ func (h *Handler) resolveAllowedAccountScope(ctx context.Context, session *Sessi
 	allowedUUIDs := []string{}
 	for _rvc := range accounts {
 		a := accounts[_rvc]
-		if auth.MatchesAccount(allowed, a.ID, a.Name) {
+		if allowed.Allows(a.ID, a.Name) {
 			allowedUUIDs = append(allowedUUIDs, a.ID)
 		}
 	}
@@ -167,11 +166,11 @@ func (h *Handler) resolveAllowedAccountScope(ctx context.Context, session *Sessi
 // to the recommendations list before aggregation so scoped users don't see
 // cross-account totals. Admin/unrestricted sessions pass through unchanged.
 func (h *Handler) filterDashboardRecommendations(ctx context.Context, session *Session, recs []config.RecommendationRecord) ([]config.RecommendationRecord, error) {
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if auth.IsUnrestrictedAccess(allowed) {
+	if allowed.AllowsAll() {
 		return recs, nil
 	}
 
@@ -182,7 +181,7 @@ func (h *Handler) filterDashboardRecommendations(ctx context.Context, session *S
 			continue
 		}
 		id := *recs[_rvc].CloudAccountID
-		if auth.MatchesAccount(allowed, id, nameByID[id]) {
+		if allowed.Allows(id, nameByID[id]) {
 			filtered = append(filtered, recs[_rvc])
 		}
 	}

internal/api/handler_history.go

@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/runtime"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
@@ -991,11 +990,11 @@ func appendMissing(dst []string, vals ...string) []string {
 // another user's multi-account in-flight row (including CreatedByUserEmail PII
 // and dollar amounts) is not visible to unrelated scoped users.
 func (h *Handler) filterPurchaseHistoryByAllowedAccounts(ctx context.Context, session *Session, purchases []config.PurchaseHistoryRecord) ([]config.PurchaseHistoryRecord, error) {
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if auth.IsUnrestrictedAccess(allowed) {
+	if allowed.AllowsAll() {
 		return purchases, nil
 	}
 	nameByID := h.resolveAccountNamesByID(ctx)
@@ -1013,7 +1012,7 @@ func (h *Handler) filterPurchaseHistoryByAllowedAccounts(ctx context.Context, se
 			}
 			continue
 		}
-		if auth.MatchesAccount(allowed, p.AccountID, nameByID[p.AccountID]) {
+		if allowed.Allows(p.AccountID, nameByID[p.AccountID]) {
 			filtered = append(filtered, p)
 		}
 	}

internal/api/handler_ladder.go

@@ -6,7 +6,6 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/aws/aws-lambda-go/events"
 )
@@ -47,18 +46,18 @@ func (h *Handler) getLadderConfigs(ctx context.Context, req *events.LambdaFuncti
 // outside the session's allowed_accounts. Admin/unrestricted sessions pass
 // through unchanged.
 func (h *Handler) filterLadderConfigsByAllowedAccounts(ctx context.Context, session *Session, configs []config.LadderConfigDB) ([]config.LadderConfigDB, error) {
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
-	if auth.IsUnrestrictedAccess(allowed) {
+	if allowed.AllowsAll() {
 		return configs, nil
 	}
 	nameByID := h.resolveAccountNamesByID(ctx)
 	filtered := make([]config.LadderConfigDB, 0, len(configs))
 	for _rvc := range configs {
 		c := configs[_rvc]
-		if auth.MatchesAccount(allowed, c.CloudAccountID, nameByID[c.CloudAccountID]) {
+		if allowed.Allows(c.CloudAccountID, nameByID[c.CloudAccountID]) {
 			filtered = append(filtered, c)
 		}
 	}

internal/api/handler_marketplace.go

@@ -427,19 +427,17 @@ func (h *Handler) authorizeAllowedAccount(ctx context.Context, session *Session,
 			return nil
 		}
 	}
-	allowed, err := h.getAllowedAccounts(ctx, session)
+	scope, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return fmt.Errorf("failed to check allowed accounts: %w", err)
 	}
-	// Empty list means "no restriction" (the user has access to all accounts).
-	if len(allowed) == 0 {
+	// Was a hand-rolled `len(allowed) == 0` meaning "no restriction", which
+	// read an unestablishable scope as unrestricted independently of any
+	// producer (issue #1748). AccountScope makes unrestricted an explicit
+	// flag, so the zero value denies.
+	if scope.Allows(cloudAccountID, "") {
 		return nil
 	}
-	for _, id := range allowed {
-		if id == "*" || id == cloudAccountID {
-			return nil
-		}
-	}
 	return NewClientError(403, "permission denied: purchase is in a cloud account not covered by your session's allowed accounts")
 }
 

internal/api/handler_purchases_revoke.go

@@ -391,11 +391,15 @@ func (h *Handler) checkRevokeOwnAccountAccess(ctx context.Context, userID string
 	if record.CloudAccountID == nil || *record.CloudAccountID == "" {
 		return NewClientError(403, "permission denied: cannot verify ownership for this purchase")
 	}
-	allowed, err := h.auth.GetAllowedAccountsAPI(ctx, userID)
+	// Goes through getAccountScope rather than calling GetAllowedAccountsAPI
+	// directly: the direct call skipped the admin-API-key and nil-auth
+	// branches, and the `len(allowed) > 0 &&` guard read an empty list as
+	// unrestricted independently of any producer (issue #1748).
+	scope, err := h.getAccountScope(ctx, &Session{UserID: userID})
 	if err != nil {
 		return fmt.Errorf("account access check failed: %w", err)
 	}
-	if len(allowed) > 0 && !stringInSlice(*record.CloudAccountID, allowed) {
+	if !scope.Allows(*record.CloudAccountID, "") {
 		return NewClientError(403, "permission denied: purchase is in an account you do not have access to")
 	}
 	return nil