From 271aebb40f5d434deb4d3e1e9b0aea0e5d94b3a8 Mon Sep 17 00:00:00 2001 From: Michael Desa Date: Fri, 3 Nov 2017 12:06:18 -0400 Subject: [PATCH] Prevent duplicate organization roles in user CRUD --- server/users.go | 5 ++ server/users_test.go | 208 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+) diff --git a/server/users.go b/server/users.go index 08e3158e8..b92c6a77d 100644 --- a/server/users.go +++ b/server/users.go @@ -55,8 +55,13 @@ func (r *userRequest) ValidUpdate() error { } func (r *userRequest) ValidRoles() error { + orgs := map[string]bool{} if len(r.Roles) > 0 { for _, r := range r.Roles { + if _, ok := orgs[r.Organization]; ok { + return fmt.Errorf("duplicate organization %q in roles", r.Organization) + } + orgs[r.Organization] = true switch r.Name { case MemberRoleName, ViewerRoleName, EditorRoleName, AdminRoleName: continue diff --git a/server/users_test.go b/server/users_test.go index 2007cb062..23a562678 100644 --- a/server/users_test.go +++ b/server/users_test.go @@ -160,6 +160,110 @@ func TestService_NewUser(t *testing.T) { wantContentType: "application/json", wantBody: `{"id":"1338","superAdmin":false,"name":"bob","provider":"github","scheme":"oauth2","roles":[],"links":{"self":"/chronograf/v1/users/1338"}}`, }, + { + name: "Create a new Chronograf User with multiple roles", + args: args{ + w: httptest.NewRecorder(), + r: httptest.NewRequest( + "POST", + "http://any.url", + nil, + ), + user: &userRequest{ + Name: "bob", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "billieta org", + }, + }, + }, + }, + fields: fields{ + Logger: log.New(log.DebugLevel), + UsersStore: &mocks.UsersStore{ + AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) { + return &chronograf.User{ + ID: 1338, + Name: "bob", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "billieta org", + }, + }, + }, nil + }, + }, + }, + wantStatus: http.StatusCreated, + wantContentType: "application/json", + wantBody: `{"id":"1338","superAdmin":false,"name":"bob","provider":"github","scheme":"oauth2","roles":[{"name":"admin","organization":"bobbetta org"},{"name":"viewer","organization":"billieta org"}],"links":{"self":"/chronograf/v1/users/1338"}}`, + }, + { + name: "Create a new Chronograf User with multiple roles same org", + args: args{ + w: httptest.NewRecorder(), + r: httptest.NewRequest( + "POST", + "http://any.url", + nil, + ), + user: &userRequest{ + Name: "bob", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "bobbetta org", + }, + }, + }, + }, + fields: fields{ + Logger: log.New(log.DebugLevel), + UsersStore: &mocks.UsersStore{ + AddF: func(ctx context.Context, user *chronograf.User) (*chronograf.User, error) { + return &chronograf.User{ + ID: 1338, + Name: "bob", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "bobbetta org", + }, + }, + }, nil + }, + }, + }, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: "application/json", + wantBody: `{"code":422,"message":"duplicate organization \"bobbetta org\" in roles"}`, + }, } for _, tt := range tests { @@ -346,6 +450,110 @@ func TestService_UpdateUser(t *testing.T) { wantContentType: "application/json", wantBody: `{"id":"1336","superAdmin":false,"name":"bobbetta","provider":"github","scheme":"oauth2","links":{"self":"/chronograf/v1/users/1336"},"roles":[{"name":"admin"}]}`, }, + { + name: "Update a Chronograf user roles different orgs", + fields: fields{ + Logger: log.New(log.DebugLevel), + UsersStore: &mocks.UsersStore{ + UpdateF: func(ctx context.Context, user *chronograf.User) error { + return nil + }, + GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) { + switch *q.ID { + case 1336: + return &chronograf.User{ + ID: 1336, + Name: "bobbetta", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + EditorRole, + }, + }, nil + default: + return nil, fmt.Errorf("User with ID %d not found", *q.ID) + } + }, + }, + }, + args: args{ + w: httptest.NewRecorder(), + r: httptest.NewRequest( + "PATCH", + "http://any.url", + nil, + ), + user: &userRequest{ + ID: 1336, + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "billieta org", + }, + }, + }, + }, + id: "1336", + wantStatus: http.StatusOK, + wantContentType: "application/json", + wantBody: `{"id":"1336","superAdmin":false,"name":"bobbetta","provider":"github","scheme":"oauth2","links":{"self":"/chronograf/v1/users/1336"},"roles":[{"name":"admin","organization":"bobbetta org"},{"name":"viewer","organization":"billieta org"}]}`, + }, + { + name: "Update a Chronograf user roles same org", + fields: fields{ + Logger: log.New(log.DebugLevel), + UsersStore: &mocks.UsersStore{ + UpdateF: func(ctx context.Context, user *chronograf.User) error { + return nil + }, + GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) { + switch *q.ID { + case 1336: + return &chronograf.User{ + ID: 1336, + Name: "bobbetta", + Provider: "github", + Scheme: "oauth2", + Roles: []chronograf.Role{ + EditorRole, + }, + }, nil + default: + return nil, fmt.Errorf("User with ID %d not found", *q.ID) + } + }, + }, + }, + args: args{ + w: httptest.NewRecorder(), + r: httptest.NewRequest( + "PATCH", + "http://any.url", + nil, + ), + user: &userRequest{ + ID: 1336, + Roles: []chronograf.Role{ + { + Name: AdminRoleName, + Organization: "bobbetta org", + }, + { + Name: ViewerRoleName, + Organization: "bobbetta org", + }, + }, + }, + }, + id: "1336", + wantStatus: http.StatusUnprocessableEntity, + wantContentType: "application/json", + wantBody: `{"code":422,"message":"duplicate organization \"bobbetta org\" in roles"}`, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {