fix: omit users for 'Everyone' group in response (#5937)

sreya · web-flow · commit c162c0f284e6 · 2023-01-31T13:30:20.000-06:00
diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
@@ -4051,24 +4051,6 @@ func (q *fakeQuerier) GetGroupsByOrganizationID(_ context.Context, organizationI
 	return groups, nil
 }
 
-func (q *fakeQuerier) GetAllOrganizationMembers(_ context.Context, organizationID uuid.UUID) ([]database.User, error) {
-	q.mutex.RLock()
-	defer q.mutex.RUnlock()
-
-	var users []database.User
-	for _, member := range q.organizationMembers {
-		if member.OrganizationID == organizationID {
-			for _, user := range q.users {
-				if user.ID == member.UserID {
-					users = append(users, user)
-				}
-			}
-		}
-	}
-
-	return users, nil
-}
-
 func (q *fakeQuerier) DeleteGroupByID(_ context.Context, id uuid.UUID) error {
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql
@@ -36,18 +36,6 @@ AND
 AND
 	users.deleted = 'false';
 
--- name: GetAllOrganizationMembers :many
-SELECT
-	users.*
-FROM
-	users
-JOIN
-	organization_members
-ON
-	users.id = organization_members.user_id
-WHERE
-	organization_members.organization_id = $1;
-
 -- name: GetGroupsByOrganizationID :many
 SELECT
 	*
diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go
@@ -605,6 +605,26 @@ func TestGroup(t *testing.T) {
 		require.NotContains(t, group.Members, user1)
 		require.Contains(t, group.Members, user2)
 	})
+
+	t.Run("everyoneGroupReturnsEmpty", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdenttest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+
+		_ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{
+			Features: license.Features{
+				codersdk.FeatureTemplateRBAC: 1,
+			},
+		})
+		ctx, _ := testutil.Context(t)
+		// The 'Everyone' group always has an ID that matches the organization ID.
+		group, err := client.Group(ctx, user.OrganizationID)
+		require.NoError(t, err)
+		require.Len(t, group.Members, 0)
+		require.Equal(t, "Everyone", group.Name)
+		require.Equal(t, user.OrganizationID, group.OrganizationID)
+	})
 }
 
 // TODO: test auth.
diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go
@@ -78,16 +78,11 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
 	for _, group := range dbGroups {
 		var members []database.User
 
-		if group.Name == database.AllUsersGroup {
-			members, err = api.Database.GetAllOrganizationMembers(ctx, group.OrganizationID)
-		} else {
-			members, err = api.Database.GetGroupMembers(ctx, group.ID)
-		}
+		members, err = api.Database.GetGroupMembers(ctx, group.ID)
 		if err != nil {
 			httpapi.InternalServerError(rw, err)
 			return
 		}
-
 		groups = append(groups, codersdk.TemplateGroup{
 			Group: convertGroup(group.Group, members),
 			Role:  convertToTemplateRole(group.Actions),
diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go
@@ -66,7 +66,7 @@ func TestTemplateACL(t *testing.T) {
 		require.Contains(t, acl.Users, templateUser3)
 	})
 
-	t.Run("allUsersGroup", func(t *testing.T) {
+	t.Run("everyoneGroup", func(t *testing.T) {
 		t.Parallel()
 		client := coderdenttest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
@@ -76,7 +76,8 @@ func TestTemplateACL(t *testing.T) {
 			},
 		})
 
-		_, user1 := coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
+		// Create a user to assert they aren't returned in the response.
+		_, _ = coderdtest.CreateAnotherUserWithUser(t, client, user.OrganizationID)
 		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
 
@@ -87,8 +88,8 @@ func TestTemplateACL(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Len(t, acl.Groups, 1)
-		require.Len(t, acl.Groups[0].Members, 2)
-		require.Contains(t, acl.Groups[0].Members, user1)
+		// We don't return members for the 'Everyone' group.
+		require.Len(t, acl.Groups[0].Members, 0)
 		require.Len(t, acl.Users, 0)
 	})
 
