Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 68 additions & 37 deletions cmd/argocd/commands/admin/settings_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ import (
"github.com/argoproj/argo-cd/v2/util/rbac"
)

type actionTraitMap map[string]rbacTrait

type rbacTrait struct {
agaudreault marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand All @@ -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,
Expand All @@ -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'
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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
}
91 changes: 66 additions & 25 deletions cmd/argocd/commands/admin/settings_rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading