Skip to content

Commit 66c3f3f

Browse files
committed
Update verbs used to check SAR for users when k8s components are used
Update SAR checks for user permissions in webhook server to check whether a user can get/create/update/delete the resource rather than checking for '*' permissions. This is required as even if the user has the admin rolebinding, they do not have '*' permissions from the perspective of the cluster. Signed-off-by: Angel Misevski <amisevsk@redhat.com>
1 parent a4c8aaa commit 66c3f3f

1 file changed

Lines changed: 47 additions & 29 deletions

File tree

webhook/workspace/handler/kubernetes.go

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ import (
2626
"sigs.k8s.io/yaml"
2727
)
2828

29+
var (
30+
// Verbs used in SAR checks when users attempt to use a Kubernetes/OpenShift component. A longer list results in
31+
// more SAR checks per request. We cannot use '*' as a verb as the SAR will check for literal '*' permissions
32+
// rather than "all verbs"
33+
userVerbs = []string{"get", "create", "update", "delete"}
34+
)
35+
2936
func (h *WebhookHandler) validateKubernetesObjectPermissionsOnCreate(ctx context.Context, req admission.Request, wksp *dwv2.DevWorkspaceTemplateSpec) error {
3037
kubeComponents := getKubeComponentsFromWorkspace(wksp)
3138
for componentName, component := range kubeComponents {
@@ -97,37 +104,14 @@ func (h *WebhookHandler) validatePermissionsOnObject(ctx context.Context, req ad
97104
// Convert e.g. Pod -> pods, Deployment -> deployments
98105
resourceType := fmt.Sprintf("%ss", strings.ToLower(typeMeta.Kind))
99106

100-
sar := &authv1.LocalSubjectAccessReview{
101-
ObjectMeta: metav1.ObjectMeta{
102-
Namespace: req.Namespace,
103-
},
104-
Spec: authv1.SubjectAccessReviewSpec{
105-
ResourceAttributes: &authv1.ResourceAttributes{
106-
Namespace: req.Namespace,
107-
Verb: "*",
108-
Group: typeMeta.GroupVersionKind().Group,
109-
Version: typeMeta.GroupVersionKind().Version,
110-
Resource: resourceType,
111-
},
112-
User: req.UserInfo.Username,
113-
Groups: req.UserInfo.Groups,
114-
UID: req.UserInfo.UID,
115-
},
116-
}
117-
118-
err := h.Client.Create(ctx, sar)
119-
if err != nil {
120-
return fmt.Errorf("failed to create subjectaccessreview for request: %w", err)
121-
}
122-
123-
username := req.UserInfo.Username
124-
if username == "" {
125-
username = req.UserInfo.UID
126-
}
127-
if !sar.Status.Allowed {
128-
return fmt.Errorf("user %s does not have permissions to work with objects of kind %s defined in component %s", username, typeMeta.GroupVersionKind().String(), componentName)
107+
// Check that user has permissions to use the resource
108+
for _, verb := range userVerbs {
109+
if err := h.checkSAR(ctx, req, typeMeta, resourceType, verb, componentName); err != nil {
110+
return err
111+
}
129112
}
130113

114+
// Check that DWO has '*' permissions for the relevant resource
131115
ssar := &authv1.LocalSubjectAccessReview{
132116
ObjectMeta: metav1.ObjectMeta{
133117
Namespace: req.Namespace,
@@ -250,3 +234,37 @@ func getKubeLikeComponent_v1alpha1(component *dwv1.Component) (*dwv1.K8sLikeComp
250234
}
251235
return nil, fmt.Errorf("component does not specify kubernetes or openshift fields")
252236
}
237+
238+
func (h *WebhookHandler) checkSAR(ctx context.Context, req admission.Request, typeMeta *metav1.TypeMeta, resourceType, verb, componentName string) error {
239+
sar := &authv1.LocalSubjectAccessReview{
240+
ObjectMeta: metav1.ObjectMeta{
241+
Namespace: req.Namespace,
242+
},
243+
Spec: authv1.SubjectAccessReviewSpec{
244+
ResourceAttributes: &authv1.ResourceAttributes{
245+
Namespace: req.Namespace,
246+
Verb: verb,
247+
Group: typeMeta.GroupVersionKind().Group,
248+
Version: typeMeta.GroupVersionKind().Version,
249+
Resource: resourceType,
250+
},
251+
User: req.UserInfo.Username,
252+
Groups: req.UserInfo.Groups,
253+
UID: req.UserInfo.UID,
254+
},
255+
}
256+
257+
err := h.Client.Create(ctx, sar)
258+
if err != nil {
259+
return fmt.Errorf("failed to create subjectaccessreview for request: %w", err)
260+
}
261+
262+
username := req.UserInfo.Username
263+
if username == "" {
264+
username = req.UserInfo.UID
265+
}
266+
if !sar.Status.Allowed {
267+
return fmt.Errorf("user %s does not have permissions to '%s' objects of kind %s defined in component %s", username, verb, typeMeta.GroupVersionKind().String(), componentName)
268+
}
269+
return nil
270+
}

0 commit comments

Comments
 (0)