From 6f0d55d19e4c333172c91803fabe36218c05fe9d Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Wed, 9 Jan 2019 10:35:21 -0500 Subject: [PATCH 1/5] feat(platform): add constructor for org member and admin permssions --- authz.go | 42 +++++++++++++++++++++++++++++++++++----- user_resource_mapping.go | 9 +++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/authz.go b/authz.go index 21a6f777101..4360bbece3a 100644 --- a/authz.go +++ b/authz.go @@ -99,7 +99,17 @@ var AllResources = []Resource{ UsersResource, // 7 } -// Valid checks if the resource is a member of the Resource enum +// OrgResources is the list of all known resource types that belong to an organization. +var OrgResources = []Resource{ + BucketsResource, // 1 + DashboardsResource, // 2 + SourcesResource, // 4 + TasksResource, // 5 + TelegrafsResource, // 6 + UsersResource, // 7 +} + +// Valid checks if the resource is a member of the Resource enum. func (r Resource) Valid() (err error) { switch r { case AuthorizationsResource: // 0 @@ -133,7 +143,7 @@ func (p Permission) String() string { return str } -// Valid checks if there the resource and action provided is known +// Valid checks if there the resource and action provided is known. func (p *Permission) Valid() error { if err := p.Resource.Valid(); err != nil { return &Error{ @@ -162,7 +172,7 @@ func (p *Permission) Valid() error { return nil } -// NewPermission returns a permission with provided arguments +// NewPermission returns a permission with provided arguments. func NewPermission(a Action, r Resource) (*Permission, error) { p := &Permission{ Action: a, @@ -172,7 +182,7 @@ func NewPermission(a Action, r Resource) (*Permission, error) { return p, p.Valid() } -// NewPermissionAtID creates a permission with the provided arguments +// NewPermissionAtID creates a permission with the provided arguments. func NewPermissionAtID(id ID, a Action, r Resource) (*Permission, error) { p := &Permission{ Action: a, @@ -183,7 +193,7 @@ func NewPermissionAtID(id ID, a Action, r Resource) (*Permission, error) { return p, p.Valid() } -// OperPermissions are the default permissions for those who setup the application +// OperPermissions are the default permissions for those who setup the application. func OperPermissions() []Permission { ps := []Permission{} for _, r := range AllResources { @@ -194,3 +204,25 @@ func OperPermissions() []Permission { return ps } + +// OrgAdminPermissions are the default permissions for org admins. +func OrgAdminPermissions(orgID ID) []Permission { + ps := []Permission{} + for _, r := range OrgResources { + for _, a := range actions { + ps = append(ps, Permission{ID: &orgID, Action: a, Resource: r}) + } + } + + return ps +} + +// OrgMemberPermissions are the default permissions for org members. +func OrgMemberPermissions(orgID ID) []Permission { + ps := []Permission{} + for _, r := range OrgResources { + ps = append(ps, Permission{ID: &orgID, Action: ReadAction, Resource: r}) + } + + return ps +} diff --git a/user_resource_mapping.go b/user_resource_mapping.go index 2c931905c06..42d4679b3b9 100644 --- a/user_resource_mapping.go +++ b/user_resource_mapping.go @@ -97,6 +97,11 @@ func (m *UserResourceMapping) ownerPerms() ([]Permission, error) { } ps = append(ps, *p) + + if m.Resource == OrgsResource { + ps = append(ps, OrgAdminPermissions(m.ResourceID)...) + } + } return ps, nil @@ -111,6 +116,10 @@ func (m *UserResourceMapping) memberPerms() ([]Permission, error) { } ps = append(ps, *p) + + if m.Resource == OrgsResource { + ps = append(ps, OrgMemberPermissions(m.ResourceID)...) + } } return ps, nil From ead4a0a170b735e167a9b1e1eb94a3198f4d8b2f Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Wed, 9 Jan 2019 10:35:50 -0500 Subject: [PATCH 2/5] fix(bolt): grand first user org admin privileges during onboarding --- bolt/onboarding.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bolt/onboarding.go b/bolt/onboarding.go index 13016e028dc..737e0c88e94 100644 --- a/bolt/onboarding.go +++ b/bolt/onboarding.go @@ -117,11 +117,15 @@ func (c *Client) Generate(ctx context.Context, req *platform.OnboardingRequest) if err = c.CreateBucket(ctx, bucket); err != nil { return nil, err } + + perms := platform.OperPermissions() + perms = append(perms, platform.OrgAdminPermissions(o.ID)...) + auth := &platform.Authorization{ UserID: u.ID, Description: fmt.Sprintf("%s's Token", u.Name), OrgID: o.ID, - Permissions: platform.OperPermissions(), + Permissions: perms, } if err = c.CreateAuthorization(ctx, auth); err != nil { return nil, err From ad75185629a288109b61c93cf4c20f05105354b2 Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Wed, 9 Jan 2019 10:36:25 -0500 Subject: [PATCH 3/5] fix(task): update the permission checked in CreateTask CreateTasks now check that the user has the write permission to the tasks resource belonging to an organization. This change comes after https://github.com/influxdata/platform/pull/2157 modified the structure of authorization. --- task/validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/task/validator.go b/task/validator.go index 2c063d6f5c7..9b9560ee969 100644 --- a/task/validator.go +++ b/task/validator.go @@ -37,7 +37,7 @@ func NewValidator(ts platform.TaskService, bs platform.BucketService) platform.T } func (ts *taskServiceValidator) CreateTask(ctx context.Context, t *platform.Task) error { - p, err := platform.NewPermissionAtID(t.ID, platform.WriteAction, platform.TasksResource) + p, err := platform.NewPermissionAtID(t.Organization, platform.WriteAction, platform.TasksResource) if err != nil { return err } From bef148ffd2077f930051eafa319acd51510b86cd Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Wed, 9 Jan 2019 11:16:02 -0500 Subject: [PATCH 4/5] fix(platform): check for matching ids when permission matching --- auth.go | 2 +- authz.go | 20 +++++++- authz_test.go | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++ session.go | 2 +- 4 files changed, 153 insertions(+), 4 deletions(-) diff --git a/auth.go b/auth.go index 6c9c76c57ed..94852dd7784 100644 --- a/auth.go +++ b/auth.go @@ -30,7 +30,7 @@ func (a *Authorization) Allowed(p Permission) bool { return false } - return allowed(p, a.Permissions) + return PermissionAllowed(p, a.Permissions) } // IsActive is a stub for idpe. diff --git a/authz.go b/authz.go index 4360bbece3a..9aba1444b4d 100644 --- a/authz.go +++ b/authz.go @@ -29,9 +29,25 @@ type Authorizer interface { Kind() string } -func allowed(p Permission, ps []Permission) bool { +// PermissionAllowed +func PermissionAllowed(p Permission, ps []Permission) bool { + pID := ID(0) + if p.ID != nil { + pID = *p.ID + if !pID.Valid() { + return false + } + } + for _, perm := range ps { - if perm.Action == p.Action && perm.Resource == p.Resource { + permID := ID(0) + if perm.ID != nil { + permID = *perm.ID + if !permID.Valid() { + return false + } + } + if perm.Action == p.Action && perm.Resource == p.Resource && permID == pID { return true } } diff --git a/authz_test.go b/authz_test.go index 3beea5beeee..9f324bd3e42 100644 --- a/authz_test.go +++ b/authz_test.go @@ -6,6 +6,135 @@ import ( "github.com/influxdata/platform" ) +func TestAuthorizer_PermissionAllowed(t *testing.T) { + tests := []struct { + name string + permission platform.Permission + permissions []platform.Permission + allowed bool + }{ + { + name: "bad resource id in permission", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(0), + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + }, + allowed: false, + }, + { + name: "bad resource id in permissions", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(0), + }, + }, + allowed: false, + }, + { + name: "matching action resource and ID", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + }, + allowed: true, + }, + { + name: "matching action resource no ID", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.BucketsResource, + }, + }, + allowed: true, + }, + { + name: "matching action resource differing ID", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(2), + }, + }, + allowed: false, + }, + { + name: "differing action same resource", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + permissions: []platform.Permission{ + { + Action: platform.ReadAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + }, + allowed: false, + }, + { + name: "same action differing resource", + permission: platform.Permission{ + Action: platform.WriteAction, + Resource: platform.BucketsResource, + ID: IDPtr(1), + }, + permissions: []platform.Permission{ + { + Action: platform.WriteAction, + Resource: platform.TasksResource, + ID: IDPtr(1), + }, + }, + allowed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allowed := platform.PermissionAllowed(tt.permission, tt.permissions) + if allowed != tt.allowed { + t.Errorf("got allowed = %v, expected allowed = %v", allowed, tt.allowed) + } + }) + } +} + func TestPermission_Valid(t *testing.T) { type fields struct { Action platform.Action @@ -159,3 +288,7 @@ func validID() *platform.ID { id := platform.ID(100) return &id } + +func IDPtr(id platform.ID) *platform.ID { + return &id +} diff --git a/session.go b/session.go index 99d7a00db46..f7bc123571a 100644 --- a/session.go +++ b/session.go @@ -48,7 +48,7 @@ func (s *Session) Allowed(p Permission) bool { return false } - return allowed(p, s.Permissions) + return PermissionAllowed(p, s.Permissions) } // Kind returns session and is used for auditing. From 0ec22953dfab0ee0271144318251ee34e8e6623c Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Wed, 9 Jan 2019 11:43:58 -0500 Subject: [PATCH 5/5] fix(testing:bolt:inmem): add org admin permissions during onboarding generate --- bolt/onboarding.go | 9 +++++++++ bolt/onboarding_test.go | 2 +- inmem/onboarding.go | 15 ++++++++++++++- testing/onboarding.go | 18 +++++++++++++++++- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/bolt/onboarding.go b/bolt/onboarding.go index 737e0c88e94..cb5d2e0d8bd 100644 --- a/bolt/onboarding.go +++ b/bolt/onboarding.go @@ -120,6 +120,15 @@ func (c *Client) Generate(ctx context.Context, req *platform.OnboardingRequest) perms := platform.OperPermissions() perms = append(perms, platform.OrgAdminPermissions(o.ID)...) + writeBucketPerm, err := platform.NewPermissionAtID(bucket.ID, platform.WriteAction, platform.BucketsResource) + if err != nil { + return nil, err + } + readBucketPerm, err := platform.NewPermissionAtID(bucket.ID, platform.ReadAction, platform.BucketsResource) + if err != nil { + return nil, err + } + perms = append(perms, *writeBucketPerm, *readBucketPerm) auth := &platform.Authorization{ UserID: u.ID, diff --git a/bolt/onboarding_test.go b/bolt/onboarding_test.go index 9bcc21a9b08..0142c9a6b13 100644 --- a/bolt/onboarding_test.go +++ b/bolt/onboarding_test.go @@ -28,6 +28,6 @@ func initOnboardingService(f platformtesting.OnboardingFields, t *testing.T) (pl } } -func TestGenerate(t *testing.T) { +func TestOnboardingService_Generate(t *testing.T) { platformtesting.Generate(initOnboardingService, t) } diff --git a/inmem/onboarding.go b/inmem/onboarding.go index aebcea745ff..c174f57e36a 100644 --- a/inmem/onboarding.go +++ b/inmem/onboarding.go @@ -92,11 +92,24 @@ func (s *Service) Generate(ctx context.Context, req *platform.OnboardingRequest) if err = s.CreateBucket(ctx, bucket); err != nil { return nil, err } + + perms := platform.OperPermissions() + perms = append(perms, platform.OrgAdminPermissions(o.ID)...) + writeBucketPerm, err := platform.NewPermissionAtID(bucket.ID, platform.WriteAction, platform.BucketsResource) + if err != nil { + return nil, err + } + readBucketPerm, err := platform.NewPermissionAtID(bucket.ID, platform.ReadAction, platform.BucketsResource) + if err != nil { + return nil, err + } + perms = append(perms, *writeBucketPerm, *readBucketPerm) + auth := &platform.Authorization{ UserID: u.ID, Description: fmt.Sprintf("%s's Token", u.Name), OrgID: o.ID, - Permissions: platform.OperPermissions(), + Permissions: perms, } if err = s.CreateAuthorization(ctx, auth); err != nil { return nil, err diff --git a/testing/onboarding.go b/testing/onboarding.go index fa6942fb551..a9f0cc802fb 100644 --- a/testing/onboarding.go +++ b/testing/onboarding.go @@ -170,7 +170,7 @@ func Generate( UserID: MustIDBase16(oneID), Description: "admin's Token", OrgID: MustIDBase16(twoID), - Permissions: platform.OperPermissions(), + Permissions: mustGeneratePermissions(MustIDBase16(twoID), MustIDBase16(threeID)), }, }, }, @@ -203,6 +203,22 @@ func Generate( } +func mustGeneratePermissions(orgID, bucketID platform.ID) []platform.Permission { + perms := platform.OperPermissions() + perms = append(perms, platform.OrgAdminPermissions(orgID)...) + writeBucketPerm, err := platform.NewPermissionAtID(bucketID, platform.WriteAction, platform.BucketsResource) + if err != nil { + panic(err) + } + readBucketPerm, err := platform.NewPermissionAtID(bucketID, platform.ReadAction, platform.BucketsResource) + if err != nil { + panic(err) + } + perms = append(perms, *writeBucketPerm, *readBucketPerm) + + return perms +} + const ( oneID = "020f755c3c082000" twoID = "020f755c3c082001"