Skip to content

Commit

Permalink
Validate billingEntityRef on Organization object (#87)
Browse files Browse the repository at this point in the history
bastjan authored Jan 10, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent fa0bfa8 commit 7e036f5
Showing 9 changed files with 227 additions and 24 deletions.
4 changes: 3 additions & 1 deletion api.go
Original file line number Diff line number Diff line change
@@ -24,11 +24,12 @@ import (
func APICommand() *cobra.Command {
roles := []string{}
usernamePrefix := ""
var allowEmptyBillingEntity bool

ob := &odooStorageBuilder{}

cmd, err := builder.APIServer.
WithResourceAndHandler(&orgv1.Organization{}, orgStore.New(&roles, &usernamePrefix)).
WithResourceAndHandler(&orgv1.Organization{}, orgStore.New(&roles, &usernamePrefix, &allowEmptyBillingEntity)).
WithResourceAndHandler(&billingv1.BillingEntity{}, ob.Build).
WithoutEtcd().
ExposeLoopbackAuthorizer().
@@ -40,6 +41,7 @@ func APICommand() *cobra.Command {
cmd.Use = "api"
cmd.Flags().StringSliceVar(&roles, "cluster-roles", []string{}, "Cluster Roles to bind when creating an organization")
cmd.Flags().StringVar(&usernamePrefix, "username-prefix", "", "Prefix prepended to username claims. Usually the same as \"--oidc-username-prefix\" of the Kubernetes API server")
cmd.Flags().BoolVar(&allowEmptyBillingEntity, "allow-empty-billing-entity", true, "Allow empty billing entity references")

cmd.Flags().StringVar(&ob.billingEntityStorage, "billing-entity-storage", "fake", "Storage backend for billing entities. Supported values: fake, odoo8")
cmd.Flags().BoolVar(&ob.billingEntityFakeMetadataSupport, "billing-entity-fake-metadata-support", false, "Enable metadata support for the fake storage backend")
73 changes: 73 additions & 0 deletions apiserver/organization/billingentity_validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package organization

import (
"context"
"fmt"

"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/endpoints/request"
restclient "k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"

billingv1 "github.com/appuio/control-api/apis/billing/v1"
orgv1 "github.com/appuio/control-api/apis/organization/v1"
)

// +kubebuilder:rbac:groups="",resources=users;groups;serviceaccounts,verbs=impersonate

// impersonator can build a client that impersonates a user
type impersonator interface {
Impersonate(u user.Info) (client.Client, error)
}

// impersonatorFromRestconf can build a client that impersonates a user
// from a rest.Config and client.Options
type impersonatorFromRestconf struct {
config *restclient.Config
opts client.Options
}

var _ impersonator = impersonatorFromRestconf{}

// Impersonate returns a client that impersonates the given user
func (c impersonatorFromRestconf) Impersonate(u user.Info) (client.Client, error) {
conf := restclient.CopyConfig(c.config)

conf.Impersonate = restclient.ImpersonationConfig{
UserName: u.GetName(),
UID: u.GetUID(),
Groups: u.GetGroups(),
Extra: u.GetExtra(),
}
return client.New(conf, c.opts)
}

// billingEntityValidator validates that the billing entity exists and the requesting user has access to it.
// it does so by impersonating the user and trying to get the billing entity.
func (s *organizationStorage) billingEntityValidator(ctx context.Context, org, oldOrg *orgv1.Organization) error {
// check if changed
if oldOrg != nil && oldOrg.Spec.BillingEntityRef == org.Spec.BillingEntityRef {
return nil
}
// check if we allow empty billing entities
if org.Spec.BillingEntityRef == "" && s.allowEmptyBillingEntity {
return nil
}

user, ok := request.UserFrom(ctx)
if !ok {
return fmt.Errorf("no user in context")
}

var be billingv1.BillingEntity
c, err := s.impersonator.Impersonate(user)
if err != nil {
return fmt.Errorf("failed to impersonate user: %w", err)
}

if err := c.Get(ctx, client.ObjectKey{Name: org.Spec.BillingEntityRef}, &be); err != nil {
return err
}

return nil
}
3 changes: 3 additions & 0 deletions apiserver/organization/create.go
Original file line number Diff line number Diff line change
@@ -25,6 +25,9 @@ func (s *organizationStorage) Create(ctx context.Context, obj runtime.Object, cr
if err := createValidation(ctx, obj); err != nil {
return nil, err
}
if err := s.billingEntityValidator(ctx, org, nil); err != nil {
return nil, fmt.Errorf("failed to validate billing entity reference: %w", err)
}

return s.create(ctx, org, options)
}
27 changes: 27 additions & 0 deletions apiserver/organization/create_test.go
Original file line number Diff line number Diff line change
@@ -48,6 +48,33 @@ func TestOrganizationStorage_Create(t *testing.T) {
memberName: "smith",
organizationOut: fooOrg,
},
"GivenCreateOrgValidBillingEntity_ThenSuccess": {
userID: "appuio#smith",
organizationIn: func() *orgv1.Organization {
fooOrg := fooOrg.DeepCopy()
fooOrg.Spec.BillingEntityRef = "foo"
return fooOrg
}(),
authDecision: authResponse{
decision: authorizer.DecisionAllow,
},
memberName: "smith",
organizationOut: fooOrg,
},
"GivenCreateOrgInvalidBillingEntity_ThenFail": {
userID: "appuio#smith",
organizationIn: func() *orgv1.Organization {
fooOrg := fooOrg.DeepCopy()
fooOrg.Spec.BillingEntityRef = "blub"
return fooOrg
}(),
authDecision: authResponse{
decision: authorizer.DecisionAllow,
},
memberName: "smith",
err: errors.New("failed to validate billing entity reference: billingentities.billing.appuio.io \"blub\" not found"),
skipRoleBindings: true,
},
"GivenCreateOrgFromNonAPPUiOUser_ThenSuccessButNoSA": {
userID: "cluster-admin",
organizationIn: fooOrg,
19 changes: 15 additions & 4 deletions apiserver/organization/organization.go
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import (
"errors"
"strings"

billingv1 "github.com/appuio/control-api/apis/billing/v1"
orgv1 "github.com/appuio/control-api/apis/organization/v1"
controlv1 "github.com/appuio/control-api/apis/v1"
"github.com/appuio/control-api/apiserver/authwrapper"
@@ -27,14 +28,18 @@ import (
// +kubebuilder:rbac:groups="flowcontrol.apiserver.k8s.io",resources=prioritylevelconfigurations;flowschemas,verbs=get;list;watch

// New returns a new storage provider for Organizations
func New(clusterRoles *[]string, usernamePrefix *string) restbuilder.ResourceHandlerProvider {
func New(clusterRoles *[]string, usernamePrefix *string, allowEmptyBillingEntity *bool) restbuilder.ResourceHandlerProvider {
return func(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) {
masterConfig := loopback.GetLoopbackMasterClientConfig()

c, err := client.NewWithWatch(loopback.GetLoopbackMasterClientConfig(), client.Options{})
if err != nil {
return nil, err
}
err = controlv1.AddToScheme(c.Scheme())
if err != nil {
if err := controlv1.AddToScheme(c.Scheme()); err != nil {
return nil, err
}
if err := billingv1.AddToScheme(c.Scheme()); err != nil {
return nil, err
}

@@ -49,7 +54,9 @@ func New(clusterRoles *[]string, usernamePrefix *string) restbuilder.ResourceHan
members: kubeMemberProvider{
Client: c,
},
usernamePrefix: *usernamePrefix,
usernamePrefix: *usernamePrefix,
impersonator: impersonatorFromRestconf{masterConfig, client.Options{Scheme: c.Scheme()}},
allowEmptyBillingEntity: *allowEmptyBillingEntity,
}

return authwrapper.NewAuthorizedStorage(stor, metav1.GroupVersionResource{
@@ -68,6 +75,10 @@ type organizationStorage struct {

rbac roleBindingCreator
clusterRoles []string

impersonator impersonator

allowEmptyBillingEntity bool
}

func (s organizationStorage) New() runtime.Object {
33 changes: 32 additions & 1 deletion apiserver/organization/organization_test.go
Original file line number Diff line number Diff line change
@@ -9,11 +9,16 @@ import (
mock "github.com/appuio/control-api/apiserver/organization/mock"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

billingv1 "github.com/appuio/control-api/apis/billing/v1"
orgv1 "github.com/appuio/control-api/apis/organization/v1"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
)

@@ -23,7 +28,9 @@ func newMockedOrganizationStorage(t *testing.T, ctrl *gomock.Controller) (authwr
mnp := mock.NewMocknamespaceProvider(ctrl)
mauth := authmock.NewMockAuthorizer(ctrl)
os, err := authwrapper.NewAuthorizedStorage(&organizationStorage{
namepaces: mnp,
namepaces: mnp,
allowEmptyBillingEntity: true,
impersonator: &fakeImpersonator{t},
}, metav1.GroupVersionResource{
Group: orgv1.GroupVersion.Group,
Version: orgv1.GroupVersion.Version,
@@ -109,3 +116,27 @@ func (m authRequestMatcher) String() string {
func isAuthRequest(verb string) authRequestMatcher {
return authRequestMatcher{verb: verb}
}

type fakeImpersonator struct{ t *testing.T }

func (c fakeImpersonator) Impersonate(u user.Info) (client.Client, error) {
c.t.Helper()

scheme := runtime.NewScheme()
require.NoError(c.t, billingv1.AddToScheme(scheme))

objs := []runtime.Object{
&billingv1.BillingEntity{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
},
&billingv1.BillingEntity{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
},
},
}

return fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build(), nil
}
18 changes: 11 additions & 7 deletions apiserver/organization/update.go
Original file line number Diff line number Diff line change
@@ -18,30 +18,34 @@ func (s *organizationStorage) Update(ctx context.Context, name string, objInfo r
createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc,
forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {

newOrg := &orgv1.Organization{}

oldOrg, err := s.Get(ctx, name, nil)
oldObj, err := s.Get(ctx, name, nil)
if err != nil {

return nil, false, err
}
oldOrg, ok := oldObj.(*orgv1.Organization)
if !ok {
return nil, false, fmt.Errorf("old object is not an organization")
}

newObj, err := objInfo.UpdatedObject(ctx, oldOrg)
newObj, err := objInfo.UpdatedObject(ctx, oldObj)
if err != nil {
return nil, false, err
}

newOrg, ok := newObj.(*orgv1.Organization)
if !ok {
return nil, false, fmt.Errorf("new object is not an organization")
}

if updateValidation != nil {
err = updateValidation(ctx, newOrg, oldOrg)
err = updateValidation(ctx, newOrg, oldObj)
if err != nil {
return nil, false, err
}
}

if err := s.billingEntityValidator(ctx, newOrg, oldOrg); err != nil {
return nil, false, fmt.Errorf("failed to validate billing entity reference: %w", err)
}

return newOrg, false, convertNamespaceError(s.namepaces.UpdateNamespace(ctx, newOrg.ToNamespace(), options))
}
66 changes: 55 additions & 11 deletions apiserver/organization/update_test.go
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
)
@@ -46,10 +47,7 @@ func TestOrganizationStorage_Update(t *testing.T) {
"GivenUpdateOrg_ThenSuccess": {
name: "foo",
updateFunc: func(obj runtime.Object) runtime.Object {
org, ok := obj.(*orgv1.Organization)
if !ok {
return nil
}
org := obj.(*orgv1.Organization).DeepCopy()
org.Spec.DisplayName = "New Foo Inc."
return org
},
@@ -70,6 +68,47 @@ func TestOrganizationStorage_Update(t *testing.T) {
},
},
},
"GivenUpdateOrg_ValidBillingEntity_ThenSuccess": {
name: "foo",
updateFunc: func(obj runtime.Object) runtime.Object {
org := obj.(*orgv1.Organization).DeepCopy()
org.Spec.BillingEntityRef = "foo"
return org
},

namespace: fooNs,
authDecision: authResponse{
decision: authorizer.DecisionAllow,
},

organization: &orgv1.Organization{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Labels: map[string]string{},
Annotations: map[string]string{},
},
Spec: orgv1.OrganizationSpec{
DisplayName: "Foo Inc.",
BillingEntityRef: "foo",
},
},
},
"GivenUpdateOrg_InvalidBillingEntity_ThenFail": {
name: "foo",
updateFunc: func(obj runtime.Object) runtime.Object {
org := obj.(*orgv1.Organization).DeepCopy()
org.Spec.BillingEntityRef = "invalid"
return org
},

namespace: fooNs,
authDecision: authResponse{
decision: authorizer.DecisionAllow,
},

err: apierrors.NewBadRequest("failed to validate billing entity reference: billingentities.billing.appuio.io \"invalid\" not found"),
},

"GivenUpdateNonOrg_ThenFail": {
name: "default",
namespace: defaultNs,
@@ -149,13 +188,18 @@ func TestOrganizationStorage_Update(t *testing.T) {
Return(tc.namespaceUpdateErr).
AnyTimes()

org, _, err := os.Update(request.WithRequestInfo(request.NewContext(),
&request.RequestInfo{
Verb: "update",
APIGroup: orgv1.GroupVersion.Group,
Resource: "organizations",
Name: tc.name,
}),
org, _, err := os.Update(
request.WithUser(
request.WithRequestInfo(request.NewContext(),
&request.RequestInfo{
Verb: "update",
APIGroup: orgv1.GroupVersion.Group,
Resource: "organizations",
Name: tc.name,
}),
&user.DefaultInfo{
Name: "appuio#foo",
}),
tc.name, testUpdateInfo(tc.updateFunc), nil, nil, false, nil)

if tc.err != nil {
8 changes: 8 additions & 0 deletions config/rbac/apiserver/role.yaml
Original file line number Diff line number Diff line change
@@ -15,6 +15,14 @@ rules:
- get
- list
- watch
- apiGroups:
- ""
resources:
- groups
- serviceaccounts
- users
verbs:
- impersonate
- apiGroups:
- ""
resources:

0 comments on commit 7e036f5

Please sign in to comment.