Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
9 changes: 7 additions & 2 deletions cmd/authz/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@ func handleAuthorize(w http.ResponseWriter, r *http.Request, c client.Client, ma
}

attrs := review.Spec.ResourceAttributes
if attrs == nil || attrs.Name == "" {
if attrs == nil {
recordDenied("", "", reasonMissingAttributes, nil)
respond(w, review, false, "missing resource attributes")
respond(w, review, false, "authorization webhook cannot authorize requests with missing resource attributes")
return
}
if attrs.Name == "" {
recordDenied("", "", reasonMissingAttributes, nil)
Comment thread
Zaggy21 marked this conversation as resolved.
respond(w, review, false, "authorization webhook cannot authorize collection operations (e.g. list) - grant access via RBAC instead")
Comment thread
Zaggy21 marked this conversation as resolved.
Outdated
return
}

Expand Down
21 changes: 21 additions & 0 deletions cmd/authz/authorization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,27 @@ var _ = Describe("handleAuthorize", func() {
Expect(resp.Status.Allowed).To(BeFalse(), "requests without resource attributes should be denied")
Expect(resp.Status.Reason).To(ContainSubstring("missing resource attributes"), "denial reason should mention missing attributes")
})

It("should deny collection operations with empty resource name", func() {
h := makeHandler(nil)
review := authv1.SubjectAccessReview{
Spec: authv1.SubjectAccessReviewSpec{
User: "demo-user",
Groups: []string{"support-group:demo"},
ResourceAttributes: &authv1.ResourceAttributes{
Namespace: "my-org",
Verb: "list",
Group: greenhousev1alpha1.GroupVersion.Group,
Version: greenhousev1alpha1.GroupVersion.Version,
Resource: "plugins",
// Name intentionally empty (collection operation)
},
},
}
resp := postReview(h, review)
Expect(resp.Status.Allowed).To(BeFalse(), "collection operations with empty resource name should be denied")
Expect(resp.Status.Reason).To(ContainSubstring("cannot authorize collection operations"), "denial reason should mention collection operations")
})
Comment thread
Zaggy21 marked this conversation as resolved.
Comment thread
abhijith-darshan marked this conversation as resolved.
Outdated
})

Context("user authorization with support groups", func() {
Expand Down
5 changes: 3 additions & 2 deletions dev-env/webhook/structured-authz-insecure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ authorizers:
- expression: has(request.resourceAttributes) # only resource requests
- expression: request.resourceAttributes.namespace != ""
- expression: request.resourceAttributes.group == "greenhouse.sap"
# Note on verbs: get is needed for kubectl edit, list and watch is needed for kubectl wait triggered after kubectl delete.
- expression: request.resourceAttributes.verb in ["get","list","watch","update","patch","delete"]
# Note on verbs: get is needed for kubectl edit; watch is still needed for kubectl wait triggered after kubectl delete.
# List is intentionally not matched here and should be handled via RBAC instead.
- expression: request.resourceAttributes.verb in ["get","watch","update","patch","delete"]

5 changes: 3 additions & 2 deletions dev-env/webhook/structured-authz-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ authorizers:
- expression: has(request.resourceAttributes) # only resource requests
- expression: request.resourceAttributes.namespace != ""
- expression: request.resourceAttributes.group == "greenhouse.sap"
# Note on verbs: get is needed for kubectl edit, list and watch is needed for kubectl wait triggered after kubectl delete.
- expression: request.resourceAttributes.verb in ["get","list","watch","update","patch","delete"]
# Note on verbs: get is needed for kubectl edit; watch is still needed for kubectl wait triggered after kubectl delete.
# List is intentionally not matched here and should be handled via RBAC instead.
- expression: request.resourceAttributes.verb in ["get","watch","update","patch","delete"]

Loading