From 13631064fc3abd6b4e4de74ffeeb05a2e766d1c0 Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <jeffrey.mitchell@gmail.com>
Date: Tue, 13 Oct 2020 13:56:21 -0400
Subject: [PATCH] Don't create a default role when the scope being created is a
 project (#658)

* Don't create a default role when the scope being created is a project

* Fix up some more tests
---
 internal/iam/repository_role_test.go          |  2 +-
 internal/iam/repository_scope.go              |  2 +-
 .../handlers/scopes/scope_service_test.go     |  9 ++++++--
 internal/tests/api/roles/role_test.go         | 21 ++++++++++++-------
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/internal/iam/repository_role_test.go b/internal/iam/repository_role_test.go
index 47f2ea8582..d92b45bb65 100644
--- a/internal/iam/repository_role_test.go
+++ b/internal/iam/repository_role_test.go
@@ -584,7 +584,7 @@ func TestRepository_ListRoles(t *testing.T) {
 				withScopeId: proj.PublicId,
 				opt:         []Option{WithLimit(-1)},
 			},
-			wantCnt: repo.defaultLimit + 2,
+			wantCnt: repo.defaultLimit + 1,
 			wantErr: false,
 		},
 		{
diff --git a/internal/iam/repository_scope.go b/internal/iam/repository_scope.go
index c8e8f89d63..1cacc157a4 100644
--- a/internal/iam/repository_scope.go
+++ b/internal/iam/repository_scope.go
@@ -125,7 +125,7 @@ func (r *Repository) CreateScope(ctx context.Context, s *Scope, userId string, o
 	var defaultRoleMetadata oplog.Metadata
 	var defaultRole *Role
 	var defaultRoleRaw interface{}
-	if !opts.withSkipDefaultRoleCreation {
+	if !opts.withSkipDefaultRoleCreation && s.Type == scope.Org.String() {
 		defaultRole, err = NewRole(scopePublicId)
 		if err != nil {
 			return nil, fmt.Errorf("create scope: error instantiating new default role: %w", err)
diff --git a/internal/servers/controller/handlers/scopes/scope_service_test.go b/internal/servers/controller/handlers/scopes/scope_service_test.go
index 8ffe155841..2b9224f43a 100644
--- a/internal/servers/controller/handlers/scopes/scope_service_test.go
+++ b/internal/servers/controller/handlers/scopes/scope_service_test.go
@@ -202,7 +202,7 @@ func TestList(t *testing.T) {
 			name:    "Cant List Project Scopes",
 			scopeId: p1.GetPublicId(),
 			req:     &pbs.ListScopesRequest{ScopeId: p1.GetPublicId()},
-			err: handlers.ApiErrorWithCode(codes.InvalidArgument),
+			err:     handlers.ApiErrorWithCode(codes.InvalidArgument),
 		},
 	}
 	for _, tc := range cases {
@@ -615,7 +615,12 @@ func TestCreate(t *testing.T) {
 						require.NoError(err)
 						roles, err := repo.ListRoles(ctx, got.GetItem().GetId())
 						require.NoError(err)
-						require.Len(roles, 2)
+						switch tc.scopeId {
+						case defaultOrg.PublicId:
+							require.Len(roles, 1)
+						case "global":
+							require.Len(roles, 2)
+						}
 						for _, role := range roles {
 							switch role.GetName() {
 							case "Administration":
diff --git a/internal/tests/api/roles/role_test.go b/internal/tests/api/roles/role_test.go
index dfa4ac292a..b9bf9a86a2 100644
--- a/internal/tests/api/roles/role_test.go
+++ b/internal/tests/api/roles/role_test.go
@@ -143,22 +143,29 @@ func TestList(t *testing.T) {
 			roleClient := roles.NewClient(client)
 			p1, err := roleClient.List(tc.Context(), tt.scopeId)
 			require.NoError(err)
-			require.Len(p1.Items, 2)
-			expected = append(expected, p1.Items[0:2]...)
+			var numBuiltIn int
+			switch tt.name {
+			case "org":
+				numBuiltIn = 2
+			case "proj":
+				numBuiltIn = 1
+			}
+			require.Len(p1.Items, numBuiltIn)
+			expected = append(expected, p1.Items[0:numBuiltIn]...)
 
-			for i := 2; i < 12; i++ {
+			for i := numBuiltIn; i < 10+numBuiltIn; i++ {
 				expected = append(expected, &roles.Role{Name: fmt.Sprint(i)})
 			}
 
-			rcr, err := roleClient.Create(tc.Context(), tt.scopeId, roles.WithName(expected[2].Name))
+			rcr, err := roleClient.Create(tc.Context(), tt.scopeId, roles.WithName(expected[numBuiltIn].Name))
 			require.NoError(err)
-			expected[2] = rcr.Item
+			expected[numBuiltIn] = rcr.Item
 
 			p2, err := roleClient.List(tc.Context(), tt.scopeId)
 			require.NoError(err)
-			assert.ElementsMatch(comparableSlice(expected[0:3]), comparableSlice(p2.Items))
+			assert.ElementsMatch(comparableSlice(expected[0:numBuiltIn+1]), comparableSlice(p2.Items))
 
-			for i := 3; i < 12; i++ {
+			for i := numBuiltIn + 1; i < 10+numBuiltIn; i++ {
 				rcr, err = roleClient.Create(tc.Context(), tt.scopeId, roles.WithName(expected[i].Name))
 				assert.NoError(err)
 				expected[i] = rcr.Item