From aa2bafd8128122b2c81b01c910761e51a6466a20 Mon Sep 17 00:00:00 2001 From: Xiaopeng Han Date: Thu, 5 Sep 2024 12:37:13 +0800 Subject: [PATCH] fix(cli): `admin settings rbac can` has inconsistency among project resources (#17805) * fix admin can inconsistency among resources Signed-off-by: xiaopeng * revise logic and fix lint Signed-off-by: xiaopeng --------- Signed-off-by: xiaopeng --- cmd/argocd/commands/admin/settings_rbac.go | 20 ++++++++++++++----- .../commands/admin/settings_rbac_test.go | 8 ++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/cmd/argocd/commands/admin/settings_rbac.go b/cmd/argocd/commands/admin/settings_rbac.go index 3acbf7952e62b..dc8faf657b520 100644 --- a/cmd/argocd/commands/admin/settings_rbac.go +++ b/cmd/argocd/commands/admin/settings_rbac.go @@ -29,7 +29,7 @@ type rbacTrait struct { } // Provide a mapping of short-hand resource names to their RBAC counterparts -var resourceMap map[string]string = map[string]string{ +var resourceMap = map[string]string{ "account": rbacpolicy.ResourceAccounts, "app": rbacpolicy.ResourceApplications, "apps": rbacpolicy.ResourceApplications, @@ -53,8 +53,17 @@ var resourceMap map[string]string = map[string]string{ "repository": rbacpolicy.ResourceRepositories, } +var projectScoped = map[string]bool{ + rbacpolicy.ResourceApplications: true, + rbacpolicy.ResourceApplicationSets: true, + rbacpolicy.ResourceLogs: true, + rbacpolicy.ResourceExec: true, + rbacpolicy.ResourceClusters: true, + rbacpolicy.ResourceRepositories: true, +} + // List of allowed RBAC resources -var validRBACResourcesActions map[string]actionTraitMap = map[string]actionTraitMap{ +var validRBACResourcesActions = map[string]actionTraitMap{ rbacpolicy.ResourceAccounts: accountsActions, rbacpolicy.ResourceApplications: applicationsActions, rbacpolicy.ResourceApplicationSets: defaultCRUDActions, @@ -436,14 +445,15 @@ func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPoli } } - // Application resources have a special notation - for simplicity's sake, + // Some project scoped resources have a special notation - for simplicity's sake, // if user gives no sub-resource (or specifies simple '*'), we construct // the required notation by setting subresource to '*/*'. - if realResource == rbacpolicy.ResourceApplications { + if projectScoped[realResource] { if subResource == "*" || subResource == "" { subResource = "*/*" } - } else if realResource == rbacpolicy.ResourceLogs { + } + if realResource == rbacpolicy.ResourceLogs { if isLogRbacEnforced != nil && !isLogRbacEnforced() { return true } diff --git a/cmd/argocd/commands/admin/settings_rbac_test.go b/cmd/argocd/commands/admin/settings_rbac_test.go index c5b131c593ef0..9fe9ab6953a68 100644 --- a/cmd/argocd/commands/admin/settings_rbac_test.go +++ b/cmd/argocd/commands/admin/settings_rbac_test.go @@ -235,6 +235,14 @@ func Test_PolicyFromK8s(t *testing.T) { ok := checkPolicy("log-allow-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, falseLogRbacEnforce) require.True(t, ok) }) + t.Run("get logs", func(t *testing.T) { + ok := checkPolicy("role:test", "get", "logs", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil) + require.True(t, ok) + }) + t.Run("get logs", func(t *testing.T) { + ok := checkPolicy("role:test", "get", "logs", "", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil) + require.True(t, ok) + }) t.Run("create exec", func(t *testing.T) { ok := checkPolicy("role:test", "create", "exec", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil) require.True(t, ok)