From 12facba297c12f95f0c7298f04f3e69b4c1fd926 Mon Sep 17 00:00:00 2001
From: Michael Desa <mjdesa@gmail.com>
Date: Thu, 9 Nov 2017 10:55:18 -0500
Subject: [PATCH] Ensure users name, provider, and scheme are unique

---
 bolt/users.go      | 27 +++++++++++++++++++++++++++
 bolt/users_test.go | 35 ++++++++++++++++++++++++++++++++---
 chronograf.go      |  9 +++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/bolt/users.go b/bolt/users.go
index 1397d4039..933ed15f4 100644
--- a/bolt/users.go
+++ b/bolt/users.go
@@ -82,8 +82,35 @@ func (s *UsersStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronogr
 	return nil, fmt.Errorf("must specify either ID, or Name, Provider, and Scheme in UserQuery")
 }
 
+func (s *UsersStore) userExists(ctx context.Context, u *chronograf.User) (bool, error) {
+	_, err := s.Get(ctx, chronograf.UserQuery{
+		Name:     &u.Name,
+		Provider: &u.Provider,
+		Scheme:   &u.Scheme,
+	})
+	if err == chronograf.ErrUserNotFound {
+		return false, nil
+	}
+
+	if err != nil {
+		return false, err
+	}
+
+	return true, nil
+}
+
 // Add a new User to the UsersStore.
 func (s *UsersStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.User, error) {
+	if u == nil {
+		return nil, fmt.Errorf("user provided is nil")
+	}
+	userExists, err := s.userExists(ctx, u)
+	if err != nil {
+		return nil, err
+	}
+	if userExists {
+		return nil, chronograf.ErrUserAlreadyExists
+	}
 	if err := s.client.db.Update(func(tx *bolt.Tx) error {
 		b := tx.Bucket(UsersBucket)
 		seq, err := b.NextSequence()
diff --git a/bolt/users_test.go b/bolt/users_test.go
index b98847590..57c819ef1 100644
--- a/bolt/users_test.go
+++ b/bolt/users_test.go
@@ -2,6 +2,7 @@ package bolt_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -181,8 +182,9 @@ func TestUsersStore_GetInvalid(t *testing.T) {
 
 func TestUsersStore_Add(t *testing.T) {
 	type args struct {
-		ctx context.Context
-		u   *chronograf.User
+		ctx      context.Context
+		u        *chronograf.User
+		addFirst bool
 	}
 	tests := []struct {
 		name    string
@@ -216,6 +218,24 @@ func TestUsersStore_Add(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "User already exists",
+			args: args{
+				ctx:      context.Background(),
+				addFirst: true,
+				u: &chronograf.User{
+					Name:     "docbrown",
+					Provider: "github",
+					Scheme:   "oauth2",
+					Roles: []chronograf.Role{
+						{
+							Name: "editor",
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		client, err := NewTestClient()
@@ -227,12 +247,19 @@ func TestUsersStore_Add(t *testing.T) {
 		}
 		defer client.Close()
 		s := client.UsersStore
+		if tt.args.addFirst {
+			_, _ = s.Add(tt.args.ctx, tt.args.u)
+		}
 		got, err := s.Add(tt.args.ctx, tt.args.u)
 		if (err != nil) != tt.wantErr {
 			t.Errorf("%q. UsersStore.Add() error = %v, wantErr %v", tt.name, err, tt.wantErr)
 			continue
 		}
 
+		if tt.wantErr {
+			continue
+		}
+
 		got, err = s.Get(tt.args.ctx, chronograf.UserQuery{ID: &got.ID})
 		if err != nil {
 			t.Fatalf("failed to get user: %v", err)
@@ -287,7 +314,9 @@ func TestUsersStore_Delete(t *testing.T) {
 		s := client.UsersStore
 
 		if tt.addFirst {
-			tt.args.user, _ = s.Add(tt.args.ctx, tt.args.user)
+			var err error
+			tt.args.user, err = s.Add(tt.args.ctx, tt.args.user)
+			fmt.Println(err)
 		}
 		if err := s.Delete(tt.args.ctx, tt.args.user); (err != nil) != tt.wantErr {
 			t.Errorf("%q. UsersStore.Delete() error = %v, wantErr %v", tt.name, err, tt.wantErr)
diff --git a/chronograf.go b/chronograf.go
index b540b1d2d..3b6f21ebf 100644
--- a/chronograf.go
+++ b/chronograf.go
@@ -26,6 +26,7 @@ const (
 	ErrLayoutNotFound                  = Error("layout not found")
 	ErrDashboardNotFound               = Error("dashboard not found")
 	ErrUserNotFound                    = Error("user not found")
+	ErrUserAlreadyExists               = Error("user already exists")
 	ErrOrganizationNotFound            = Error("organization not found")
 	ErrLayoutInvalid                   = Error("layout is invalid")
 	ErrAlertNotFound                   = Error("alert not found")
@@ -633,6 +634,9 @@ type User struct {
 
 // UserQuery represents the attributes that a user may be retrieved by.
 // It is predominantly used in the UsersStore.Get method.
+//
+// It is expected that only one of ID or Name, Provider, and Scheme will be
+// specified, but all are provided UserStores should prefer ID.
 type UserQuery struct {
 	ID       *uint64
 	Name     *string
@@ -641,6 +645,11 @@ type UserQuery struct {
 }
 
 // UsersStore is the Storage and retrieval of authentication information
+//
+// While not necessary for the app to function correctly, it is
+// expected that Implementors of the UsersStore will take
+// care to guarantee that the combinartion of a  users Name, Provider,
+// and Scheme are unique.
 type UsersStore interface {
 	// All lists all users from the UsersStore
 	All(context.Context) ([]User, error)