Add ability to update roles on a user resource

enterprise/enterprise.go

@@ -32,6 +32,8 @@ type Ctrl interface {
 	DeleteRole(ctx context.Context, name string) error
 	SetRolePerms(ctx context.Context, name string, perms Permissions) error
 	SetRoleUsers(ctx context.Context, name string, users []string) error
+	AddRoleUsers(ctx context.Context, name string, users []string) error
+	RemoveRoleUsers(ctx context.Context, name string, users []string) error
 }
 
 // Client is a device for retrieving time series data from an Influx Enterprise

enterprise/meta.go

@@ -272,32 +272,52 @@ func (m *MetaClient) SetRolePerms(ctx context.Context, name string, perms Permis
 	return m.Post(ctx, "/role", a, nil)
 }
 
-// RemoveAllRoleUsers removes all users from a role
-func (m *MetaClient) RemoveAllRoleUsers(ctx context.Context, name string) error {
+// SetRoleUsers removes all users 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 {
 		return err
 	}
-
-	// No users to remove
-	if len(role.Users) == 0 {
-		return nil
-	}
-
-	a := &RoleAction{
-		Action: "remove-users",
-		Role:   role,
-	}
-	return m.Post(ctx, "/role", a, nil)
-}
-
-// SetRoleUsers removes all users and then adds the requested users to role
-func (m *MetaClient) SetRoleUsers(ctx context.Context, name string, users []string) error {
-	err := m.RemoveAllRoleUsers(ctx, name)
-	if err != nil {
+	revoke, add := Difference(users, role.Users)
+	if err := m.RemoveRoleUsers(ctx, name, revoke); err != nil {
 		return err
 	}
 
+	return m.AddRoleUsers(ctx, name, add)
+}
+
+// Difference compares two sets and returns a set to be removed and a set to be added
+func Difference(wants []string, haves []string) (revoke []string, add []string) {
+	for _, want := range wants {
+		found := false
+		for _, got := range haves {
+			if want != got {
+				continue
+			}
+			found = true
+		}
+		if !found {
+			add = append(add, want)
+		}
+	}
+	for _, got := range haves {
+		found := false
+		for _, want := range wants {
+			if want != got {
+				continue
+			}
+			found = true
+			break
+		}
+		if !found {
+			revoke = append(revoke, got)
+		}
+	}
+	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
 	if len(users) == 0 {
 		return nil
@@ -313,6 +333,23 @@ func (m *MetaClient) SetRoleUsers(ctx context.Context, name string, users []stri
 	return m.Post(ctx, "/role", a, nil)
 }
 
+// RemoveRoleUsers updates a role to remove some users.
+func (m *MetaClient) RemoveRoleUsers(ctx context.Context, name string, users []string) error {
+	// No permissions to add, so, role is in the right state
+	if len(users) == 0 {
+		return nil
+	}
+
+	a := &RoleAction{
+		Action: "remove-users",
+		Role: &Role{
+			Name:  name,
+			Users: users,
+		},
+	}
+	return m.Post(ctx, "/role", a, nil)
+}
+
 // Post is a helper function to POST to Influx Enterprise
 func (m *MetaClient) Post(ctx context.Context, path string, action interface{}, params map[string]string) error {
 	b, err := json.Marshal(action)

enterprise/meta_test.go

@@ -1252,12 +1252,11 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 		name    string
 		fields  fields
 		args    args
-		wantRm  string
-		wantAdd string
+		wants   []string
 		wantErr bool
 	}{
 		{
-			name: "Successful set users role",
+			name: "Successful set users role (remove user from role)",
 			fields: fields{
 				URL: &url.URL{
 					Host:   "twinpinesmall.net:8091",
@@ -1274,7 +1273,7 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 				ctx:  context.Background(),
 				name: "admin",
 			},
-			wantRm: `{"action":"remove-users","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]},"users":["marty"]}}`,
+			wants: []string{`{"action":"remove-users","role":{"name":"admin","users":["marty"]}}`},
 		},
 		{
 			name: "Successful set single user role",
@@ -1285,7 +1284,7 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 				},
 				client: NewMockClient(
 					http.StatusOK,
-					[]byte(`{"roles":[{"name":"admin","users":["marty"],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`),
+					[]byte(`{"roles":[{"name":"admin","users":[],"permissions":{"":["ViewAdmin","ViewChronograf"]}}]}`),
 					nil,
 					nil,
 				),
@@ -1295,8 +1294,9 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 				name:  "admin",
 				users: []string{"marty"},
 			},
-			wantRm:  `{"action":"remove-users","role":{"name":"admin","permissions":{"":["ViewAdmin","ViewChronograf"]},"users":["marty"]}}`,
-			wantAdd: `{"action":"add-users","role":{"name":"admin","users":["marty"]}}`,
+			wants: []string{
+				`{"action":"add-users","role":{"name":"admin","users":["marty"]}}`,
+			},
 		},
 	}
 	for _, tt := range tests {
@@ -1312,8 +1312,8 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 			continue
 		}
 		reqs := tt.fields.client.(*MockClient).Requests
-		if len(reqs) < 2 {
-			t.Errorf("%q. MetaClient.SetRoleUsers() expected 2 but got %d", tt.name, len(reqs))
+		if len(reqs) != len(tt.wants)+1 {
+			t.Errorf("%q. MetaClient.SetRoleUsers() expected %d but got %d", tt.name, len(tt.wants)+1, len(reqs))
 			continue
 		}
 
@@ -1324,21 +1324,8 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 		if usr.URL.Path != "/role" {
 			t.Errorf("%q. MetaClient.SetRoleUsers() expected /user path but got %s", tt.name, usr.URL.Path)
 		}
-
-		prm := reqs[1]
-		if prm.Method != "POST" {
-			t.Errorf("%q. MetaClient.SetRoleUsers() expected GET method", tt.name)
-		}
-		if prm.URL.Path != "/role" {
-			t.Errorf("%q. MetaClient.SetRoleUsers() expected /role path but got %s", tt.name, prm.URL.Path)
-		}
-
-		got, _ := ioutil.ReadAll(prm.Body)
-		if string(got) != tt.wantRm {
-			t.Errorf("%q. MetaClient.SetRoleUsers() = %v, want %v", tt.name, string(got), tt.wantRm)
-		}
-		if tt.wantAdd != "" {
-			prm := reqs[2]
+		for i := range tt.wants {
+			prm := reqs[i+1]
 			if prm.Method != "POST" {
 				t.Errorf("%q. MetaClient.SetRoleUsers() expected GET method", tt.name)
 			}
@@ -1347,8 +1334,8 @@ func TestMetaClient_SetRoleUsers(t *testing.T) {
 			}
 
 			got, _ := ioutil.ReadAll(prm.Body)
-			if string(got) != tt.wantAdd {
-				t.Errorf("%q. MetaClient.SetRoleUsers() = %v, want %v", tt.name, string(got), tt.wantAdd)
+			if string(got) != tt.wants[i] {
+				t.Errorf("%q. MetaClient.SetRoleUsers() = %v, want %v", tt.name, string(got), tt.wants[i])
 			}
 		}
 	}

enterprise/mocks_test.go

@@ -88,6 +88,14 @@ func (cc *ControlClient) SetRoleUsers(ctx context.Context, name string, users []
 	return nil
 }
 
+func (cc *ControlClient) AddRoleUsers(ctx context.Context, name string, users []string) error {
+	return nil
+}
+
+func (cc *ControlClient) RemoveRoleUsers(ctx context.Context, name string, users []string) error {
+	return nil
+}
+
 type TimeSeries struct {
 	URLs     []string
 	Response Response

enterprise/users.go

@@ -21,6 +21,11 @@ func (c *UserStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.Us
 	if err := c.Ctrl.SetUserPerms(ctx, u.Name, perms); err != nil {
 		return nil, err
 	}
+	for _, role := range u.Roles {
+		if err := c.Ctrl.AddRoleUsers(ctx, role.Name, []string{u.Name}); err != nil {
+			return nil, err
+		}
+	}
 
 	return c.Get(ctx, u.Name)
 }
@@ -63,6 +68,43 @@ func (c *UserStore) Update(ctx context.Context, u *chronograf.User) error {
 	if u.Passwd != "" {
 		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
+	}
+
+	// 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)
+
+	// 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
+		}
+	}
+
 	perms := ToEnterprise(u.Permissions)
 	return c.Ctrl.SetUserPerms(ctx, u.Name, perms)
 }

enterprise/users_test.go

@@ -72,6 +72,74 @@ func TestClient_Add(t *testing.T) {
 				Roles: []chronograf.Role{},
 			},
 		},
+		{
+			name: "Successful Create User with roles",
+			fields: fields{
+				Ctrl: &mockCtrl{
+					createUser: func(ctx context.Context, name, passwd string) error {
+						return nil
+					},
+					setUserPerms: func(ctx context.Context, name string, perms enterprise.Permissions) error {
+						return nil
+					},
+					user: func(ctx context.Context, name string) (*enterprise.User, error) {
+						return &enterprise.User{
+							Name:     "marty",
+							Password: "johnny be good",
+							Permissions: map[string][]string{
+								"": {
+									"ViewChronograf",
+									"ReadData",
+									"WriteData",
+								},
+							},
+						}, nil
+					},
+					userRoles: func(ctx context.Context) (map[string]enterprise.Roles, error) {
+						return map[string]enterprise.Roles{
+							"marty": enterprise.Roles{
+								Roles: []enterprise.Role{
+									{
+										Name: "admin",
+									},
+								},
+							},
+						}, nil
+					},
+					addRoleUsers: func(ctx context.Context, name string, users []string) error {
+						return nil
+					},
+				},
+			},
+			args: args{
+				ctx: context.Background(),
+				u: &chronograf.User{
+					Name:   "marty",
+					Passwd: "johnny be good",
+					Roles: []chronograf.Role{
+						{
+							Name: "admin",
+						},
+					},
+				},
+			},
+			want: &chronograf.User{
+				Name: "marty",
+				Permissions: chronograf.Permissions{
+					{
+						Scope:   chronograf.AllScope,
+						Allowed: chronograf.Allowances{"ViewChronograf", "ReadData", "WriteData"},
+					},
+				},
+				Roles: []chronograf.Role{
+					{
+						Name:        "admin",
+						Users:       []chronograf.User{},
+						Permissions: chronograf.Permissions{},
+					},
+				},
+			},
+		},
 		{
 			name: "Failure to Create User",
 			fields: fields{
@@ -102,7 +170,7 @@ func TestClient_Add(t *testing.T) {
 			continue
 		}
 		if !reflect.DeepEqual(got, tt.want) {
-			t.Errorf("%q. Client.Add() = %v, want %v", tt.name, got, tt.want)
+			t.Errorf("%q. Client.Add() = \n%#v\n, want \n%#v\n", tt.name, got, tt.want)
 		}
 	}
 }
@@ -375,6 +443,9 @@ func TestClient_Update(t *testing.T) {
 					setUserPerms: func(ctx context.Context, name string, perms enterprise.Permissions) error {
 						return nil
 					},
+					userRoles: func(ctx context.Context) (map[string]enterprise.Roles, error) {
+						return map[string]enterprise.Roles{}, nil
+					},
 				},
 			},
 			args: args{
@@ -391,6 +462,40 @@ func TestClient_Update(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Success setting permissions and roles for user",
+			fields: fields{
+				Ctrl: &mockCtrl{
+					setUserPerms: func(ctx context.Context, name string, perms enterprise.Permissions) error {
+						return nil
+					},
+					addRoleUsers: func(ctx context.Context, name string, users []string) error {
+						return nil
+					},
+					userRoles: func(ctx context.Context) (map[string]enterprise.Roles, error) {
+						return map[string]enterprise.Roles{}, nil
+					},
+				},
+			},
+			args: args{
+				ctx: context.Background(),
+				u: &chronograf.User{
+					Name: "marty",
+					Permissions: chronograf.Permissions{
+						{
+							Scope:   chronograf.AllScope,
+							Allowed: chronograf.Allowances{"ViewChronograf", "KapacitorAPI"},
+						},
+					},
+					Roles: []chronograf.Role{
+						{
+							Name: "adminrole",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
 		{
 			name: "Failure setting permissions User",
 			fields: fields{
@@ -398,6 +503,9 @@ func TestClient_Update(t *testing.T) {
 					setUserPerms: func(ctx context.Context, name string, perms enterprise.Permissions) error {
 						return fmt.Errorf("They found me, I don't know how, but they found me.")
 					},
+					userRoles: func(ctx context.Context) (map[string]enterprise.Roles, error) {
+						return map[string]enterprise.Roles{}, nil
+					},
 				},
 			},
 			args: args{
@@ -595,12 +703,14 @@ type mockCtrl struct {
 
 	userRoles func(ctx context.Context) (map[string]enterprise.Roles, error)
 
-	roles        func(ctx context.Context, name *string) (*enterprise.Roles, error)
-	role         func(ctx context.Context, name string) (*enterprise.Role, error)
-	createRole   func(ctx context.Context, name string) error
-	deleteRole   func(ctx context.Context, name string) error
-	setRolePerms func(ctx context.Context, name string, perms enterprise.Permissions) error
-	setRoleUsers func(ctx context.Context, name string, users []string) error
+	roles           func(ctx context.Context, name *string) (*enterprise.Roles, error)
+	role            func(ctx context.Context, name string) (*enterprise.Role, error)
+	createRole      func(ctx context.Context, name string) error
+	deleteRole      func(ctx context.Context, name string) error
+	setRolePerms    func(ctx context.Context, name string, perms enterprise.Permissions) error
+	setRoleUsers    func(ctx context.Context, name string, users []string) error
+	addRoleUsers    func(ctx context.Context, name string, users []string) error
+	removeRoleUsers func(ctx context.Context, name string, users []string) error
 }
 
 func (m *mockCtrl) ShowCluster(ctx context.Context) (*enterprise.Cluster, error) {
@@ -658,3 +768,11 @@ func (m *mockCtrl) SetRolePerms(ctx context.Context, name string, perms enterpri
 func (m *mockCtrl) SetRoleUsers(ctx context.Context, name string, users []string) error {
 	return m.setRoleUsers(ctx, name, users)
 }
+
+func (m *mockCtrl) AddRoleUsers(ctx context.Context, name string, users []string) error {
+	return m.addRoleUsers(ctx, name, users)
+}
+
+func (m *mockCtrl) RemoveRoleUsers(ctx context.Context, name string, users []string) error {
+	return m.removeRoleUsers(ctx, name, users)
+}

server/users.go

@@ -153,6 +153,7 @@ func (h *Service) UpdateSourceUser(w http.ResponseWriter, r *http.Request) {
 		Name:        uid,
 		Passwd:      req.Password,
 		Permissions: req.Permissions,
+		Roles:       req.Roles,
 	}
 	store := ts.Users(ctx)