Merge pull request #71 from authzed/authorize-updates

add support for authorizing update request (`update` and `patch` verbs)

Author: ecordell

e2e/proxy_test.go

@@ -4,6 +4,7 @@ package e2e
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"sync"
 	"time"
@@ -15,10 +16,12 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
+	"k8s.io/utils/pointer"
 
 	"github.com/authzed/spicedb-kubeapi-proxy/pkg/authz/distributedtx"
 	"github.com/authzed/spicedb-kubeapi-proxy/pkg/config/proxyrule"
@@ -136,6 +139,41 @@ var _ = Describe("Proxy", func() {
 			_, err := client.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{})
 			return err
 		}
+		GetAndUpdatePodLabels := func(ctx context.Context, client kubernetes.Interface, namespace, name string, labels map[string]string) error {
+			pod, err := client.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			pod.Labels = labels
+			_, err = client.CoreV1().Pods(namespace).Update(ctx, pod, metav1.UpdateOptions{})
+			return err
+		}
+		PatchPodLabels := func(ctx context.Context, client kubernetes.Interface, namespace, name string, labels map[string]string) error {
+			patch := &corev1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Pod",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: namespace,
+					Labels:    labels,
+				},
+			}
+			data, err := json.Marshal(patch)
+			if err != nil {
+				return err
+			}
+			_, err = client.CoreV1().Pods(namespace).Patch(ctx, name, types.ApplyPatchType, data, metav1.PatchOptions{FieldManager: "Test", Force: pointer.Bool(true)})
+			return err
+		}
+		ListPods := func(ctx context.Context, client kubernetes.Interface, namespace string) []string {
+			visibleNamespaces, err := client.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{})
+			Expect(err).To(Succeed())
+			return lo.Map(visibleNamespaces.Items, func(item corev1.Pod, index int) string {
+				return item.Name
+			})
+		}
 		WatchPods := func(ctx context.Context, client kubernetes.Interface, namespace string, expected int, timeout time.Duration) []string {
 			ctx, cancel := context.WithTimeout(ctx, timeout)
 			defer cancel()
@@ -167,6 +205,43 @@ var _ = Describe("Proxy", func() {
 		})
 
 		AssertDualWriteBehavior := func() {
+
+			It("supports rules for every verb", func(ctx context.Context) {
+				// watch
+				var wg sync.WaitGroup
+				defer wg.Wait()
+				wg.Add(1)
+				go func() {
+					defer GinkgoRecover()
+					Expect(WatchPods(ctx, paulClient, paulNamespace, 1, 10*time.Second)).To(ContainElement(paulPod))
+					wg.Done()
+				}()
+
+				// create
+				Expect(CreateNamespace(ctx, paulClient, paulNamespace)).To(Succeed())
+				Expect(CreatePod(ctx, paulClient, paulNamespace, paulPod)).To(Succeed())
+
+				// get
+				Expect(GetPod(ctx, paulClient, paulNamespace, paulPod)).To(Succeed())
+				Expect(GetPod(ctx, chaniClient, paulNamespace, paulPod)).To(Not(Succeed()))
+
+				// update
+				Expect(GetAndUpdatePodLabels(ctx, paulClient, paulNamespace, paulPod, map[string]string{"a": "label"})).To(Succeed())
+				Expect(GetAndUpdatePodLabels(ctx, chaniClient, paulNamespace, paulPod, map[string]string{"a": "label"})).To(Not(Succeed()))
+
+				// patch
+				Expect(PatchPodLabels(ctx, paulClient, paulNamespace, paulPod, map[string]string{"b": "label"})).To(Succeed())
+				Expect(PatchPodLabels(ctx, chaniClient, paulNamespace, paulPod, map[string]string{"b": "label"})).To(Not(Succeed()))
+
+				// list
+				Expect(ListPods(ctx, paulClient, paulNamespace)).To(Equal([]string{paulPod}))
+				Expect(ListPods(ctx, chaniClient, paulNamespace)).To(Equal([]string{}))
+
+				// delete
+				Expect(DeletePod(ctx, chaniClient, paulNamespace, paulPod)).To(Not(Succeed()))
+				Expect(DeletePod(ctx, paulClient, paulNamespace, paulPod)).To(Succeed())
+			})
+
 			It("doesn't show users namespaces the other has created", func(ctx context.Context) {
 				var wg sync.WaitGroup
 				defer wg.Wait()
@@ -623,6 +698,9 @@ var (
 				Resource:     "pods",
 				Verbs:        []string{"delete"},
 			}},
+			Checks: []proxyrule.StringOrTemplate{{
+				Template: "pod:{{namespacedName}}#edit@user:{{user.Name}}",
+			}},
 			Writes: []proxyrule.StringOrTemplate{{
 				Template: "pod:{{namespacedName}}#creator@user:{{user.Name}}",
 			}, {
@@ -644,6 +722,19 @@ var (
 		},
 	}
 
+	updatePod = proxyrule.Config{
+		Spec: proxyrule.Spec{
+			Matches: []proxyrule.Match{{
+				GroupVersion: "v1",
+				Resource:     "pods",
+				Verbs:        []string{"update", "patch"},
+			}},
+			Checks: []proxyrule.StringOrTemplate{{
+				Template: "pod:{{namespacedName}}#edit@user:{{user.Name}}",
+			}},
+		},
+	}
+
 	listWatchPod = proxyrule.Config{
 		Spec: proxyrule.Spec{
 			Matches: []proxyrule.Match{{
@@ -669,6 +760,7 @@ func testOptimisticMatcher() rules.MapMatcher {
 		createPod(),
 		deletePod(),
 		getPod,
+		updatePod,
 		listWatchPod,
 	})
 	Expect(err).To(Succeed())
@@ -701,6 +793,7 @@ func testPessimisticMatcher() rules.MapMatcher {
 		pessimisticCreatePod,
 		pessimisticDeletePod,
 		getPod,
+		updatePod,
 		listWatchPod,
 	})
 	Expect(err).To(Succeed())

pkg/authz/authz.go

@@ -51,6 +51,7 @@ func WithAuthorization(handler, failed http.Handler, permissionsClient v1.Permis
 				"APIVersion", input.Request.APIVersion,
 				"Resource", input.Request.Resource)
 		}
+
 		// run all checks for this request
 		if err := runAllMatchingChecks(ctx, matchingRules, input, permissionsClient); err != nil {
 			handleError(w, failed, req, err)
@@ -72,7 +73,7 @@ func WithAuthorization(handler, failed http.Handler, permissionsClient v1.Permis
 			removedNNC: make(chan types.NamespacedName),
 			allowedNN:  map[types.NamespacedName]struct{}{},
 		}
-		authorizeGet(input, authzData)
+		alreadyAuthorized(input, authzData)
 		if err := filterResponse(ctx, matchingRules, input, authzData, permissionsClient, watchClient); err != nil {
 			failed.ServeHTTP(w, req)
 			return

pkg/authz/filter.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"slices"
 
 	"github.com/kyverno/go-jmespath"
 	"k8s.io/klog/v2"
@@ -49,16 +50,19 @@ func filterResponse(ctx context.Context, matchingRules []*rules.RunnableRule, in
 	return nil
 }
 
-func authorizeGet(input *rules.ResolveInput, authzData *AuthzData) {
-	if input.Request.Verb != "get" {
+// alreadyAuthorized adds the input object to the authorized list.
+// This is used for requests where the `check` on the rule is expected to authorize
+// a single object for filtering, i.e. for `get`, `update`, or `patch`
+// (where update/patch don't create the object).
+func alreadyAuthorized(input *rules.ResolveInput, authzData *AuthzData) {
+	if !slices.Contains([]string{"get", "update", "patch"}, input.Request.Verb) {
 		return
 	}
 	close(authzData.allowedNNC)
 	close(authzData.removedNNC)
 	authzData.Lock()
 	defer authzData.Unlock()
-	// gets aren't really filtered, but we add them to the allowed list so that
-	// downstream can know it's passed all configured checks.
+
 	authzData.allowedNN[types.NamespacedName{Name: input.Name, Namespace: input.Namespace}] = struct{}{}
 }
 

pkg/proxy/server.go

@@ -120,6 +120,7 @@ func NewServer(ctx context.Context, o Options) (*Server, error) {
 	}
 	s.WorkflowWorker = worker
 
+	// Matcher is a pointer to an interface to make it easy to swap at runtime in tests
 	s.Matcher = &s.opts.Matcher
 	handler, err := authz.WithAuthorization(clusterProxy, failHandler, o.PermissionsClient, o.WatchClient, workflowClient, s.Matcher, s.opts.inputExtractor)
 	if err != nil {

pkg/rules/rules.go

@@ -114,11 +114,13 @@ type RelExpr struct {
 // with resolved values.
 type ResolvedRel UncompiledRelExpr
 
+// ResolveInputExtractor defines how ResolveInput are extracted from requests.
+// This interface exists so that tests can easily fake the request data.
 type ResolveInputExtractor interface {
 	ExtractFromHttp(req *http.Request) (*ResolveInput, error)
 }
 
-// ResolveInputExtractorFunc is a function type that implements Matcher
+// ResolveInputExtractorFunc is a function type that implements ResolveInputExtractor
 type ResolveInputExtractorFunc func(req *http.Request) (*ResolveInput, error)
 
 func (f ResolveInputExtractorFunc) ExtractFromHttp(req *http.Request) (*ResolveInput, error) {
@@ -163,6 +165,8 @@ func NewResolveInputFromHttp(req *http.Request) (*ResolveInput, error) {
 			return nil, fmt.Errorf("unable to decode request body as kube object: %w", err)
 		}
 		object = &pom
+
+		req.Body = io.NopCloser(bytes.NewReader(body))
 	}
 	return NewResolveInput(requestInfo, userInfo.(*user.DefaultInfo), object, body, req.Header.Clone()), nil
 }

pkg/spicedb/bootstrap.yaml

@@ -11,6 +11,7 @@ schema: |-
     relation namespace: namespace
     relation creator: user
     relation viewer: user
+    permission edit = creator
     permission view = viewer + creator
   }
   definition lock {