From 9e043b34fd0d34d2705faffc09487bf07b9e0a84 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 10 May 2019 11:33:32 +0200 Subject: [PATCH] Resource/operation combo whitelist (refactored validation) --- compose/internal/service/access_control.go | 60 +++++++++- internal/permissions/permissions.go | 24 ++++ internal/permissions/service.go | 9 +- messaging/internal/service/access_control.go | 46 +++++++- messaging/internal/service/main_test.go | 6 +- system/internal/service/access_control.go | 51 ++++++++- system/internal/service/validation.go | 112 ------------------- system/internal/service/validation_test.go | 17 --- 8 files changed, 186 insertions(+), 139 deletions(-) delete mode 100644 system/internal/service/validation.go delete mode 100644 system/internal/service/validation_test.go diff --git a/compose/internal/service/access_control.go b/compose/internal/service/access_control.go index e0c6e3671..ac2e4084e 100644 --- a/compose/internal/service/access_control.go +++ b/compose/internal/service/access_control.go @@ -14,7 +14,7 @@ type ( accessControlPermissionServicer interface { Can(context.Context, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool - Grant(context.Context, ...*permissions.Rule) error + Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error } permissionResource interface { @@ -149,7 +149,7 @@ func (svc accessControl) can(ctx context.Context, res permissionResource, op per } func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { - return svc.permissions.Grant(ctx, rr...) + return svc.permissions.Grant(ctx, svc.Whitelist(), rr...) } // DefaultRules returns list of default rules for this compose service @@ -201,3 +201,59 @@ func (svc accessControl) DefaultRules() permissions.RuleSet { allowAdm(pages, "delete"), } } + +func (svc accessControl) Whitelist() permissions.Whitelist { + var wl = permissions.Whitelist{} + + wl.Set( + types.ComposePermissionResource, + "access", + "grant", + "namespace.create", + ) + + wl.Set( + types.NamespacePermissionResource, + "read", + "update", + "delete", + "module.create", + "chart.create", + "trigger.create", + "page.create", + ) + + wl.Set( + types.ModulePermissionResource, + "read", + "update", + "delete", + "record.create", + "record.read", + "record.update", + "record.delete", + ) + + wl.Set( + types.ChartPermissionResource, + "read", + "update", + "delete", + ) + + wl.Set( + types.TriggerPermissionResource, + "read", + "update", + "delete", + ) + + wl.Set( + types.PagePermissionResource, + "read", + "update", + "delete", + ) + + return wl +} diff --git a/internal/permissions/permissions.go b/internal/permissions/permissions.go index c4efa0e9e..043892b63 100644 --- a/internal/permissions/permissions.go +++ b/internal/permissions/permissions.go @@ -8,6 +8,8 @@ type ( // CheckAccessFunc function. CheckAccessFunc func() Access + + Whitelist map[Resource]map[Operation]bool ) const ( @@ -52,3 +54,25 @@ func Allowed() Access { func Denied() Access { return Deny } + +func (wl *Whitelist) Set(r Resource, oo ...Operation) { + (*wl)[r] = map[Operation]bool{} + + for _, o := range oo { + (*wl)[r][o] = true + } +} + +func (wl Whitelist) Check(rule *Rule) bool { + if rule == nil { + return false + } + + res := rule.Resource.TrimID() + + if _, ok := wl[res]; !ok { + return false + } + + return wl[res][rule.Operation] +} diff --git a/internal/permissions/service.go b/internal/permissions/service.go index b39c41b60..31d20fe47 100644 --- a/internal/permissions/service.go +++ b/internal/permissions/service.go @@ -5,6 +5,7 @@ import ( "sync" "time" + "github.com/pkg/errors" "go.uber.org/zap" ) @@ -83,10 +84,16 @@ func (svc service) Check(res Resource, op Operation, roles ...uint64) (v Access) // Grant appends and/or overwrites internal rules slice // // All rules with Inherit are removed -func (svc *service) Grant(ctx context.Context, rules ...*Rule) (err error) { +func (svc *service) Grant(ctx context.Context, wl Whitelist, rules ...*Rule) (err error) { svc.l.Lock() defer svc.l.Unlock() + for _, r := range rules { + if !wl.Check(r) { + return errors.Errorf("invalid rule: '%s' on '%s'", r.Operation, r.Resource) + } + } + if svc.rules, err = svc.rules.merge(rules...); err != nil { return } diff --git a/messaging/internal/service/access_control.go b/messaging/internal/service/access_control.go index 7abdfe0b9..1042a73e2 100644 --- a/messaging/internal/service/access_control.go +++ b/messaging/internal/service/access_control.go @@ -15,7 +15,7 @@ type ( accessControlPermissionServicer interface { Can(context.Context, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool - Grant(context.Context, ...*permissions.Rule) error + Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error } permissionResource interface { @@ -209,7 +209,7 @@ func (svc accessControl) can(ctx context.Context, res permissionResource, op per } func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { - return svc.permissions.Grant(ctx, rr...) + return svc.permissions.Grant(ctx, svc.Whitelist(), rr...) } // DefaultRules returns list of default rules for this compose service @@ -261,3 +261,45 @@ func (svc accessControl) DefaultRules() permissions.RuleSet { allowAdm(channels, "message.react"), } } + +func (svc accessControl) Whitelist() permissions.Whitelist { + var wl = permissions.Whitelist{} + + wl.Set( + types.MessagingPermissionResource, + "access", + "grant", + "channel.public.create", + "channel.private.create", + "channel.group.create", + "webhook.create", + "webhook.manage.all", + "webhook.manage.own", + ) + + wl.Set( + types.ChannelPermissionResource, + "update", + "read", + "join", + "leave", + "delete", + "undelete", + "archive", + "unarchive", + "members.manage", + "webhooks.manage", + "attachments.manage", + "message.send", + "message.reply", + "message.embed", + "message.attach", + "message.update.own", + "message.update.all", + "message.delete.own", + "message.delete.all", + "message.react", + ) + + return wl +} diff --git a/messaging/internal/service/main_test.go b/messaging/internal/service/main_test.go index 0e7a926b1..2274a87f0 100644 --- a/messaging/internal/service/main_test.go +++ b/messaging/internal/service/main_test.go @@ -3,11 +3,11 @@ package service import ( + "context" "fmt" "os" "testing" - "github.com/SentimensRG/ctx" "github.com/namsral/flag" "github.com/titpetric/factory" "go.uber.org/zap/zapcore" @@ -63,8 +63,8 @@ func TestMain(m *testing.M) { } } - systemService.Init(ctx.Background()) - Init(ctx.Background()) + systemService.Init(context.Background()) + Init(context.Background()) os.Exit(m.Run()) } diff --git a/system/internal/service/access_control.go b/system/internal/service/access_control.go index 3f6b6da5e..6d12c11d4 100644 --- a/system/internal/service/access_control.go +++ b/system/internal/service/access_control.go @@ -14,7 +14,7 @@ type ( accessControlPermissionServicer interface { Can(context.Context, permissions.Resource, permissions.Operation, ...permissions.CheckAccessFunc) bool - Grant(context.Context, ...*permissions.Rule) error + Grant(context.Context, permissions.Whitelist, ...*permissions.Rule) error } permissionResource interface { @@ -120,7 +120,7 @@ func (svc accessControl) can(ctx context.Context, res permissionResource, op per } func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) error { - return svc.permissions.Grant(ctx, rr...) + return svc.permissions.Grant(ctx, svc.Whitelist(), rr...) } // DefaultRules returns list of default rules for this compose service @@ -167,3 +167,50 @@ func (svc accessControl) DefaultRules() permissions.RuleSet { allowAdm(roles, "members.manage"), } } + +func (svc accessControl) Whitelist() permissions.Whitelist { + var wl = permissions.Whitelist{} + + wl.Set( + types.SystemPermissionResource, + "access", + "grant", + "settings.read", + "settings.manage", + "organisation.create", + "role.create", + "user.create", + "application.create", + ) + + wl.Set( + types.OrganisationPermissionResource, + "access", + ) + + wl.Set( + types.ApplicationPermissionResource, + "read", + "update", + "delete", + ) + + wl.Set( + types.UserPermissionResource, + "read", + "update", + "delete", + "suspend", + "unsuspend", + ) + + wl.Set( + types.RolePermissionResource, + "read", + "update", + "delete", + "members.manage", + ) + + return wl +} diff --git a/system/internal/service/validation.go b/system/internal/service/validation.go deleted file mode 100644 index 913f1b0cc..000000000 --- a/system/internal/service/validation.go +++ /dev/null @@ -1,112 +0,0 @@ -package service - -var ( - permissionList = map[string]map[string]bool{ - "system": map[string]bool{ - "access": true, - "grant": true, - "settings.read": true, - "settings.manage": true, - "organisation.create": true, - "role.create": true, - "application.create": true, - }, - "system:organisation:": map[string]bool{ - "access": true, - }, - "system:role:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - "members.manage": true, - }, - "system:application:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - }, - "messaging": map[string]bool{ - "access": true, - "grant": true, - "channel.public.create": true, - "channel.private.create": true, - "channel.group.create": true, - }, - "messaging:channel:": map[string]bool{ - "update": true, - "read": true, - "join": true, - "leave": true, - "delete": true, - "undelete": true, - "archive": true, - "unarchive": true, - "members.manage": true, - "webhooks.manage": true, - "attachments.manage": true, - "message.send": true, - "message.reply": true, - "message.embed": true, - "message.attach": true, - "message.update.own": true, - "message.update.all": true, - "message.delete.own": true, - "message.delete.all": true, - "message.react": true, - }, - "compose": map[string]bool{ - "access": true, - "grant": true, - "namespace.create": true, - }, - "compose:namespace:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - "module.create": true, - "chart.create": true, - "trigger.create": true, - "page.create": true, - }, - "compose:module:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - "record.create": true, - "record.read": true, - "record.update": true, - "record.delete": true, - }, - "compose:chart:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - }, - "compose:trigger:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - }, - "compose:page:": map[string]bool{ - "read": true, - "update": true, - "delete": true, - }, - } -) - -// func validatePermission(resource internalpermissions.Resource, operation string) error { -// if !resource.IsValid() { -// return errors.Errorf("invalid resource format: %q", resource) -// } -// -// res := resource.TrimID().String() -// -// if service, ok := permissionList[res]; ok { -// if op := service[operation]; op { -// return nil -// } -// return errors.Errorf("Unknown operation: '%s'", operation) -// } -// return errors.Errorf("Unknown resource name: '%s'", resource) -// } diff --git a/system/internal/service/validation_test.go b/system/internal/service/validation_test.go deleted file mode 100644 index 6cc119bda..000000000 --- a/system/internal/service/validation_test.go +++ /dev/null @@ -1,17 +0,0 @@ -// +build unit - -package service - -import ( - "testing" - - "github.com/crusttech/crust/internal/test" -) - -func TestPermissionsValidation(t *testing.T) { - test.Error(t, validatePermission("bogus", "bogus"), "expected error") - test.Error(t, validatePermission("bogus", "bogus"), "expected error") - test.Error(t, validatePermission("messaging:channel", "bogus"), "expected error") - test.Error(t, validatePermission("messaging:channel:", "message.send"), "expected error") - test.NoError(t, validatePermission("messaging:channel:1", "message.send"), "expected valid response") -}