Skip to content

Commit

Permalink
Add access control for watch and list and refactor authenticator
Browse files Browse the repository at this point in the history
  • Loading branch information
@glrf
glrf committed Jan 12, 2022
1 parent 7dfe5d7 commit c802232
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 41 deletions.
28 changes: 15 additions & 13 deletions apiserver/organization/authorizer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ func (a rbacAuthorizer) Authorize(ctx context.Context, attr authorizer.Attribute
User: attr.GetUser(),
Verb: attr.GetVerb(),
Name: attr.GetName(),
Namespace: attr.GetName(),
Namespace: attr.GetName(), // We handle cluster wide resources
APIGroup: rbacGV.Group,
APIVersion: rbacGV.Version,
Resource: attr.GetResource(),
Subresource: attr.GetSubresource(),
ResourceRequest: attr.IsResourceRequest(),
ResourceRequest: true, // Always a resource request
Path: attr.GetPath(),
})

Expand All @@ -59,21 +59,23 @@ func (a rbacAuthorizer) AuthorizeContext(ctx context.Context) error {
return a.Authorize(ctx, attr)
}

func (a rbacAuthorizer) AuthorizeVerb(ctx context.Context, verb string) error {
func (a rbacAuthorizer) AuthorizeVerb(ctx context.Context, verb string, name string) error {
attr, err := filters.GetAuthorizerAttributes(ctx)
if err != nil {
return err
}
return a.Authorize(ctx, authorizer.AttributesRecord{
User: attr.GetUser(),
Verb: verb,
Name: attr.GetName(),
Namespace: attr.GetName(),
APIGroup: attr.GetAPIGroup(),
APIVersion: attr.GetAPIVersion(),
Resource: attr.GetResource(),
Subresource: attr.GetSubresource(),
ResourceRequest: attr.IsResourceRequest(),
Path: attr.GetPath(),
User: attr.GetUser(),
Verb: verb,
Name: name,
Namespace: attr.GetNamespace(),
APIGroup: attr.GetAPIGroup(),
APIVersion: attr.GetAPIVersion(),
Resource: attr.GetResource(),
Path: attr.GetPath(),
})
}

func (a rbacAuthorizer) AuthorizeGet(ctx context.Context, name string) error {
return a.AuthorizeVerb(ctx, "get", name)
}
2 changes: 1 addition & 1 deletion apiserver/organization/organization.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *organizationStorage) NamespaceScoped() bool {
var _ rest.Getter = &organizationStorage{}

func (s *organizationStorage) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
err := s.authorizer.AuthorizeVerb(ctx, "get")
err := s.authorizer.AuthorizeGet(ctx, name)
if err != nil {
return nil, err
}
Expand Down
21 changes: 18 additions & 3 deletions apiserver/organization/organization_list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,25 @@ func (s organizationStorage) NewList() runtime.Object {
}

func (s *organizationStorage) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) {
err := s.authorizer.AuthorizeContext(ctx)
if err != nil {
return nil, err
}
namespaces, err := s.namepaces.ListNamespaces(ctx, addOrganizationLabelSelector(options))
if err != nil {
return nil, convertNamespaceError(err)
}

res := orgv1.OrganizationList{
ListMeta: namespaces.ListMeta,
Items: make([]orgv1.Organization, len(namespaces.Items)),
}

for i := range namespaces.Items {
res.Items[i] = *orgv1.NewOrganizationFromNS(&namespaces.Items[i])
for _, ns := range namespaces.Items {
err := s.authorizer.AuthorizeGet(ctx, ns.Name)
if err != nil {
continue
}
res.Items = append(res.Items, *orgv1.NewOrganizationFromNS(&ns))
}

return &res, nil
Expand All @@ -41,6 +48,10 @@ func (s *organizationStorage) List(ctx context.Context, options *metainternalver
var _ rest.Watcher = &organizationStorage{}

func (s *organizationStorage) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) {
err := s.authorizer.AuthorizeContext(ctx)
if err != nil {
return nil, err
}

nsWatcher, err := s.namepaces.WatchNamespaces(ctx, addOrganizationLabelSelector(options))
if err != nil {
Expand All @@ -58,6 +69,10 @@ func (s *organizationStorage) Watch(ctx context.Context, options *metainternalve
// This is most likely an error so we pass it on
return in, true
}
err := s.authorizer.AuthorizeGet(ctx, ns.Name)
if err != nil {
return in, false
}

in.Object = orgv1.NewOrganizationFromNS(ns)

Expand Down
179 changes: 155 additions & 24 deletions apiserver/organization/organization_list_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package organization

import (
"context"
"errors"
"testing"

mock "github.com/appuio/control-api/apiserver/organization/mock"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -15,13 +14,18 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/request"
)

func TestOrganizationStorage_List(t *testing.T) {
tests := map[string]struct {
namespaces *corev1.NamespaceList
namespaceErr error

authList authResponse
authGet []authResponse

organizations *orgv1.OrganizationList
err error
}{
Expand All @@ -32,7 +36,13 @@ func TestOrganizationStorage_List(t *testing.T) {
*barNs,
},
},

authList: authResponse{
decision: authorizer.DecisionAllow,
},
authGet: []authResponse{
{decision: authorizer.DecisionAllow},
{decision: authorizer.DecisionAllow},
},
organizations: &orgv1.OrganizationList{
Items: []orgv1.Organization{
*fooOrg,
Expand All @@ -41,6 +51,9 @@ func TestOrganizationStorage_List(t *testing.T) {
},
},
"GivenErrNotFound_ThenErrNotFound": {
authList: authResponse{
decision: authorizer.DecisionAllow,
},
namespaceErr: apierrors.NewNotFound(schema.GroupResource{
Resource: "namepaces",
}, "not-found"),
Expand All @@ -49,23 +62,65 @@ func TestOrganizationStorage_List(t *testing.T) {
Resource: "organizations",
}, "not-found"),
},
"GivenForbidden_ThenForbidden": {
authList: authResponse{
decision: authorizer.DecisionDeny,
reason: "confidential",
},
err: apierrors.NewForbidden(schema.GroupResource{
Group: orgv1.GroupVersion.Group,
Resource: "organizations",
}, "", errors.New("confidential")),
},
"GivenList_ThenFilter": {
namespaces: &corev1.NamespaceList{
Items: []corev1.Namespace{
*fooNs,
*barNs,
},
},
authList: authResponse{
decision: authorizer.DecisionAllow,
},
authGet: []authResponse{
{decision: authorizer.DecisionAllow},
{decision: authorizer.DecisionDeny},
},
organizations: &orgv1.OrganizationList{
Items: []orgv1.Organization{
*fooOrg,
},
},
},
}

ctrl := gomock.NewController(t)
defer ctrl.Finish()

mnp := mock.NewMocknamespaceProvider(ctrl)
os := organizationStorage{
namepaces: mnp,
}

for n, tc := range tests {
t.Run(n, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
os, mnp, mauth := newMockedOrganizationStorage(ctrl)

mauth.EXPECT().
Authorize(gomock.Any(), isAuthRequest("list")).
Return(tc.authList.decision, tc.authList.reason, tc.authList.err).
Times(1)
mnp.EXPECT().
ListNamespaces(gomock.Any(), gomock.Any()).
Return(tc.namespaces, tc.namespaceErr).
Times(1)
org, err := os.List(context.TODO(), nil)
AnyTimes()

for _, getAuth := range tc.authGet {
mauth.EXPECT().
Authorize(gomock.Any(), isAuthRequest("get")).
Return(getAuth.decision, getAuth.reason, getAuth.err).
Times(1)
}

org, err := os.List(request.WithRequestInfo(request.NewContext(),
&request.RequestInfo{
Verb: "list",
APIGroup: orgv1.GroupVersion.Group,
Resource: "organizations",
}), nil)
if tc.err != nil {
assert.EqualError(t, err, tc.err.Error())
return
Expand All @@ -91,6 +146,9 @@ func TestOrganizationStorage_Watch(t *testing.T) {
namespacesEvents []watch.Event
namespaceErr error

authWatch authResponse
authGet []authResponse

organizationEvents []watch.Event
err error
}{
Expand All @@ -105,6 +163,13 @@ func TestOrganizationStorage_Watch(t *testing.T) {
Object: barNs,
},
},
authWatch: authResponse{
decision: authorizer.DecisionAllow,
},
authGet: []authResponse{
{decision: authorizer.DecisionAllow},
{decision: authorizer.DecisionAllow},
},
organizationEvents: []watch.Event{
{
Type: watch.Added,
Expand All @@ -120,35 +185,101 @@ func TestOrganizationStorage_Watch(t *testing.T) {
namespaceErr: apierrors.NewNotFound(schema.GroupResource{
Resource: "namepaces",
}, "not-found"),
authWatch: authResponse{
decision: authorizer.DecisionAllow,
},
err: apierrors.NewNotFound(schema.GroupResource{
Group: orgv1.GroupVersion.Group,
Resource: "organizations",
}, "not-found"),
},
}

ctrl := gomock.NewController(t)
defer ctrl.Finish()

mnp := mock.NewMocknamespaceProvider(ctrl)
os := organizationStorage{
namepaces: mnp,
"GivenForbidden_ThenForbidden": {
authWatch: authResponse{
decision: authorizer.DecisionDeny,
reason: "confidential",
},
err: apierrors.NewForbidden(schema.GroupResource{
Group: orgv1.GroupVersion.Group,
Resource: "organizations",
}, "", errors.New("confidential")),
},
"GivenNsEvents_ThenFilter": {
namespacesEvents: []watch.Event{
{
Type: watch.Added,
Object: fooNs,
},
{
Type: watch.Modified,
Object: barNs,
},
{
Type: watch.Modified,
Object: fooNs,
},
{
Type: watch.Modified,
Object: barNs,
},
},
authWatch: authResponse{
decision: authorizer.DecisionAllow,
},
authGet: []authResponse{
{decision: authorizer.DecisionAllow},
{decision: authorizer.DecisionDeny},
{decision: authorizer.DecisionAllow},
{decision: authorizer.DecisionDeny},
},
organizationEvents: []watch.Event{
{
Type: watch.Added,
Object: fooOrg,
},
{
Type: watch.Modified,
Object: fooOrg,
},
},
},
}

for n, tc := range tests {
t.Run(n, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
os, mnp, mauth := newMockedOrganizationStorage(ctrl)

nsWatcher := testWatcher{
events: make(chan watch.Event, len(tc.namespacesEvents)),
}
for _, e := range tc.namespacesEvents {
nsWatcher.events <- e
}
close(nsWatcher.events)

mauth.EXPECT().
Authorize(gomock.Any(), isAuthRequest("watch")).
Return(tc.authWatch.decision, tc.authWatch.reason, tc.authWatch.err).
Times(1)

mnp.EXPECT().
WatchNamespaces(gomock.Any(), gomock.Any()).
Return(nsWatcher, tc.namespaceErr).
Times(1)
orgWatch, err := os.Watch(context.TODO(), nil)
AnyTimes()

for _, getAuth := range tc.authGet {
mauth.EXPECT().
Authorize(gomock.Any(), isAuthRequest("get")).
Return(getAuth.decision, getAuth.reason, getAuth.err).
Times(1)
}
orgWatch, err := os.Watch(request.WithRequestInfo(request.NewContext(),
&request.RequestInfo{
Verb: "watch",
APIGroup: orgv1.GroupVersion.Group,
Resource: "organizations",
}), nil)
if tc.err != nil {
assert.EqualError(t, err, tc.err.Error())
return
Expand Down

0 comments on commit c802232

Please sign in to comment.