Skip to content

Commit

Permalink
feat(rbac): fine-grained update/delete for application resources (arg…
Browse files Browse the repository at this point in the history 
…oproj#18124)

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

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* rewrite rbac (draft)

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* add other stuff

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* spellcheck

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* update map

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* spell check

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* linter not happy about deprecated claims

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* not happy about claims at all

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* generated

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* fix list syntax

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* use same link pattern

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* rewrite permissions to policy when it applies

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

* Update docs/operator-manual/rbac.md

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>

---------

Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
Signed-off-by: Javier Solana <javier.solana@cabify.com>
Signed-off-by: Javier Solana <javier.solana@cabify.com>
  • Loading branch information
@agaudreault
agaudreault authored and Javier Solana committed Jul 24, 2024
1 parent fddf00c commit fa9a53f
Show file tree
Hide file tree
Showing 7 changed files with 525 additions and 290 deletions.
105 changes: 68 additions & 37 deletions cmd/argocd/commands/admin/settings_rbac.go
View file
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 {
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
View file
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

0 comments on commit fa9a53f

Please sign in to comment.