Skip to content

Commit

Permalink
Enable fine-grained update/delete RBAC enforcement by default
Browse files Browse the repository at this point in the history
Change applications resource RBAC to use fine-grained update/delete
enforcement by default. This allows us to enforce RBAC on the
application itself, separately from the sub-resources related to it.

See also GitHub argoproj#18124, argoproj#20600
  • Loading branch information
fffinkel committed Nov 5, 2024
1 parent 8a4e7e0 commit a931647
Show file tree
Hide file tree
Showing 36 changed files with 418 additions and 18 deletions.
4 changes: 3 additions & 1 deletion assets/builtin-policy.csv
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ p, role:readonly, logs, get, */*, allow

p, role:admin, applications, create, */*, allow
p, role:admin, applications, update, */*, allow
p, role:admin, applications, update/*, */*, allow
p, role:admin, applications, delete, */*, allow
p, role:admin, applications, delete/*, */*, allow
p, role:admin, applications, sync, */*, allow
p, role:admin, applications, override, */*, allow
p, role:admin, applications, action/*, */*, allow
Expand All @@ -43,4 +45,4 @@ p, role:admin, gpgkeys, delete, *, allow
p, role:admin, exec, create, */*, allow

g, role:admin, role:readonly
g, admin, role:admin
g, admin, role:admin
15 changes: 6 additions & 9 deletions docs/operator-manual/rbac.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ The `applications` resource is an [Application-Specific Policy](#application-spe

#### Fine-grained Permissions for `update`/`delete` action

The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself **and** all of its resources.
The `update` and `delete` actions, when granted on an application, will allow the user to perform the operation on the application itself but not its resources.
It can be desirable to only allow `update` or `delete` on specific resources within an application.

To do so, when the action if performed on an application's resource, the `<action>` will have the `<action>/<group>/<kind>/<ns>/<name>` format.
Expand All @@ -138,15 +138,12 @@ p, example-user, applications, delete, default/prod-app, deny
p, example-user, applications, delete/*/Pod/*, default/prod-app, allow
```

!!! note

It is not possible to deny fine-grained permissions for a sub-resource if the action was **explicitly allowed on the application**.
For instance, the following policies will **allow** a user to delete the Pod and any other resources in the application:
If we want to explicitly allow updates to the application, but deny updates to any sub-resources:

```csv
p, example-user, applications, delete, default/prod-app, allow
p, example-user, applications, delete/*/Pod/*, default/prod-app, deny
```
```csv
p, example-user, applications, update, default/prod-app, allow
p, example-user, applications, update/*, default/prod-app, deny
```

#### The `action` action

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
aloi
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
repo: https://somewhere
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
invalid:
- I don't know
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
helm:
parameters:
- name: image.tag
value: '0.2'
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: my-chart
version: 1.1.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:{{.Values.image.tag}}
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
image:
tag: 0.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
aloi
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
repo: https://somewhere
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
invalid:
- I don't know
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
helm:
parameters:
- name: image.tag
value: '0.2'
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name: my-chart
version: 1.1.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:{{.Values.image.tag}}
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
image:
tag: 0.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kustomize:
images:
- gcr.io/heptio-images/ks-guestbook-demo:0.2
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: guestbook-ui
spec:
selector:
matchLabels:
app: guestbook-ui
template:
metadata:
labels:
app: guestbook-ui
spec:
containers:
- image: gcr.io/heptio-images/ks-guestbook-demo:0.1
name: guestbook-ui
ports:
- containerPort: 81
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- guestbook.yaml
images:
- name: gcr.io/heptio-images/ks-guestbook-demo
newTag: "0.1"
6 changes: 2 additions & 4 deletions server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -1333,12 +1333,10 @@ func (s *Server) getAppResources(ctx context.Context, a *appv1.Application) (*ap
}

func (s *Server) getAppLiveResource(ctx context.Context, action string, q *application.ApplicationResourceRequest) (*appv1.ResourceNode, *rest.Config, *appv1.Application, error) {
a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName())
if err != nil && errors.Is(err, permissionDeniedErr) && (action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate) {
// If users dont have permission on the whole applications, maybe they have fine-grained access to the specific resources
if action == rbacpolicy.ActionDelete || action == rbacpolicy.ActionUpdate {
action = fmt.Sprintf("%s/%s/%s/%s/%s", action, q.GetGroup(), q.GetKind(), q.GetNamespace(), q.GetResourceName())
a, _, err = s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName())
}
a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName())
if err != nil {
return nil, nil, nil, err
}
Expand Down
Loading

0 comments on commit a931647

Please sign in to comment.