Ensure additional Users aren't created OrganizationUser.Add
Performs a Get against the global UsersStore, and if the User already exists, it appends to that User instead of trying to add the "new" User blindly. Signed-off-by: Michael de Sa <mjdesa@gmail.com>pull/5028/head
parent
d45335f281
commit
a7558f6ce2
|
@ -86,7 +86,23 @@ func (s *OrganizationUsersStore) Add(ctx context.Context, u *chronograf.User) (*
|
|||
if err := validOrganizationRoles(orgID, u); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
usr, err := s.client.UsersStore.Get(ctx, chronograf.UserQuery{
|
||||
Name: &u.Name,
|
||||
Provider: &u.Provider,
|
||||
Scheme: &u.Scheme,
|
||||
})
|
||||
if err != nil && err != chronograf.ErrUserNotFound {
|
||||
return nil, err
|
||||
}
|
||||
if err == chronograf.ErrUserNotFound {
|
||||
return s.client.UsersStore.Add(ctx, u)
|
||||
}
|
||||
usr.Roles = append(usr.Roles, u.Roles...)
|
||||
if err := s.client.UsersStore.Update(ctx, usr); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
u.ID = usr.ID
|
||||
return u, nil
|
||||
}
|
||||
|
||||
// Delete the users from the OrganizationUsersStore
|
||||
|
|
|
@ -129,10 +129,12 @@ func TestOrganizationUsersStore_Add(t *testing.T) {
|
|||
ctx context.Context
|
||||
u *chronograf.User
|
||||
orgID string
|
||||
uInitial *chronograf.User
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
addFirst bool
|
||||
want *chronograf.User
|
||||
wantErr bool
|
||||
}{
|
||||
|
@ -183,6 +185,77 @@ func TestOrganizationUsersStore_Add(t *testing.T) {
|
|||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Add non-new user without Role",
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
u: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{},
|
||||
},
|
||||
orgID: "1336",
|
||||
uInitial: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{},
|
||||
},
|
||||
},
|
||||
addFirst: true,
|
||||
want: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Add non-new user with Role",
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
u: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{
|
||||
{
|
||||
OrganizationID: "1336",
|
||||
Name: "Admin",
|
||||
},
|
||||
},
|
||||
},
|
||||
orgID: "1336",
|
||||
uInitial: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{
|
||||
{
|
||||
OrganizationID: "1337",
|
||||
Name: "Editor",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
addFirst: true,
|
||||
want: &chronograf.User{
|
||||
Name: "docbrown",
|
||||
Provider: "GitHub",
|
||||
Scheme: "OAuth2",
|
||||
Roles: []chronograf.Role{
|
||||
{
|
||||
OrganizationID: "1337",
|
||||
Name: "Editor",
|
||||
},
|
||||
{
|
||||
OrganizationID: "1336",
|
||||
Name: "Admin",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Has invalid Role: missing OrganizationID",
|
||||
args: args{
|
||||
|
@ -243,6 +316,11 @@ func TestOrganizationUsersStore_Add(t *testing.T) {
|
|||
|
||||
tt.args.ctx = context.WithValue(tt.args.ctx, "organizationID", tt.args.orgID)
|
||||
s := client.OrganizationUsersStore
|
||||
|
||||
if tt.addFirst {
|
||||
client.UsersStore.Add(tt.args.ctx, tt.args.uInitial)
|
||||
}
|
||||
|
||||
got, err := s.Add(tt.args.ctx, tt.args.u)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("%q. OrganizationUsersStore.Add() error = %v, wantErr %v", tt.name, err, tt.wantErr)
|
||||
|
|
Loading…
Reference in New Issue