fix(rbac): resolve RBAC regression for project-scoped resources in multi-namespace architecture (#25289) (#26573)

Signed-off-by: tcfwbper <pesci861207@gmail.com>
Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

docs/operator-manual/app-any-namespace.md

@@ -45,8 +45,8 @@ In order to enable this feature, the Argo CD administrator must reconfigure the
 The `--application-namespaces` parameter takes a comma-separated list of namespaces where `Applications` are to be allowed in. Each entry of the list supports:
 
 - shell-style wildcards such as `*`, so for example the entry `app-team-*` would match `app-team-one` and `app-team-two`. To enable all namespaces on the cluster where Argo CD is running on, you can just specify `*`, i.e. `--application-namespaces=*`.
-- regex, requires wrapping the string in ```/```, example to allow all namespaces except a particular one: ```/^((?!not-allowed).)*$/```.
-  
+- regex, requires wrapping the string in `/`, example to allow all namespaces except a particular one: `/^((?!not-allowed).)*$/`.
+
 The startup parameters for both, the `argocd-server` and the `argocd-application-controller` can also be conveniently set up and kept in sync by specifying the `application.namespaces` settings in the `argocd-cmd-params-cm` ConfigMap _instead_ of changing the manifests for the respective workloads. For example:
 
 ```yaml
@@ -94,7 +94,7 @@ metadata:
   namespace: argocd
 spec:
   sourceNamespaces:
-  - namespace-one
+    - namespace-one
 ```
 
 and
@@ -107,7 +107,7 @@ metadata:
   namespace: argocd
 spec:
   sourceNamespaces:
-  - namespace-two
+    - namespace-two
 ```
 
 In order for an Application to set `.spec.project` to `project-one`, it would have to be created in either namespace `namespace-one` or `argocd`. Likewise, in order for an Application to set `.spec.project` to `project-two`, it would have to be created in either namespace `namespace-two` or `argocd`.
@@ -138,7 +138,11 @@ For backwards compatibility, if the namespace of the Application is the control
 
 The RBAC syntax for Application objects has been changed from `<project>/<application>` to `<project>/<namespace>/<application>` to accommodate the need to restrict access based on the source namespace of the Application to be managed.
 
-For backwards compatibility, Applications in the `argocd` namespace can still be referred to as `<project>/<application>` in the RBAC policy rules.
+For backwards compatibility, Applications in the `argocd` namespace will still be referred to as `<project>/<application>` in the RBAC policy rules.
+
+!!! note
+
+    Due to backward compatibility, it is not possible to define RBAC policies specifically for applications in the Argo CD control plane namespace (typically `argocd`) using the pattern `foo/argocd/*`. Applications in the control plane namespace are always normalized to the 2-segment format `<project>/<application>` in RBAC enforcement. For security reasons, an AppProject should never grant access to the control plane namespace through the `.spec.sourceNamespaces` field, as this would allow users to create applications with elevated privileges.
 
 Wildcards do not make any distinction between project and application namespaces yet. For example, the following RBAC rule would match any application belonging to project `foo`, regardless of the namespace it is created in:
 
@@ -151,7 +155,7 @@ If you want to restrict access to be granted only to `Applications` in project `
 ```
 p, somerole, applications, get, foo/bar/*, allow
 ```
-  
+
 ## Managing applications in other namespaces
 
 ### Declaratively
@@ -175,10 +179,10 @@ The project `some-project` will then need to specify `some-namespace` in the lis
 kind: AppProject
 apiVersion: argoproj.io/v1alpha1
 metadata:
-    name: some-project
-    namespace: argocd
+  name: some-project
+  namespace: argocd
 spec:
-    sourceNamespaces:
+  sourceNamespaces:
     - some-namespace
 ```
 

docs/operator-manual/applicationset/Appset-Any-Namespace.md

@@ -205,7 +205,11 @@ For backwards compatibility, if the namespace of the ApplicationSet is the contr
 
 The RBAC syntax for Application objects has been changed from `<project>/<applicationset>` to `<project>/<namespace>/<applicationset>` to accommodate the need to restrict access based on the source namespace of the Application to be managed.
 
-For backwards compatibility, Applications in the argocd namespace can still be referred to as `<project>/<applicationset>` in the RBAC policy rules.
+For backwards compatibility, Applications in the `argocd` namespace will still be referred to as `<project>/<applicationset>` in the RBAC policy rules.
+
+!!! note
+
+    Due to backward compatibility, it is not possible to define RBAC policies specifically for applications in the Argo CD control plane namespace (typically `argocd`) using the pattern `foo/argocd/*`. Applications in the control plane namespace are always normalized to the 2-segment format `<project>/<application>` in RBAC enforcement. For security reasons, an AppProject should never grant access to the control plane namespace through the `.spec.sourceNamespaces` field, as this would allow users to create applications with elevated privileges.
 
 Wildcards do not make any distinction between project and applicationset namespaces yet. For example, the following RBAC rule would match any application belonging to project foo, regardless of the namespace it is created in:
 

server/account/account.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"regexp"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/google/uuid"
@@ -19,6 +20,7 @@ import (
 	"github.com/argoproj/argo-cd/v3/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v3/util/password"
 	"github.com/argoproj/argo-cd/v3/util/rbac"
+	"github.com/argoproj/argo-cd/v3/util/security"
 	"github.com/argoproj/argo-cd/v3/util/session"
 	"github.com/argoproj/argo-cd/v3/util/settings"
 )
@@ -28,11 +30,12 @@ type Server struct {
 	sessionMgr  *session.SessionManager
 	settingsMgr *settings.SettingsManager
 	enf         *rbac.Enforcer
+	namespace   string
 }
 
 // NewServer returns a new instance of the Session service
-func NewServer(sessionMgr *session.SessionManager, settingsMgr *settings.SettingsManager, enf *rbac.Enforcer) *Server {
-	return &Server{sessionMgr, settingsMgr, enf}
+func NewServer(sessionMgr *session.SessionManager, settingsMgr *settings.SettingsManager, enf *rbac.Enforcer, namespace string) *Server {
+	return &Server{sessionMgr, settingsMgr, enf, namespace}
 }
 
 // UpdatePassword updates the password of the currently authenticated account or the account specified in the request.
@@ -126,7 +129,22 @@ func (s *Server) CanI(ctx context.Context, r *account.CanIRequest) (*account.Can
 		return nil, status.Errorf(codes.InvalidArgument, "%v does not contain %s", rbac.Resources, r.Resource)
 	}
 
-	ok := s.enf.Enforce(ctx.Value("claims"), r.Resource, r.Action, r.Subresource)
+	subresource := r.Subresource
+
+	// For project-scoped resources, normalize the subresource using security.RBACName
+	// This converts "project/defaultNS/name" to "project/name" for backward compatibility
+	if rbac.ProjectScoped[r.Resource] && s.namespace != "" && subresource != "" {
+		parts := strings.Split(subresource, "/")
+		if len(parts) == 3 {
+			// 3-part format: project/namespace/name
+			// Normalize: if namespace == defaultNS, becomes project/name; otherwise stays project/namespace/name
+			subresource = security.RBACName(s.namespace, parts[0], parts[1], parts[2])
+		}
+		// if 2 parts, always assume the default namespace
+		// else: keep as-is (wildcards, etc.)
+	}
+
+	ok := s.enf.Enforce(ctx.Value("claims"), r.Resource, r.Action, subresource)
 	if ok {
 		return &account.CanIResponse{Value: "yes"}, nil
 	}

server/account/account_test.go

@@ -70,7 +70,7 @@ func newTestAccountServerExt(t *testing.T, ctx context.Context, enforceFn rbac.C
 	enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
 	enforcer.SetClaimsEnforcerFunc(enforceFn)
 
-	return NewServer(sessionMgr, settingsMgr, enforcer), session.NewServer(sessionMgr, settingsMgr, nil, nil, nil)
+	return NewServer(sessionMgr, settingsMgr, enforcer, testNamespace), session.NewServer(sessionMgr, settingsMgr, nil, nil, nil)
 }
 
 func getAdminAccount(mgr *settings.SettingsManager) (*settings.Account, error) {
@@ -332,3 +332,137 @@ func TestCanI_GetLogsDeny(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "no", resp.Value)
 }
+
+func TestCanI_RBACPolicyMatchingWithNormalizedSubresource(t *testing.T) {
+	tests := []struct {
+		name         string
+		policy       string
+		expectedResp string
+	}{
+		{
+			name:         "allow policy without namespace",
+			policy:       "p, role:log-viewer, logs, get, myproject/*, allow",
+			expectedResp: "yes",
+		},
+		{
+			name:         "deny explicit default namespace policy",
+			policy:       "p, role:log-viewer, logs, get, myproject/default/*, allow",
+			expectedResp: "no",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			accountServer, _ := newTestAccountServerExt(t, t.Context(), nil)
+			require.NoError(t, accountServer.enf.SetBuiltinPolicy(tt.policy))
+			accountServer.enf.SetDefaultRole("role:log-viewer")
+
+			resp, err := accountServer.CanI(adminContext(t.Context()), &account.CanIRequest{
+				Resource:    "logs",
+				Action:      "get",
+				Subresource: "myproject/default/myapp",
+			})
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedResp, resp.Value)
+		})
+	}
+}
+
+func TestCanI_NormalizeDefaultNamespace(t *testing.T) {
+	// Test: subresource "myproject/default/myapp" with default namespace "default"
+	// Expected: normalized to "myproject/myapp" (matches */* policy)
+	enforcer := func(_ jwt.Claims, rvals ...any) bool {
+		// Verify the subresource was normalized to 2 segments
+		if len(rvals) >= 4 {
+			if obj, ok := rvals[3].(string); ok {
+				return obj == "myproject/myapp"
+			}
+		}
+		return false
+	}
+
+	accountServer, _ := newTestAccountServerExt(t, t.Context(), enforcer)
+	ctx := adminContext(t.Context())
+
+	// UI sends 3-segment format with default namespace
+	resp, err := accountServer.CanI(ctx, &account.CanIRequest{
+		Resource:    "logs",
+		Action:      "get",
+		Subresource: "myproject/default/myapp", // default is default namespace
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "yes", resp.Value)
+}
+
+func TestCanI_PreserveNonDefaultNamespace(t *testing.T) {
+	// Test: subresource "myproject/other-ns/myapp" with default namespace "default"
+	// Expected: preserved as "myproject/other-ns/myapp" (needs */*/* policy)
+	enforcer := func(_ jwt.Claims, rvals ...any) bool {
+		// Verify the subresource was NOT normalized (3 segments)
+		if len(rvals) >= 4 {
+			if obj, ok := rvals[3].(string); ok {
+				return obj == "myproject/other-ns/myapp"
+			}
+		}
+		return false
+	}
+
+	accountServer, _ := newTestAccountServerExt(t, t.Context(), enforcer)
+	ctx := adminContext(t.Context())
+
+	resp, err := accountServer.CanI(ctx, &account.CanIRequest{
+		Resource:    "logs",
+		Action:      "get",
+		Subresource: "myproject/other-ns/myapp", // other-ns != default
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "yes", resp.Value)
+}
+
+func TestCanI_BackwardCompatibleTwoSegment(t *testing.T) {
+	// Test: old UI sends "myproject/myapp" (2 segments)
+	// Expected: stays as "myproject/myapp"
+	enforcer := func(_ jwt.Claims, rvals ...any) bool {
+		if len(rvals) >= 4 {
+			if obj, ok := rvals[3].(string); ok {
+				return obj == "myproject/myapp"
+			}
+		}
+		return false
+	}
+
+	accountServer, _ := newTestAccountServerExt(t, t.Context(), enforcer)
+	ctx := adminContext(t.Context())
+
+	resp, err := accountServer.CanI(ctx, &account.CanIRequest{
+		Resource:    "logs",
+		Action:      "get",
+		Subresource: "myproject/myapp",
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "yes", resp.Value)
+}
+
+func TestCanI_NonProjectScopedResource(t *testing.T) {
+	// Test: non-project-scoped resources should not be normalized
+	enforcer := func(_ jwt.Claims, rvals ...any) bool {
+		if len(rvals) >= 4 {
+			if obj, ok := rvals[3].(string); ok {
+				// Should receive the original format unchanged
+				return obj == "some/value/here"
+			}
+		}
+		return false
+	}
+
+	accountServer, _ := newTestAccountServerExt(t, t.Context(), enforcer)
+	ctx := adminContext(t.Context())
+
+	resp, err := accountServer.CanI(ctx, &account.CanIRequest{
+		Resource:    "accounts", // not project-scoped
+		Action:      "update",
+		Subresource: "some/value/here",
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "yes", resp.Value)
+}

server/server.go

@@ -1067,7 +1067,7 @@ func newArgoCDServiceSet(a *ArgoCDServer) *ArgoCDServiceSet {
 	projectService := project.NewServer(a.Namespace, a.KubeClientset, a.AppClientset, a.enf, projectLock, a.sessionMgr, a.policyEnforcer, a.projInformer, a.settingsMgr, a.db, a.EnableK8sEvent)
 	appsInAnyNamespaceEnabled := len(a.ApplicationNamespaces) > 0
 	settingsService := settings.NewServer(a.settingsMgr, a.RepoClientset, a, a.DisableAuth, appsInAnyNamespaceEnabled, a.HydratorEnabled, a.SyncWithReplaceAllowed)
-	accountService := account.NewServer(a.sessionMgr, a.settingsMgr, a.enf)
+	accountService := account.NewServer(a.sessionMgr, a.settingsMgr, a.enf, a.Namespace)
 
 	notificationService := notification.NewServer(a.apiFactory)
 	certificateService := certificate.NewServer(a.db, a.enf)

ui/src/app/applications/components/resource-details/resource-details.tsx

@@ -280,8 +280,8 @@ export const ResourceDetails = (props: ResourceDetailsProps) => {
 
                         const settings = await services.authService.settings();
                         const execEnabled = settings.execEnabled;
-                        const logsAllowed = await services.accounts.canI('logs', 'get', application.spec.project + '/' + application.metadata.name);
-                        const execAllowed = execEnabled && (await services.accounts.canI('exec', 'create', application.spec.project + '/' + application.metadata.name));
+                        const logsAllowed = await services.accounts.canI('logs', 'get', AppUtils.appRBACName(application));
+                        const execAllowed = execEnabled && (await services.accounts.canI('exec', 'create', AppUtils.appRBACName(application)));
                         const links = await services.applications.getResourceLinks(application.metadata.name, application.metadata.namespace, selectedNode).catch(() => null);
                         const resourceActionsMenuItems = await AppUtils.getResourceActionsMenuItems(selectedNode, application.metadata, appContext);
                         return {controlledState, liveState, events, podState, execEnabled, execAllowed, logsAllowed, links, childResources, resourceActionsMenuItems};

ui/src/app/applications/components/utils.test.tsx

@@ -12,6 +12,7 @@ import {
 } from '../../shared/models';
 import * as jsYaml from 'js-yaml';
 import {
+    appRBACName,
     ComparisonStatusIcon,
     getAppOperationState,
     getOperationType,
@@ -892,4 +893,54 @@ status:
 
         expect(reason).toBe('SchedulingGated');
     });
+});
+
+describe('appRBACName', () => {
+    it('returns project/namespace/name when namespace is defined', () => {
+        const app = {
+            metadata: {
+                name: 'my-app',
+                namespace: 'my-namespace'
+            },
+            spec: {
+                project: 'my-project'
+            }
+        } as Application;
+
+        const result = appRBACName(app);
+
+        expect(result).toBe('my-project/my-namespace/my-app');
+    });
+
+    it('returns project/name when namespace is undefined', () => {
+        const app = {
+            metadata: {
+                name: 'my-app'
+            },
+            spec: {
+                project: 'my-project'
+            }
+        } as Application;
+
+        const result = appRBACName(app);
+
+        expect(result).toBe('my-project/my-app');
+    });
+
+    it('handles empty namespace string as undefined', () => {
+        const app = {
+            metadata: {
+                name: 'test-app',
+                namespace: ''
+            },
+            spec: {
+                project: 'test-project'
+            }
+        } as Application;
+
+        // Note: The function uses a falsy check on namespace, so empty string is treated the same as undefined
+        const result = appRBACName(app);
+
+        expect(result).toBe('test-project/test-app');
+    });
 });

ui/src/app/applications/components/utils.tsx

@@ -749,7 +749,7 @@ function getActionItems(
 
     const logsAction = isApp(application)
         ? services.accounts
-              .canI('logs', 'get', application.spec.project + '/' + application.metadata.name)
+              .canI('logs', 'get', appRBACName(application))
               .then(async allowed => {
                   if (allowed && (isPod || findChildPod(resource, tree as appModels.ApplicationTree))) {
                       return [
@@ -776,7 +776,7 @@ function getActionItems(
         ? services.authService
               .settings()
               .then(async settings => {
-                  const execAllowed = settings.execEnabled && (await services.accounts.canI('exec', 'create', application.spec.project + '/' + application.metadata.name));
+                  const execAllowed = settings.execEnabled && (await services.accounts.canI('exec', 'create', appRBACName(application)));
                   if (isPod && execAllowed) {
                       return [
                           {
@@ -1829,6 +1829,22 @@ export function appQualifiedName(app: appModels.AbstractApplication, nsEnabled:
     return (nsEnabled ? app.metadata.namespace + '/' : '') + app.metadata.name;
 }
 
+/**
+ * Constructs the RBAC subresource name for canI() checks.
+ **/
+export function appRBACName(app: appModels.Application): string {
+    const project = app.spec.project;
+    const namespace = app.metadata.namespace;
+    const name = app.metadata.name;
+
+    // Always include namespace if available - server will normalize
+    if (namespace) {
+        return `${project}/${namespace}/${name}`;
+    }
+    // Fallback to 2-segment format if namespace is missing
+    return `${project}/${name}`;
+}
+
 export function appInstanceName(app: appModels.AbstractApplication): string {
     return app.metadata.namespace + '_' + app.metadata.name;
 }