Merge pull request #2216 from influxdata/multitenancy_unique_org_names

Ensure unique organization names.

bolt/organizations.go

@@ -43,8 +43,21 @@ func (s *OrganizationsStore) Migrate(ctx context.Context) error {
 	})
 }
 
+func (s *OrganizationsStore) nameIsUnique(ctx context.Context, name string) bool {
+	_, err := s.Get(ctx, chronograf.OrganizationQuery{Name: &name})
+	switch err {
+	case chronograf.ErrOrganizationNotFound:
+		return true
+	default:
+		return false
+	}
+}
+
 // Add creates a new Organization in the OrganizationsStore
 func (s *OrganizationsStore) Add(ctx context.Context, o *chronograf.Organization) (*chronograf.Organization, error) {
+	if !s.nameIsUnique(ctx, o.Name) {
+		return nil, chronograf.ErrOrganizationNameTaken
+	}
 	if err := s.client.db.Update(func(tx *bolt.Tx) error {
 		b := tx.Bucket(OrganizationsBucket)
 		seq, err := b.NextSequence()
@@ -228,6 +241,9 @@ func (s *OrganizationsStore) Update(ctx context.Context, o *chronograf.Organizat
 	if err != nil {
 		return err
 	}
+	if o.Name != org.Name && !s.nameIsUnique(ctx, o.Name) {
+		return chronograf.ErrOrganizationNameTaken
+	}
 	return s.client.db.Update(func(tx *bolt.Tx) error {
 		org.Name = o.Name
 		if v, err := internal.MarshalOrganization(org); err != nil {

bolt/organizations_test.go

@@ -231,6 +231,9 @@ func TestOrganizationsStore_All(t *testing.T) {
 }
 
 func TestOrganizationsStore_Update(t *testing.T) {
+	type fields struct {
+		orgs []chronograf.Organization
+	}
 	type args struct {
 		ctx  context.Context
 		org  *chronograf.Organization
@@ -238,6 +241,7 @@ func TestOrganizationsStore_Update(t *testing.T) {
 	}
 	tests := []struct {
 		name     string
+		fields   fields
 		args     args
 		addFirst bool
 		want     *chronograf.Organization
@@ -245,6 +249,7 @@ func TestOrganizationsStore_Update(t *testing.T) {
 	}{
 		{
 			name:   "No such organization",
+			fields: fields{},
 			args: args{
 				ctx: context.Background(),
 				org: &chronograf.Organization{
@@ -256,6 +261,7 @@ func TestOrganizationsStore_Update(t *testing.T) {
 		},
 		{
 			name:   "Update organization name",
+			fields: fields{},
 			args: args{
 				ctx: context.Background(),
 				org: &chronograf.Organization{
@@ -268,6 +274,25 @@ func TestOrganizationsStore_Update(t *testing.T) {
 			},
 			addFirst: true,
 		},
+		{
+			name: "Update organization name - name already taken",
+			fields: fields{
+				orgs: []chronograf.Organization{
+					{
+						Name: "The Bad Place",
+					},
+				},
+			},
+			args: args{
+				ctx: context.Background(),
+				org: &chronograf.Organization{
+					Name: "The Good Place",
+				},
+				name: "The Bad Place",
+			},
+			wantErr:  true,
+			addFirst: true,
+		},
 	}
 	for _, tt := range tests {
 		client, err := NewTestClient()
@@ -280,13 +305,17 @@ func TestOrganizationsStore_Update(t *testing.T) {
 		defer client.Close()
 		s := client.OrganizationsStore
 
-		if tt.addFirst {
+		for _, org := range tt.fields.orgs {
-			tt.args.org, err = s.Add(tt.args.ctx, tt.args.org)
+			_, err = s.Add(tt.args.ctx, &org)
 			if err != nil {
 				t.Fatal(err)
 			}
 		}
 
+		if tt.addFirst {
+			tt.args.org, err = s.Add(tt.args.ctx, tt.args.org)
+		}
+
 		if tt.args.name != "" {
 			tt.args.org.Name = tt.args.name
 		}
@@ -361,3 +390,75 @@ func TestOrganizationStore_Delete(t *testing.T) {
 		}
 	}
 }
+
+func TestOrganizationsStore_Add(t *testing.T) {
+	type fields struct {
+		orgs []chronograf.Organization
+	}
+	type args struct {
+		ctx context.Context
+		org *chronograf.Organization
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    *chronograf.Organization
+		wantErr bool
+	}{
+		{
+			name: "Add organization - name already taken",
+			fields: fields{
+				orgs: []chronograf.Organization{
+					{
+						Name: "The Good Place",
+					},
+				},
+			},
+			args: args{
+				ctx: context.Background(),
+				org: &chronograf.Organization{
+					Name: "The Good Place",
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		client, err := NewTestClient()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := client.Open(context.TODO()); err != nil {
+			t.Fatal(err)
+		}
+		defer client.Close()
+		s := client.OrganizationsStore
+
+		for _, org := range tt.fields.orgs {
+			_, err = s.Add(tt.args.ctx, &org)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		_, err = s.Add(tt.args.ctx, tt.args.org)
+
+		if (err != nil) != tt.wantErr {
+			t.Errorf("%q. OrganizationsStore.Update() error = %v, wantErr %v", tt.name, err, tt.wantErr)
+		}
+
+		// for the empty test
+		if tt.want == nil {
+			continue
+		}
+
+		got, err := s.Get(tt.args.ctx, chronograf.OrganizationQuery{Name: &tt.args.org.Name})
+		if err != nil {
+			t.Fatalf("failed to get organization: %v", err)
+		}
+		if diff := cmp.Diff(got, tt.want, orgCmpOptions...); diff != "" {
+			t.Errorf("%q. OrganizationsStore.Update():\n-got/+want\ndiff %s", tt.name, diff)
+		}
+	}
+}

chronograf.go

@@ -32,6 +32,7 @@ const (
 	ErrAuthentication        = Error("user not authenticated")
 	ErrUninitialized         = Error("client uninitialized. Call Open() method")
 	ErrInvalidAxis           = Error("Unexpected axis in cell. Valid axes are 'x', 'y', and 'y2'")
+	ErrOrganizationNameTaken = Error("organization name is taken")
 )
 
 // Error is a domain error encountered while processing chronograf requests
@@ -786,6 +787,11 @@ type OrganizationQuery struct {
 }
 
 // OrganizationsStore is the storage and retrieval of Organizations
+//
+// While not necessary for the app to function correctly, it is
+// expected that Implementors of the OrganizationsStore will take
+// care to guarantee that the Organization.Name is unqiue. Allowing
+// for duplicate names creates a confusing UX experience for the User.
 type OrganizationsStore interface {
 	// Add creates a new Organization.
 	// The Created organization is returned back to the user with the