Make the creator of an Invitation the owner (#120)

apiserver/user/invitation_storage.go

@@ -1,6 +1,7 @@
 package user
 
 import (
+	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic"
@@ -26,6 +27,10 @@ func NewInvitationStorage(backingNS, usernamePrefix string) restbuilder.Resource
 		if err != nil {
 			return nil, err
 		}
+		err = rbacv1.AddToScheme(c.Scheme())
+		if err != nil {
+			return nil, err
+		}
 		stor, err := secretstorage.NewStorage(&userv1.Invitation{}, c, backingNS)
 		if err != nil {
 			return nil, err
@@ -37,6 +42,11 @@ func NewInvitationStorage(backingNS, usernamePrefix string) restbuilder.Resource
 			usernamePrefix:        usernamePrefix,
 		}
 
+		stor = &rbacCreatorIsOwner{
+			ScopedStandardStorage: stor,
+			client:                c,
+		}
+
 		astor, err := authwrapper.NewAuthorizedStorage(stor, metav1.GroupVersionResource{
 			Group:    "rbac.appuio.io",
 			Version:  "v1",

apiserver/user/rbac.go

@@ -0,0 +1,153 @@
+package user
+
+import (
+	"context"
+	"crypto/sha1"
+	"encoding/hex"
+	"fmt"
+
+	"go.uber.org/multierr"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/endpoints/filters"
+	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/klog/v2"
+	"k8s.io/utils/strings"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/appuio/control-api/apiserver/secretstorage"
+)
+
+// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch;create;delete;patch;update;edit
+// +kubebuilder:rbac:groups=rbac.appuio.io;user.appuio.io,resources=invitations,verbs=get;edit;update;patch;delete
+
+// rbacCreatorIsOwner is a wrapper around the Invitation storage that creates a ClusterRole and ClusterRoleBinding
+// to make the creator of the Invitation the owner of the Invitation.
+type rbacCreatorIsOwner struct {
+	secretstorage.ScopedStandardStorage
+	client client.Client
+}
+
+// Create passes the object to the wrapped storage and creates a ClusterRole and ClusterRoleBinding for the creator of the object using the returned object's name.
+func (c *rbacCreatorIsOwner) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, opts *metav1.CreateOptions) (runtime.Object, error) {
+	attr, err := filters.GetAuthorizerAttributes(ctx)
+	if err != nil {
+		return nil, err
+	}
+	user := attr.GetUser()
+
+	createdObj, err := c.ScopedStandardStorage.Create(ctx, obj, createValidation, opts)
+	if err != nil {
+		return createdObj, err
+	}
+
+	ac := apimeta.NewAccessor()
+	objName, err := ac.Name(createdObj)
+	if err != nil {
+		return createdObj, fmt.Errorf("could not get name of created object: %w", err)
+	}
+
+	rolename := roleName(objName)
+
+	role := &rbacv1.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: rolename,
+		},
+		Rules: []rbacv1.PolicyRule{
+			{
+				APIGroups:     []string{"rbac.appuio.io", "user.appuio.io"},
+				Resources:     []string{"invitations"},
+				Verbs:         []string{"get", "edit", "update", "patch", "delete"},
+				ResourceNames: []string{objName},
+			},
+		},
+	}
+
+	rolebinding := &rbacv1.ClusterRoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: rolename,
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind:     "User",
+				APIGroup: "rbac.authorization.k8s.io",
+				Name:     user.GetName(),
+			},
+		},
+		RoleRef: rbacv1.RoleRef{
+			Kind:     "ClusterRole",
+			APIGroup: "rbac.authorization.k8s.io",
+			Name:     rolename,
+		},
+	}
+
+	rollback := func() error {
+		_, _, err := c.ScopedStandardStorage.Delete(ctx, objName, nil, &metav1.DeleteOptions{DryRun: opts.DryRun})
+		return err
+	}
+
+	err = c.client.Create(ctx, role, &client.CreateOptions{DryRun: opts.DryRun})
+	if err != nil {
+		rollbackErr := rollback()
+		return createdObj, multierr.Append(err, rollbackErr)
+	}
+	err = c.client.Create(ctx, rolebinding, &client.CreateOptions{DryRun: opts.DryRun})
+	if err != nil {
+		rollbackErr := rollback()
+		roleRollbackErr := c.client.Delete(ctx, role, &client.DeleteOptions{DryRun: opts.DryRun})
+		return createdObj, multierr.Combine(err, rollbackErr, roleRollbackErr)
+	}
+
+	return createdObj, nil
+}
+
+// Delete passes the object to the wrapped storage and deletes the ClusterRole and ClusterRoleBinding associated with the object.
+func (c *rbacCreatorIsOwner) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, opts *metav1.DeleteOptions) (runtime.Object, bool, error) {
+	deletedObj, im, err := c.ScopedStandardStorage.Delete(ctx, name, deleteValidation, opts)
+	if err != nil {
+		return deletedObj, im, err
+	}
+
+	rolename := roleName(name)
+	err1 := c.client.Delete(ctx, &rbacv1.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: rolename,
+		},
+	}, &client.DeleteOptions{DryRun: opts.DryRun})
+	err2 := c.client.Delete(ctx, &rbacv1.ClusterRoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: rolename,
+		},
+	}, &client.DeleteOptions{DryRun: opts.DryRun})
+
+	if err := multierr.Combine(err1, err2); err != nil {
+		klog.FromContext(ctx).Error(err, "failed to clean up RBAC resources")
+	}
+
+	return deletedObj, im, nil
+}
+
+func (s *rbacCreatorIsOwner) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) {
+	return nil, fmt.Errorf("not implemented")
+}
+
+func roleName(objName string) string {
+	prefix := "invitations-"
+	suffix := "-owner"
+
+	if len(prefix)+len(suffix)+len(objName) <= 63 {
+		return fmt.Sprintf("%s%s%s", prefix, objName, suffix)
+	}
+
+	h := sha1.New()
+	h.Write([]byte(objName))
+	hsh := strings.ShortenString(hex.EncodeToString(h.Sum(nil)), 7)
+
+	maxLength := 63 - len(prefix) - len(suffix) - len(hsh) - 1
+	maxSafe := strings.ShortenString(objName, maxLength)
+
+	return fmt.Sprintf("%s%s-%s%s", prefix, maxSafe, hsh, suffix)
+}

apiserver/user/rbac_test.go

@@ -0,0 +1,162 @@
+package user
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/registry/rest"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/appuio/control-api/apiserver/authwrapper/mock"
+	"github.com/appuio/control-api/apiserver/testresource"
+)
+
+func Test_createRBACWrapper_E2E(t *testing.T) {
+	td := []struct {
+		description  string
+		resourceName string
+		roleName     string
+	}{
+		{
+			description:  "short name",
+			resourceName: "inv-test",
+			roleName:     "invitations-inv-test-owner",
+		},
+		{
+			description:  "long name",
+			resourceName: strings.Repeat("a", 63),
+			roleName:     "invitations-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-03f09f5-owner",
+		},
+	}
+
+	for _, tc := range td {
+		t.Run(tc.description, func(t *testing.T) {
+			user := "testuser"
+
+			c := newClient()
+			ctrl, store := newStore(t)
+			defer ctrl.Finish()
+
+			subject := &rbacCreatorIsOwner{
+				ScopedStandardStorage: clusterScopedStorage{store},
+				client:                c,
+			}
+
+			store.EXPECT().
+				Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+				Return(&testresource.TestResource{ObjectMeta: metav1.ObjectMeta{Name: tc.resourceName}}, nil).
+				Times(1)
+
+			store.EXPECT().
+				Delete(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+				Return(&testresource.TestResource{ObjectMeta: metav1.ObjectMeta{Name: tc.resourceName}}, true, nil).
+				Times(2)
+
+			// Create
+			_, err := subject.Create(ctxWithInfo("create", "", user), &testresource.TestResource{}, nil, &metav1.CreateOptions{})
+			require.NoError(t, err)
+
+			var role rbacv1.ClusterRole
+			require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: tc.roleName}, &role))
+			var clusterrole rbacv1.ClusterRoleBinding
+			require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: tc.roleName}, &clusterrole))
+			assert.Equal(t, []rbacv1.Subject{{APIGroup: "rbac.authorization.k8s.io", Kind: "User", Name: user}}, clusterrole.Subjects)
+
+			// Delete
+			_, _, err = subject.Delete(ctxWithInfo("delete", "", user), tc.resourceName, nil, &metav1.DeleteOptions{})
+			require.NoError(t, err)
+			assert.True(t, apierrors.IsNotFound(c.Get(context.Background(), types.NamespacedName{Name: tc.roleName}, &role)))
+			assert.True(t, apierrors.IsNotFound(c.Get(context.Background(), types.NamespacedName{Name: tc.roleName}, &clusterrole)))
+			_, _, err = subject.Delete(ctxWithInfo("delete", "", user), tc.resourceName, nil, &metav1.DeleteOptions{})
+			require.NoError(t, err, "expected no error if role is already deleted")
+		})
+	}
+}
+
+func Test_createRBACWrapper_rollback(t *testing.T) {
+	user := "testuser"
+	returnedResourceName := "inv-test"
+
+	c := newClient()
+	ctrl, store := newStore(t)
+	defer ctrl.Finish()
+
+	subject := &rbacCreatorIsOwner{
+		ScopedStandardStorage: clusterScopedStorage{store},
+		client:                c,
+	}
+
+	store.EXPECT().
+		Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+		Return(&testresource.TestResource{ObjectMeta: metav1.ObjectMeta{Name: returnedResourceName}}, nil).
+		Times(1)
+
+	store.EXPECT().
+		Delete(gomock.Any(), returnedResourceName, gomock.Any(), gomock.Any()).
+		Return(nil, true, nil).
+		Times(1)
+
+	// Force an already exist error to trigger rollback
+	rn := "invitations-" + returnedResourceName + "-owner"
+	require.NoError(t,
+		c.Create(context.Background(), &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: rn}}),
+	)
+
+	_, err := subject.Create(ctxWithInfo("create", "", user), &testresource.TestResource{}, nil, &metav1.CreateOptions{})
+	require.Error(t, err, "expected error on create to trigger rollback")
+
+	var role rbacv1.ClusterRole
+	err = c.Get(context.Background(), types.NamespacedName{Name: rn}, &role)
+	assert.True(t, apierrors.IsNotFound(err), "expected role to be deleted on rollback")
+}
+
+func newStore(t *testing.T) (*gomock.Controller, *mock.MockStandardStorage) {
+	ctrl := gomock.NewController(t)
+	store := mock.NewMockStandardStorage(ctrl)
+	return ctrl, store
+}
+
+func newClient() client.WithWatch {
+	scheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	return fake.NewClientBuilder().WithScheme(scheme).Build()
+}
+
+type clusterScopedStorage struct {
+	rest.StandardStorage
+}
+
+func (clusterScopedStorage) NamespaceScoped() bool {
+	return false
+}
+
+func ctxWithInfo(verb string, objName string, username string) context.Context {
+	gvr := (&testresource.TestResource{}).GetGroupVersionResource()
+	return request.WithUser(
+		request.WithRequestInfo(request.NewContext(),
+			&request.RequestInfo{
+				APIGroup:   gvr.Group,
+				APIVersion: gvr.Version,
+				Resource:   gvr.Resource,
+
+				Verb: verb,
+				Name: objName,
+			}),
+		&user.DefaultInfo{
+			Name: username,
+		})
+}

config/rbac/apiserver/role.yaml

@@ -146,6 +146,17 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - rbac.appuio.io
+  - user.appuio.io
+  resources:
+  - invitations
+  verbs:
+  - delete
+  - edit
+  - get
+  - patch
+  - update
 - apiGroups:
   - rbac.authorization.k8s.io
   resources: