fix(cli): admin settings rbac can has inconsistency among project r…

…esources (#17805) * fix admin can inconsistency among resources Signed-off-by: xiaopeng <[email protected]> * revise logic and fix lint Signed-off-by: xiaopeng <[email protected]> --------- Signed-off-by: xiaopeng <[email protected]>
argoproj · Sep 5, 2024 · aa2bafd · aa2bafd
1 parent d3fbeec
commit aa2bafd
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 5 deletions.
diff --git 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
@@ -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)