diff --git a/CHANGELOG.md b/CHANGELOG.md index b8a501f4bc..399333a95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ 1. [#2408](https://github.com/influxdata/chronograf/pull/2408): Fix updated Dashboard names not updating dashboard list 1. [#2416](https://github.com/influxdata/chronograf/pull/2416): Fix default y-axis labels not displaying properly 1. [#2423](https://github.com/influxdata/chronograf/pull/2423): Gracefully scale Template Variables Manager overlay on smaller displays +1. [#2426](https://github.com/influxdata/chronograf/pull/2426): Fix Influx Enterprise users from deletion in race condition ### Features diff --git a/enterprise/meta.go b/enterprise/meta.go index f27012224e..5a07453e76 100644 --- a/enterprise/meta.go +++ b/enterprise/meta.go @@ -23,8 +23,6 @@ type MetaClient struct { client client } -type ClientBuilder func() client - // NewMetaClient represents a meta node in an Influx Enterprise cluster func NewMetaClient(url *url.URL) *MetaClient { return &MetaClient{ @@ -118,39 +116,10 @@ func (m *MetaClient) DeleteUser(ctx context.Context, name string) error { return m.Post(ctx, "/user", a, nil) } -// RemoveAllUserPerms revokes all permissions for a user in Influx Enterprise -func (m *MetaClient) RemoveAllUserPerms(ctx context.Context, name string) error { - user, err := m.User(ctx, name) - if err != nil { - return err - } - - // No permissions to remove - if len(user.Permissions) == 0 { - return nil - } - +// RemoveUserPerms revokes permissions for a user in Influx Enterprise +func (m *MetaClient) RemoveUserPerms(ctx context.Context, name string, perms Permissions) error { a := &UserAction{ Action: "remove-permissions", - User: user, - } - return m.Post(ctx, "/user", a, nil) -} - -// SetUserPerms removes all permissions and then adds the requested perms -func (m *MetaClient) SetUserPerms(ctx context.Context, name string, perms Permissions) error { - err := m.RemoveAllUserPerms(ctx, name) - if err != nil { - return err - } - - // No permissions to add, so, user is in the right state - if len(perms) == 0 { - return nil - } - - a := &UserAction{ - Action: "add-permissions", User: &User{ Name: name, Permissions: perms, @@ -159,6 +128,38 @@ func (m *MetaClient) SetUserPerms(ctx context.Context, name string, perms Permis return m.Post(ctx, "/user", a, nil) } +// SetUserPerms removes permissions not in set and then adds the requested perms +func (m *MetaClient) SetUserPerms(ctx context.Context, name string, perms Permissions) error { + user, err := m.User(ctx, name) + if err != nil { + return err + } + + revoke, add := permissionsDifference(perms, user.Permissions) + + // first, revoke all the permissions the user currently has, but, + // shouldn't... + if len(revoke) > 0 { + err := m.RemoveUserPerms(ctx, name, revoke) + if err != nil { + return err + } + } + + // ... next, add any permissions the user should have + if len(add) > 0 { + a := &UserAction{ + Action: "add-permissions", + User: &User{ + Name: name, + Permissions: add, + }, + } + return m.Post(ctx, "/user", a, nil) + } + return nil +} + // UserRoles returns a map of users to all of their current roles func (m *MetaClient) UserRoles(ctx context.Context) (map[string]Roles, error) { res, err := m.Roles(ctx, nil) @@ -235,39 +236,10 @@ func (m *MetaClient) DeleteRole(ctx context.Context, name string) error { return m.Post(ctx, "/role", a, nil) } -// RemoveAllRolePerms removes all permissions from a role -func (m *MetaClient) RemoveAllRolePerms(ctx context.Context, name string) error { - role, err := m.Role(ctx, name) - if err != nil { - return err - } - - // No permissions to remove - if len(role.Permissions) == 0 { - return nil - } - +// RemoveRolePerms revokes permissions from a role +func (m *MetaClient) RemoveRolePerms(ctx context.Context, name string, perms Permissions) error { a := &RoleAction{ Action: "remove-permissions", - Role: role, - } - return m.Post(ctx, "/role", a, nil) -} - -// SetRolePerms removes all permissions and then adds the requested perms to role -func (m *MetaClient) SetRolePerms(ctx context.Context, name string, perms Permissions) error { - err := m.RemoveAllRolePerms(ctx, name) - if err != nil { - return err - } - - // No permissions to add, so, role is in the right state - if len(perms) == 0 { - return nil - } - - a := &RoleAction{ - Action: "add-permissions", Role: &Role{ Name: name, Permissions: perms, @@ -276,7 +248,39 @@ func (m *MetaClient) SetRolePerms(ctx context.Context, name string, perms Permis return m.Post(ctx, "/role", a, nil) } -// SetRoleUsers removes all users and then adds the requested users to role +// SetRolePerms removes permissions not in set and then adds the requested perms to role +func (m *MetaClient) SetRolePerms(ctx context.Context, name string, perms Permissions) error { + role, err := m.Role(ctx, name) + if err != nil { + return err + } + + revoke, add := permissionsDifference(perms, role.Permissions) + + // first, revoke all the permissions the role currently has, but, + // shouldn't... + if len(revoke) > 0 { + err := m.RemoveRolePerms(ctx, name, revoke) + if err != nil { + return err + } + } + + // ... next, add any permissions the role should have + if len(add) > 0 { + a := &RoleAction{ + Action: "add-permissions", + Role: &Role{ + Name: name, + Permissions: add, + }, + } + return m.Post(ctx, "/role", a, nil) + } + return nil +} + +// SetRoleUsers removes users not in role and then adds the requested users to role func (m *MetaClient) SetRoleUsers(ctx context.Context, name string, users []string) error { role, err := m.Role(ctx, name) if err != nil { @@ -320,6 +324,29 @@ func Difference(wants []string, haves []string) (revoke []string, add []string) return } +func permissionsDifference(wants Permissions, haves Permissions) (revoke Permissions, add Permissions) { + revoke = make(Permissions) + add = make(Permissions) + for scope, want := range wants { + have, ok := haves[scope] + if ok { + r, a := Difference(want, have) + revoke[scope] = r + add[scope] = a + } else { + add[scope] = want + } + } + + for scope, have := range haves { + _, ok := wants[scope] + if !ok { + revoke[scope] = have + } + } + return +} + // AddRoleUsers updates a role to have additional users. func (m *MetaClient) AddRoleUsers(ctx context.Context, name string, users []string) error { // No permissions to add, so, role is in the right state diff --git a/enterprise/meta_test.go b/enterprise/meta_test.go index fd1a223014..774ceea9b6 100644 --- a/enterprise/meta_test.go +++ b/enterprise/meta_test.go @@ -595,7 +595,7 @@ func TestMetaClient_SetUserPerms(t *testing.T) { wantErr bool }{ { - name: "Successful set permissions User", + name: "Remove all permissions for a user", fields: fields{ URL: &url.URL{ Host: "twinpinesmall.net:8091", @@ -615,7 +615,7 @@ func TestMetaClient_SetUserPerms(t *testing.T) { wantRm: `{"action":"remove-permissions","user":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]}}}`, }, { - name: "Successful set permissions User", + name: "Remove some permissions and add others", fields: fields{ URL: &url.URL{ Host: "twinpinesmall.net:8091", @@ -1137,7 +1137,7 @@ func TestMetaClient_SetRolePerms(t *testing.T) { wantErr bool }{ { - name: "Successful set permissions role", + name: "Remove all roles from user", fields: fields{ URL: &url.URL{ Host: "twinpinesmall.net:8091", @@ -1154,10 +1154,10 @@ func TestMetaClient_SetRolePerms(t *testing.T) { ctx: context.Background(), name: "admin", }, - wantRm: `{"action":"remove-permissions","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]},"users":["marty"]}}`, + wantRm: `{"action":"remove-permissions","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]}}}`, }, { - name: "Successful set single permissions role", + name: "Remove some users and add permissions to other", fields: fields{ URL: &url.URL{ Host: "twinpinesmall.net:8091", @@ -1179,7 +1179,7 @@ func TestMetaClient_SetRolePerms(t *testing.T) { }, }, }, - wantRm: `{"action":"remove-permissions","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]},"users":["marty"]}}`, + wantRm: `{"action":"remove-permissions","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]}}}`, wantAdd: `{"action":"add-permissions","role":{"name":"admin","permissions":{"telegraf":["ReadData"]}}}`, }, } @@ -1218,7 +1218,7 @@ func TestMetaClient_SetRolePerms(t *testing.T) { got, _ := ioutil.ReadAll(prm.Body) if string(got) != tt.wantRm { - t.Errorf("%q. MetaClient.SetRolePerms() = %v, want %v", tt.name, string(got), tt.wantRm) + t.Errorf("%q. MetaClient.SetRolePerms() removal = \n%v\n, want \n%v\n", tt.name, string(got), tt.wantRm) } if tt.wantAdd != "" { prm := reqs[2] @@ -1231,7 +1231,7 @@ func TestMetaClient_SetRolePerms(t *testing.T) { got, _ := ioutil.ReadAll(prm.Body) if string(got) != tt.wantAdd { - t.Errorf("%q. MetaClient.SetRolePerms() = %v, want %v", tt.name, string(got), tt.wantAdd) + t.Errorf("%q. MetaClient.SetRolePerms() addition = \n%v\n, want \n%v\n", tt.name, string(got), tt.wantAdd) } } } diff --git a/enterprise/users.go b/enterprise/users.go index 68c04d193c..aa1b9f23ed 100644 --- a/enterprise/users.go +++ b/enterprise/users.go @@ -70,44 +70,49 @@ func (c *UserStore) Update(ctx context.Context, u *chronograf.User) error { return c.Ctrl.ChangePassword(ctx, u.Name, u.Passwd) } - // Make a list of the roles we want this user to have: - want := make([]string, len(u.Roles)) - for i, r := range u.Roles { - want[i] = r.Name - } + if u.Roles != nil { + // Make a list of the roles we want this user to have: + want := make([]string, len(u.Roles)) + for i, r := range u.Roles { + want[i] = r.Name + } - // Find the list of all roles this user is currently in - userRoles, err := c.UserRoles(ctx) - if err != nil { - return nil - } - // Make a list of the roles the user currently has - roles := userRoles[u.Name] - have := make([]string, len(roles.Roles)) - for i, r := range roles.Roles { - have[i] = r.Name - } + // Find the list of all roles this user is currently in + userRoles, err := c.UserRoles(ctx) + if err != nil { + return nil + } + // Make a list of the roles the user currently has + roles := userRoles[u.Name] + have := make([]string, len(roles.Roles)) + for i, r := range roles.Roles { + have[i] = r.Name + } - // Calculate the roles the user will be removed from and the roles the user - // will be added to. - revoke, add := Difference(want, have) + // Calculate the roles the user will be removed from and the roles the user + // will be added to. + revoke, add := Difference(want, have) - // First, add the user to the new roles - for _, role := range add { - if err := c.Ctrl.AddRoleUsers(ctx, role, []string{u.Name}); err != nil { - return err + // First, add the user to the new roles + for _, role := range add { + if err := c.Ctrl.AddRoleUsers(ctx, role, []string{u.Name}); err != nil { + return err + } + } + + // ... and now remove the user from an extra roles + for _, role := range revoke { + if err := c.Ctrl.RemoveRoleUsers(ctx, role, []string{u.Name}); err != nil { + return err + } } } - // ... and now remove the user from an extra roles - for _, role := range revoke { - if err := c.Ctrl.RemoveRoleUsers(ctx, role, []string{u.Name}); err != nil { - return err - } + if u.Permissions != nil { + perms := ToEnterprise(u.Permissions) + return c.Ctrl.SetUserPerms(ctx, u.Name, perms) } - - perms := ToEnterprise(u.Permissions) - return c.Ctrl.SetUserPerms(ctx, u.Name, perms) + return nil } // All is all users in influx diff --git a/server/swagger.json b/server/swagger.json index 0580955f6f..b8a1c34e75 100644 --- a/server/swagger.json +++ b/server/swagger.json @@ -550,6 +550,7 @@ "patch": { "tags": ["sources", "users"], "summary": "Update user configuration", + "description": "Update one parameter at a time (one of password, permissions or roles)", "parameters": [ { "name": "id",