feat(rbac): fine-grained update/delete for application resources

cmd/argocd/commands/admin/settings_rbac.go

@@ -21,6 +21,12 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/rbac"
 )
 
+type actionTraitMap map[string]rbacTrait
+
+type rbacTrait struct {
+	allowPath bool
+}
+
 // Provide a mapping of short-hand resource names to their RBAC counterparts
 var resourceMap map[string]string = map[string]string{
 	"account":         rbacpolicy.ResourceAccounts,
@@ -32,6 +38,7 @@ var resourceMap map[string]string = map[string]string{
 	"certs":           rbacpolicy.ResourceCertificates,
 	"certificate":     rbacpolicy.ResourceCertificates,
 	"cluster":         rbacpolicy.ResourceClusters,
+	"extension":       rbacpolicy.ResourceExtensions,
 	"gpgkey":          rbacpolicy.ResourceGPGKeys,
 	"key":             rbacpolicy.ResourceGPGKeys,
 	"log":             rbacpolicy.ResourceLogs,
@@ -46,28 +53,53 @@ var resourceMap map[string]string = map[string]string{
 }
 
 // List of allowed RBAC resources
-var validRBACResources map[string]bool = map[string]bool{
-	rbacpolicy.ResourceAccounts:        true,
-	rbacpolicy.ResourceApplications:    true,
-	rbacpolicy.ResourceApplicationSets: true,
-	rbacpolicy.ResourceCertificates:    true,
-	rbacpolicy.ResourceClusters:        true,
-	rbacpolicy.ResourceGPGKeys:         true,
-	rbacpolicy.ResourceLogs:            true,
-	rbacpolicy.ResourceExec:            true,
-	rbacpolicy.ResourceProjects:        true,
-	rbacpolicy.ResourceRepositories:    true,
+var validRBACResourcesActions map[string]actionTraitMap = map[string]actionTraitMap{
+	rbacpolicy.ResourceAccounts:        accountsActions,
+	rbacpolicy.ResourceApplications:    applicationsActions,
+	rbacpolicy.ResourceApplicationSets: defaultCRUDActions,
+	rbacpolicy.ResourceCertificates:    defaultCRDActions,
+	rbacpolicy.ResourceClusters:        defaultCRUDActions,
+	rbacpolicy.ResourceExtensions:      extensionActions,
+	rbacpolicy.ResourceGPGKeys:         defaultCRDActions,
+	rbacpolicy.ResourceLogs:            logsActions,
+	rbacpolicy.ResourceExec:            execActions,
+	rbacpolicy.ResourceProjects:        defaultCRUDActions,
+	rbacpolicy.ResourceRepositories:    defaultCRUDActions,
 }
 
 // List of allowed RBAC actions
-var validRBACActions map[string]bool = map[string]bool{
-	rbacpolicy.ActionAction:   true,
-	rbacpolicy.ActionCreate:   true,
-	rbacpolicy.ActionDelete:   true,
-	rbacpolicy.ActionGet:      true,
-	rbacpolicy.ActionOverride: true,
-	rbacpolicy.ActionSync:     true,
-	rbacpolicy.ActionUpdate:   true,
+var defaultCRUDActions = actionTraitMap{
+	rbacpolicy.ActionCreate: rbacTrait{},
+	rbacpolicy.ActionGet:    rbacTrait{},
+	rbacpolicy.ActionUpdate: rbacTrait{},
+	rbacpolicy.ActionDelete: rbacTrait{},
+}
+var defaultCRDActions = actionTraitMap{
+	rbacpolicy.ActionCreate: rbacTrait{},
+	rbacpolicy.ActionGet:    rbacTrait{},
+	rbacpolicy.ActionDelete: rbacTrait{},
+}
+var applicationsActions = actionTraitMap{
+	rbacpolicy.ActionCreate:   rbacTrait{},
+	rbacpolicy.ActionGet:      rbacTrait{},
+	rbacpolicy.ActionUpdate:   rbacTrait{allowPath: true},
+	rbacpolicy.ActionDelete:   rbacTrait{allowPath: true},
+	rbacpolicy.ActionAction:   rbacTrait{allowPath: true},
+	rbacpolicy.ActionOverride: rbacTrait{},
+	rbacpolicy.ActionSync:     rbacTrait{},
+}
+var accountsActions = actionTraitMap{
+	rbacpolicy.ActionCreate: rbacTrait{},
+	rbacpolicy.ActionUpdate: rbacTrait{},
+}
+var execActions = actionTraitMap{
+	rbacpolicy.ActionCreate: rbacTrait{},
+}
+var logsActions = actionTraitMap{
+	rbacpolicy.ActionGet: rbacTrait{},
+}
+var extensionActions = actionTraitMap{
+	rbacpolicy.ActionInvoke: rbacTrait{},
 }
 
 // NewRBACCommand is the command for 'rbac'
@@ -221,8 +253,8 @@ argocd admin settings rbac validate --policy-file policy.csv
 # i.e. 'policy.csv' and (optionally) 'policy.default'
 argocd admin settings rbac validate --policy-file argocd-rbac-cm.yaml
 
-# If --policy-file is not given, and instead --namespace is giventhe ConfigMap 'argocd-rbac-cm' 
-# from K8s is used. 
+# If --policy-file is not given, and instead --namespace is giventhe ConfigMap 'argocd-rbac-cm'
+# from K8s is used.
 argocd admin settings rbac validate --namespace argocd
 
 # Either --policy-file or --namespace must be given.
@@ -376,11 +408,9 @@ func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPoli
 	// If in strict mode, validate that given RBAC resource and action are
 	// actually valid tokens.
 	if strict {
-		if !isValidRBACResource(realResource) {
-			log.Fatalf("error in RBAC request: '%s' is not a valid resource name", realResource)
-		}
-		if !isValidRBACAction(action) {
-			log.Fatalf("error in RBAC request: '%s' is not a valid action name", action)
+		if err := validateRBACResourceAction(realResource, action); err != nil {
+			log.Fatalf("error in RBAC request: %v", err)
+			return false
 		}
 	}
 
@@ -406,17 +436,18 @@ func resolveRBACResourceName(name string) string {
 	}
 }
 
-// isValidRBACAction checks whether a given action is a valid RBAC action
-func isValidRBACAction(action string) bool {
-	if strings.HasPrefix(action, rbacpolicy.ActionAction+"/") {
-		return true
+// validateRBACResourceAction checks whether a given resource is a valid RBAC resource.
+// If it is, it validates that the action is a valid RBAC action for this resource.
+func validateRBACResourceAction(resource, action string) error {
+	validActions, ok := validRBACResourcesActions[resource]
+	if !ok {
+		return fmt.Errorf("'%s' is not a valid resource name", resource)
 	}
-	_, ok := validRBACActions[action]
-	return ok
-}
 
-// isValidRBACResource checks whether a given resource is a valid RBAC resource
-func isValidRBACResource(resource string) bool {
-	_, ok := validRBACResources[resource]
-	return ok
+	realAction, _, hasPath := strings.Cut(action, "/")
+	actionTrait, ok := validActions[realAction]
+	if !ok || hasPath && !actionTrait.allowPath {
+		return fmt.Errorf("'%s' is not a valid action for %s", action, resource)
+	}
+	return nil
 }

cmd/argocd/commands/admin/settings_rbac_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v2/util/assets"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -41,35 +42,75 @@ func (f *FakeClientConfig) ConfigAccess() clientcmd.ConfigAccess {
 	return nil
 }
 
-func Test_isValidRBACAction(t *testing.T) {
-	for k := range validRBACActions {
-		t.Run(k, func(t *testing.T) {
-			ok := isValidRBACAction(k)
-			assert.True(t, ok)
-		})
+func Test_validateRBACResourceAction(t *testing.T) {
+	type args struct {
+		resource string
+		action   string
+	}
+	tests := []struct {
+		name  string
+		args  args
+		valid bool
+	}{
+		{
+			name: "Test valid resource and action",
+			args: args{
+				resource: rbacpolicy.ResourceApplications,
+				action:   rbacpolicy.ActionCreate,
+			},
+			valid: true,
+		},
+		{
+			name: "Test invalid resource",
+			args: args{
+				resource: "invalid",
+			},
+			valid: false,
+		},
+		{
+			name: "Test invalid action",
+			args: args{
+				resource: rbacpolicy.ResourceApplications,
+				action:   "invalid",
+			},
+			valid: false,
+		},
+		{
+			name: "Test invalid action for resource",
+			args: args{
+				resource: rbacpolicy.ResourceLogs,
+				action:   rbacpolicy.ActionCreate,
+			},
+			valid: false,
+		},
+		{
+			name: "Test valid action with path",
+			args: args{
+				resource: rbacpolicy.ResourceApplications,
+				action:   rbacpolicy.ActionAction + "/apps/Deployment/restart",
+			},
+			valid: true,
+		},
+		{
+			name: "Test invalid action with path",
+			args: args{
+				resource: rbacpolicy.ResourceApplications,
+				action:   rbacpolicy.ActionGet + "/apps/Deployment/restart",
+			},
+			valid: false,
+		},
 	}
-	t.Run("invalid", func(t *testing.T) {
-		ok := isValidRBACAction("invalid")
-		assert.False(t, ok)
-	})
-}
-
-func Test_isValidRBACAction_ActionAction(t *testing.T) {
-	ok := isValidRBACAction("action/apps/Deployment/restart")
-	assert.True(t, ok)
-}
 
-func Test_isValidRBACResource(t *testing.T) {
-	for k := range validRBACResources {
-		t.Run(k, func(t *testing.T) {
-			ok := isValidRBACResource(k)
-			assert.True(t, ok)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := validateRBACResourceAction(tt.args.resource, tt.args.action)
+			if tt.valid {
+				assert.NoError(t, result)
+			} else {
+				assert.NotNil(t, result)
+			}
 		})
 	}
-	t.Run("invalid", func(t *testing.T) {
-		ok := isValidRBACResource("invalid")
-		assert.False(t, ok)
-	})
 }
 
 func Test_PolicyFromCSV(t *testing.T) {