diff --git a/docs/operator-manual/app-any-namespace.md b/docs/operator-manual/app-any-namespace.md index 425bf28b5e..95b172eba1 100644 --- a/docs/operator-manual/app-any-namespace.md +++ b/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 `/` to `//` 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 `/` in the RBAC policy rules. +For backwards compatibility, Applications in the `argocd` namespace will still be referred to as `/` 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 `/` 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 ``` diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index d8ac8d4044..6998f5c235 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/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 `/` to `//` 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 `/` in the RBAC policy rules. +For backwards compatibility, Applications in the `argocd` namespace will still be referred to as `/` 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 `/` 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: diff --git a/server/account/account.go b/server/account/account.go index 94e677ed3f..62b053e58a 100644 --- a/server/account/account.go +++ b/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 } diff --git a/server/account/account_test.go b/server/account/account_test.go index 5ecd8e9845..c4808e5761 100644 --- a/server/account/account_test.go +++ b/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) +} diff --git a/server/server.go b/server/server.go index 20c7ad3b08..6b9006f4ca 100644 --- a/server/server.go +++ b/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) diff --git a/ui/src/app/applications/components/resource-details/resource-details.tsx b/ui/src/app/applications/components/resource-details/resource-details.tsx index 837738b628..20a625d888 100644 --- a/ui/src/app/applications/components/resource-details/resource-details.tsx +++ b/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}; diff --git a/ui/src/app/applications/components/utils.test.tsx b/ui/src/app/applications/components/utils.test.tsx index feba54b33d..def5ec8b6f 100644 --- a/ui/src/app/applications/components/utils.test.tsx +++ b/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'); + }); }); \ No newline at end of file diff --git a/ui/src/app/applications/components/utils.tsx b/ui/src/app/applications/components/utils.tsx index 2e55b10a7f..dd4ee9ef39 100644 --- a/ui/src/app/applications/components/utils.tsx +++ b/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; }