refactor: simplify TokenRateLimitPolicy by trusting AuthPolicy validation (#543)

## Simplify TokenRateLimitPolicy by trusting AuthPolicy validation

**Jira**: https://redhat.atlassian.net/browse/RHOAIENG-53680
**jira**: https://redhat.atlassian.net/browse/RHOAIENG-53951

## Description

This PR removes ~80 lines of duplicate subscription validation logic
from the TokenRateLimitPolicy (TRLP) controller. The TRLP now trusts the
validated `auth.identity.selected_subscription` from AuthPolicy instead
of re-implementing membership checks, header validation, and deny rules.

We also now auto-select the subscription by model if the model is
available in only one subscription.

### Problem
The TRLP controller was duplicating validation that AuthPolicy already
performs:
- Checking if users belong to subscriptions (groups/users)
- Validating the `x-maas-subscription` header
- Creating 3+ deny rules per model

### Solution
AuthPolicy already validates subscriptions via
`/v1/subscriptions/select` and injects
`auth.identity.selected_subscription`. TRLP now simply uses this value.

**Before:**
```yaml
limits:
  sub-a-model-tokens:
    when:
      - predicate: auth.identity.groups_str.split(",").exists(g, g == "team-a")
      - predicate: request.headers["x-maas-subscription"] == "sub-a" || (!request.headers.exists(...) && !(exclusions...))
  deny-not-member-sub-a-model: [...]
  deny-unsubscribed-model: [...]
  deny-invalid-header-model: [...]
```

**After:**
```yaml
limits:
  sub-a-model-tokens:
    when:
      - predicate: auth.identity.selected_subscription == "sub-a"
```

### Changes
- Removed `buildMembershipCheck` function and validation logic (~80
lines)
- Simplified `subInfo` struct from 6 fields to 3
- Removed all deny rules
- Added 2 unit tests verifying simplified structure
- Updated E2E tests to expect 403 (not 429) for invalid subscriptions
- Enhanced logging with subscription count

**Result:** 75% fewer TRLP limit entries, 32% smaller function, better
error codes (403 vs 429)

## How Has This Been Tested?

### Unit Tests
```bash
cd maas-controller
go test ./pkg/controller/maas -run TestMaaSSubscription -v
```

All 7 tests pass (5 existing + 2 new):
- New: `TestMaaSSubscriptionReconciler_SimplifiedTRLP` - verifies single
predicate, no deny rules
- New: `TestMaaSSubscriptionReconciler_MultipleSubscriptionsSimplified`
- verifies no exclusion logic

### E2E Tests
```bash
cd test/e2e
pytest tests/test_subscription.py -v
```

Updated tests to expect 403 Forbidden (from AuthPolicy) instead of 429
(from TRLP) for subscription validation failures.

### Linter
```bash
golangci-lint run --new-from-rev=HEAD~1
```
No issues.

## Merge criteria:

- [x] The commits are squashed in a cohesive manner and have meaningful
messages.
- [x] Testing instructions have been added in the PR body (for PRs
involving changes that are not immediately obvious).
- [x] The developer has manually tested the changes and verified that
the changes work






<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Invalid, missing, or ambiguous subscription validation now returns 403
Forbidden (applied before rate limiting).

* **New Features**
* Explicit model selection in subscription flows with clear
"model_not_in_subscription" responses and per-model isolation for
policies.
* Namespace-scoped subscription isolation to prevent cross-tenant
collisions.
* Simplified per-subscription rate-limit generation relying on
auth-selected subscription keys.

* **Tests**
* Expanded unit and e2e coverage for simplified rate limits,
model-scoped behavior, cross-namespace isolation, 403 expectations, and
transient command retries.

* **Diagnostics**
* Enhanced debug/reporting (test user, subscription→model mappings,
model listings, configuration summary).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: jrhyness

maas-api/internal/handlers/models.go

@@ -78,7 +78,8 @@ func (h *ModelsHandler) selectSubscriptionsForListing(
 
 	// Single subscription selection (existing behavior)
 	if h.subscriptionSelector != nil {
-		result, err := h.subscriptionSelector.Select(userContext.Groups, userContext.Username, requestedSubscription)
+		//nolint:unqueryvet,nolintlint // Select is a method, not a SQL query
+		result, err := h.subscriptionSelector.Select(userContext.Groups, userContext.Username, requestedSubscription, "")
 		if err != nil {
 			h.handleSubscriptionSelectionError(c, err)
 			return nil, true
@@ -99,6 +100,7 @@ func (h *ModelsHandler) selectSubscriptionsForListing(
 // handleSubscriptionSelectionError handles errors from subscription selection and sends appropriate HTTP responses.
 func (h *ModelsHandler) handleSubscriptionSelectionError(c *gin.Context, err error) {
 	var multipleSubsErr *subscription.MultipleSubscriptionsError
+	var ambiguousErr *subscription.SubscriptionAmbiguousError
 	var accessDeniedErr *subscription.AccessDeniedError
 	var notFoundErr *subscription.SubscriptionNotFoundError
 	var noSubErr *subscription.NoSubscriptionError
@@ -117,6 +119,16 @@ func (h *ModelsHandler) handleSubscriptionSelectionError(c *gin.Context, err err
 		return
 	}
 
+	if errors.As(err, &ambiguousErr) {
+		h.logger.Debug("Subscription name is ambiguous")
+		c.JSON(http.StatusForbidden, gin.H{
+			"error": gin.H{
+				"message": err.Error(),
+				"type":    "permission_error",
+			}})
+		return
+	}
+
 	if errors.As(err, &accessDeniedErr) {
 		h.logger.Debug("Access denied to subscription")
 		c.JSON(http.StatusForbidden, gin.H{
@@ -255,7 +267,7 @@ func (h *ModelsHandler) ListLLMs(c *gin.Context) {
 			} else {
 				// User has zero accessible subscriptions - return empty list
 				h.logger.Debug("User has zero accessible subscriptions, returning empty model list")
-				// modelList is already initialized to empty slice at line 235
+				// modelList is already initialized to empty slice above
 			}
 		} else {
 			// Filter models by subscription(s) and aggregate subscriptions
@@ -268,8 +280,22 @@ func (h *ModelsHandler) ListLLMs(c *gin.Context) {
 			modelsByKey := make(map[modelKey]*models.Model)
 
 			for _, sub := range subscriptionsToUse {
-				h.logger.Debug("Filtering models by subscription", "subscription", sub.Name, "modelCount", len(list))
-				filteredModels := h.modelMgr.FilterModelsByAccess(c.Request.Context(), list, authHeader, sub.Name)
+				// Pre-filter by modelRefs if available (optimization to reduce HTTP calls)
+				modelsToCheck := list
+				if len(sub.ModelRefs) > 0 {
+					h.logger.Debug("Pre-filtering models by subscription modelRefs",
+						"subscription", sub.Name,
+						"totalModels", len(list),
+						"modelRefsCount", len(sub.ModelRefs),
+					)
+					modelsToCheck = filterModelsBySubscription(list, sub.ModelRefs)
+					h.logger.Debug("After modelRef filtering", "modelsToCheck", len(modelsToCheck))
+				}
+
+				// Use qualified "namespace/name" format for accurate authorization checks
+				qualifiedSubName := sub.Namespace + "/" + sub.Name
+				h.logger.Debug("Filtering models by subscription", "subscription", qualifiedSubName, "modelCount", len(modelsToCheck))
+				filteredModels := h.modelMgr.FilterModelsByAccess(c.Request.Context(), modelsToCheck, authHeader, qualifiedSubName)
 
 				for _, model := range filteredModels {
 					subInfo := models.SubscriptionInfo{
@@ -323,3 +349,29 @@ func (h *ModelsHandler) ListLLMs(c *gin.Context) {
 		Data:   modelList,
 	})
 }
+
+// filterModelsBySubscription filters models to only those matching the subscription's modelRefs.
+func filterModelsBySubscription(modelList []models.Model, modelRefs []subscription.ModelRefInfo) []models.Model {
+	if len(modelRefs) == 0 {
+		return modelList
+	}
+
+	// Build map of allowed models for fast lookup
+	allowed := make(map[string]bool)
+	for _, ref := range modelRefs {
+		key := ref.Namespace + "/" + ref.Name
+		allowed[key] = true
+	}
+
+	// Filter models
+	filtered := make([]models.Model, 0, len(modelList))
+	for _, model := range modelList {
+		// Models from MaaSModelRefLister have OwnedBy set to namespace
+		modelKey := model.OwnedBy + "/" + model.ID
+		if allowed[modelKey] {
+			filtered = append(filtered, model)
+		}
+	}
+
+	return filtered
+}

maas-api/internal/handlers/models_test.go

@@ -402,8 +402,9 @@ func TestListingModelsWithSubscriptionHeader(t *testing.T) {
 	testLogger := logger.Development()
 
 	// Create mock servers that require specific subscription headers
-	premiumModelServer := createMockModelServerWithSubscriptionCheck(t, "premium-model", "premium")
-	freeModelServer := createMockModelServerWithSubscriptionCheck(t, "free-model", "free")
+	// Use qualified names (namespace/name) to match the format sent by the handler
+	premiumModelServer := createMockModelServerWithSubscriptionCheck(t, "premium-model", "test-namespace/premium")
+	freeModelServer := createMockModelServerWithSubscriptionCheck(t, "free-model", "test-namespace/free")
 
 	// Build MaaSModelRef unstructured list
 	maasModelRefItems := []*unstructured.Unstructured{
@@ -573,9 +574,10 @@ func TestListModels_ReturnAllModels(t *testing.T) {
 	testLogger := logger.Development()
 
 	// Create mock servers for models
-	model1Server := createMockModelServerWithSubscriptionCheck(t, "model-1", "sub-a")
-	model2Server := createMockModelServerWithSubscriptionCheck(t, "model-2", "sub-b")
-	model3Server := createMockModelServerWithSubscriptionCheck(t, "model-3", "sub-a")
+	// Use qualified names (namespace/name) to match the format sent by the handler
+	model1Server := createMockModelServerWithSubscriptionCheck(t, "model-1", "test-namespace/sub-a")
+	model2Server := createMockModelServerWithSubscriptionCheck(t, "model-2", "test-namespace/sub-b")
+	model3Server := createMockModelServerWithSubscriptionCheck(t, "model-3", "test-namespace/sub-a")
 
 	// Setup MaaSModelRef lister with three models
 	lister := fakeMaaSModelRefLister{
@@ -1097,8 +1099,9 @@ func TestListModels_DifferentModelRefsWithSameModelIDAndDifferentSubscriptions(t
 
 	// Create two mock servers that both return the same model ID "gpt-4"
 	// One accessible via sub-a, one via sub-b
-	modelServerA := createMockModelServerWithSubscriptionCheck(t, "gpt-4", "sub-a")
-	modelServerB := createMockModelServerWithSubscriptionCheck(t, "gpt-4", "sub-b")
+	// Use qualified names (namespace/name) to match the format sent by the handler
+	modelServerA := createMockModelServerWithSubscriptionCheck(t, "gpt-4", "test-namespace/sub-a")
+	modelServerB := createMockModelServerWithSubscriptionCheck(t, "gpt-4", "test-namespace/sub-b")
 
 	// Setup MaaSModelRef lister with two different MaaSModelRefs in different namespaces
 	lister := fakeMaaSModelRefLister{

maas-api/internal/models/discovery.go

@@ -141,10 +141,8 @@ func discoveredToModels(discovered []openai.Model, original Model) []Model {
 		if d.ID == "" {
 			continue
 		}
-		ownedBy := d.OwnedBy
-		if ownedBy == "" {
-			ownedBy = original.OwnedBy
-		}
+		// Always use the trusted namespace from MaaSModelRef (original.OwnedBy)
+		// Never trust backend-returned OwnedBy to prevent namespace spoofing
 		created := d.Created
 		if created == 0 {
 			created = original.Created
@@ -154,7 +152,7 @@ func discoveredToModels(discovered []openai.Model, original Model) []Model {
 				ID:      d.ID,
 				Object:  "model",
 				Created: created,
-				OwnedBy: ownedBy,
+				OwnedBy: original.OwnedBy,
 			},
 			Kind:    original.Kind,
 			URL:     original.URL,

maas-api/internal/subscription/handler.go

@@ -64,14 +64,17 @@ func (h *Handler) SelectSubscription(c *gin.Context) {
 		"username", req.Username,
 		"groups", req.Groups,
 		"requestedSubscription", req.RequestedSubscription,
+		"requestedModel", req.RequestedModel,
 	)
 
-	response, err := h.selector.Select(req.Groups, req.Username, req.RequestedSubscription)
+	response, err := h.selector.Select(req.Groups, req.Username, req.RequestedSubscription, req.RequestedModel)
 	if err != nil {
 		var noSubErr *NoSubscriptionError
 		var notFoundErr *SubscriptionNotFoundError
 		var accessDeniedErr *AccessDeniedError
 		var multipleSubsErr *MultipleSubscriptionsError
+		var ambiguousErr *SubscriptionAmbiguousError
+		var modelNotInSubErr *ModelNotInSubscriptionError
 
 		if errors.As(err, &noSubErr) {
 			h.logger.Debug("No subscription found for user",
@@ -120,6 +123,29 @@ func (h *Handler) SelectSubscription(c *gin.Context) {
 			return
 		}
 
+		if errors.As(err, &ambiguousErr) {
+			h.logger.Debug("Subscription name is ambiguous",
+				"username", req.Username,
+			)
+			c.JSON(http.StatusOK, SelectResponse{
+				Error:   "ambiguous_subscription",
+				Message: err.Error(),
+			})
+			return
+		}
+
+		if errors.As(err, &modelNotInSubErr) {
+			h.logger.Debug("Model not included in subscription",
+				"subscription", modelNotInSubErr.Subscription,
+				"model", modelNotInSubErr.Model,
+			)
+			c.JSON(http.StatusOK, SelectResponse{
+				Error:   "model_not_in_subscription",
+				Message: err.Error(),
+			})
+			return
+		}
+
 		// All other errors are internal server errors
 		h.logger.Error("Subscription selection failed",
 			"error", err.Error(),