From 64a3edff01533a0483db31686a6385dffa81ee55 Mon Sep 17 00:00:00 2001
From: IvanHunters <xorokhotnikov@gmail.com>
Date: Thu, 23 Apr 2026 18:13:10 +0300
Subject: [PATCH] fix(api): prevent IDOR in TenantNamespace Get and Watch
 handlers

Fixed two IDOR vulnerabilities allowing authenticated users to access
metadata of any tenant namespace without proper authorization.

Changes:
- Added hasAccessToNamespace() for efficient single-namespace access checks
- Get() now verifies access before returning namespace metadata
- Watch() filters events per-namespace with proper authorization
- Returns NotFound (not Forbidden) to prevent tenant enumeration

Performance optimization:
- hasAccessToNamespace() lists RoleBindings only in target namespace
  instead of listing all cluster RoleBindings (order of magnitude faster)
- Watch handler logs authorization errors for security audit

Additional fixes:
- Handle ServiceAccount subjects with empty namespace correctly
- Add klog error logging for failed authorization checks

Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
---
 pkg/registry/core/tenantnamespace/rest.go | 89 ++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/pkg/registry/core/tenantnamespace/rest.go b/pkg/registry/core/tenantnamespace/rest.go
index f1ed3fab..7724c0e1 100644
--- a/pkg/registry/core/tenantnamespace/rest.go
+++ b/pkg/registry/core/tenantnamespace/rest.go
@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	corev1alpha1 "github.com/cozystack/cozystack/pkg/apis/core/v1alpha1"
@@ -123,8 +124,18 @@ func (r *REST) Get(
 		return nil, apierrors.NewNotFound(r.gvr.GroupResource(), name)
 	}
 
+	// Check if user has access to this namespace
+	hasAccess, err := r.hasAccessToNamespace(ctx, name)
+	if err != nil {
+		return nil, err
+	}
+	if !hasAccess {
+		// Return NotFound instead of Forbidden to prevent enumeration
+		return nil, apierrors.NewNotFound(r.gvr.GroupResource(), name)
+	}
+
 	ns := &corev1.Namespace{}
-	err := r.c.Get(ctx, types.NamespacedName{Namespace: "", Name: name}, ns, &client.GetOptions{Raw: opts})
+	err = r.c.Get(ctx, types.NamespacedName{Namespace: "", Name: name}, ns, &client.GetOptions{Raw: opts})
 	if err != nil {
 		return nil, err
 	}
@@ -189,6 +200,17 @@ func (r *REST) Watch(ctx context.Context, opts *metainternal.ListOptions) (watch
 				continue
 			}
 
+			// Check if user has access to this namespace
+			hasAccess, err := r.hasAccessToNamespace(ctx, ns.Name)
+			if err != nil {
+				klog.Errorf("Failed to check access for namespace %s in watch: %v", ns.Name, err)
+				continue
+			}
+			if !hasAccess {
+				// User doesn't have access, skip this event
+				continue
+			}
+
 			out := &corev1alpha1.TenantNamespace{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: corev1alpha1.SchemeGroupVersion.String(),
@@ -359,7 +381,11 @@ func (r *REST) filterAccessible(
 					break subjectLoop
 				}
 			case "ServiceAccount":
-				if u.GetName() == fmt.Sprintf("system:serviceaccount:%s:%s", subj.Namespace, subj.Name) {
+				saNamespace := subj.Namespace
+				if saNamespace == "" {
+					saNamespace = rbs.Items[i].Namespace
+				}
+				if u.GetName() == fmt.Sprintf("system:serviceaccount:%s:%s", saNamespace, subj.Name) {
 					allowedNameSet[rbs.Items[i].Namespace] = struct{}{}
 					break subjectLoop
 				}
@@ -373,6 +399,65 @@ func (r *REST) filterAccessible(
 	return allowed, nil
 }
 
+// hasAccessToNamespace checks if the user has access to a single namespace.
+// This is optimized for Get/Watch operations where we check one namespace at a time.
+// It lists RoleBindings only in the target namespace instead of all cluster RoleBindings.
+func (r *REST) hasAccessToNamespace(
+	ctx context.Context,
+	namespace string,
+) (bool, error) {
+	u, ok := request.UserFrom(ctx)
+	if !ok {
+		return false, fmt.Errorf("user missing in context")
+	}
+
+	// Check privileged groups
+	groups := make(map[string]struct{})
+	for _, group := range u.GetGroups() {
+		groups[group] = struct{}{}
+	}
+	if _, ok := groups["system:masters"]; ok {
+		return true, nil
+	}
+	if _, ok := groups["cozystack-cluster-admin"]; ok {
+		return true, nil
+	}
+
+	// List RoleBindings only in the target namespace
+	rbs := &rbacv1.RoleBindingList{}
+	err := r.c.List(ctx, rbs, client.InNamespace(namespace))
+	if err != nil {
+		return false, fmt.Errorf("failed to list rolebindings in %s: %w", namespace, err)
+	}
+
+	// Check if user is in any RoleBinding subjects
+	for i := range rbs.Items {
+		for j := range rbs.Items[i].Subjects {
+			subj := rbs.Items[i].Subjects[j]
+			switch subj.Kind {
+			case "Group":
+				if _, ok := groups[subj.Name]; ok {
+					return true, nil
+				}
+			case "User":
+				if subj.Name == u.GetName() {
+					return true, nil
+				}
+			case "ServiceAccount":
+				saNamespace := subj.Namespace
+				if saNamespace == "" {
+					saNamespace = rbs.Items[i].Namespace
+				}
+				if u.GetName() == fmt.Sprintf("system:serviceaccount:%s:%s", saNamespace, subj.Name) {
+					return true, nil
+				}
+			}
+		}
+	}
+
+	return false, nil
+}
+
 // -----------------------------------------------------------------------------
 // Boiler-plate
 // -----------------------------------------------------------------------------