From e47c5ed7572a6daf110667f87b9d4de58bccee87 Mon Sep 17 00:00:00 2001
From: Angel Misevski <amisevsk@redhat.com>
Date: Tue, 21 Feb 2023 11:23:45 -0500
Subject: [PATCH] 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>
---
 webhook/workspace/handler/kubernetes.go | 76 +++++++++++++++----------
 1 file changed, 47 insertions(+), 29 deletions(-)

diff --git a/webhook/workspace/handler/kubernetes.go b/webhook/workspace/handler/kubernetes.go
index 1b67130c6..9c397af90 100644
--- a/webhook/workspace/handler/kubernetes.go
+++ b/webhook/workspace/handler/kubernetes.go
@@ -26,6 +26,13 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+var (
+	// Verbs used in SAR checks when users attempt to use a Kubernetes/OpenShift component. A longer list results in
+	// more SAR checks per request. We cannot use '*' as a verb as the SAR will check for literal '*' permissions
+	// rather than "all verbs"
+	userVerbs = []string{"get", "create", "update", "delete"}
+)
+
 func (h *WebhookHandler) validateKubernetesObjectPermissionsOnCreate(ctx context.Context, req admission.Request, wksp *dwv2.DevWorkspaceTemplateSpec) error {
 	kubeComponents := getKubeComponentsFromWorkspace(wksp)
 	for componentName, component := range kubeComponents {
@@ -97,37 +104,14 @@ func (h *WebhookHandler) validatePermissionsOnObject(ctx context.Context, req ad
 	// Convert e.g. Pod -> pods, Deployment -> deployments
 	resourceType := fmt.Sprintf("%ss", strings.ToLower(typeMeta.Kind))
 
-	sar := &authv1.LocalSubjectAccessReview{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: req.Namespace,
-		},
-		Spec: authv1.SubjectAccessReviewSpec{
-			ResourceAttributes: &authv1.ResourceAttributes{
-				Namespace: req.Namespace,
-				Verb:      "*",
-				Group:     typeMeta.GroupVersionKind().Group,
-				Version:   typeMeta.GroupVersionKind().Version,
-				Resource:  resourceType,
-			},
-			User:   req.UserInfo.Username,
-			Groups: req.UserInfo.Groups,
-			UID:    req.UserInfo.UID,
-		},
-	}
-
-	err := h.Client.Create(ctx, sar)
-	if err != nil {
-		return fmt.Errorf("failed to create subjectaccessreview for request: %w", err)
-	}
-
-	username := req.UserInfo.Username
-	if username == "" {
-		username = req.UserInfo.UID
-	}
-	if !sar.Status.Allowed {
-		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)
+	// Check that user has permissions to use the resource
+	for _, verb := range userVerbs {
+		if err := h.checkSAR(ctx, req, typeMeta, resourceType, verb, componentName); err != nil {
+			return err
+		}
 	}
 
+	// Check that DWO has '*' permissions for the relevant resource
 	ssar := &authv1.LocalSubjectAccessReview{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: req.Namespace,
@@ -250,3 +234,37 @@ func getKubeLikeComponent_v1alpha1(component *dwv1.Component) (*dwv1.K8sLikeComp
 	}
 	return nil, fmt.Errorf("component does not specify kubernetes or openshift fields")
 }
+
+func (h *WebhookHandler) checkSAR(ctx context.Context, req admission.Request, typeMeta *metav1.TypeMeta, resourceType, verb, componentName string) error {
+	sar := &authv1.LocalSubjectAccessReview{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: req.Namespace,
+		},
+		Spec: authv1.SubjectAccessReviewSpec{
+			ResourceAttributes: &authv1.ResourceAttributes{
+				Namespace: req.Namespace,
+				Verb:      verb,
+				Group:     typeMeta.GroupVersionKind().Group,
+				Version:   typeMeta.GroupVersionKind().Version,
+				Resource:  resourceType,
+			},
+			User:   req.UserInfo.Username,
+			Groups: req.UserInfo.Groups,
+			UID:    req.UserInfo.UID,
+		},
+	}
+
+	err := h.Client.Create(ctx, sar)
+	if err != nil {
+		return fmt.Errorf("failed to create subjectaccessreview for request: %w", err)
+	}
+
+	username := req.UserInfo.Username
+	if username == "" {
+		username = req.UserInfo.UID
+	}
+	if !sar.Status.Allowed {
+		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)
+	}
+	return nil
+}