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

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()

coderd/database/querier.go

@@ -32,7 +32,6 @@ type sqlcQuerier interface {
 	GetAPIKeysByLoginType(ctx context.Context, loginType LoginType) ([]APIKey, error)
 	GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Time) ([]APIKey, error)
 	GetActiveUserCount(ctx context.Context) (int64, error)
-	GetAllOrganizationMembers(ctx context.Context, organizationID uuid.UUID) ([]User, error)
 	// GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided
 	// ID.
 	GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOffsetParams) ([]GetAuditLogsOffsetRow, error)

coderd/database/queries.sql.go

@@ -1002,55 +1002,6 @@ func (q *sqlQuerier) DeleteGroupMember(ctx context.Context, userID uuid.UUID) er
 	return err
 }
 
-const getAllOrganizationMembers = `-- name: GetAllOrganizationMembers :many
-SELECT
-	users.id, users.email, users.username, users.hashed_password, users.created_at, users.updated_at, users.status, users.rbac_roles, users.login_type, users.avatar_url, users.deleted, users.last_seen_at
-FROM
-	users
-JOIN
-	organization_members
-ON
-	users.id = organization_members.user_id
-WHERE
-	organization_members.organization_id = $1
-`
-
-func (q *sqlQuerier) GetAllOrganizationMembers(ctx context.Context, organizationID uuid.UUID) ([]User, error) {
-	rows, err := q.db.QueryContext(ctx, getAllOrganizationMembers, organizationID)
-	if err != nil {
-		return nil, err
-	}
-	defer rows.Close()
-	var items []User
-	for rows.Next() {
-		var i User
-		if err := rows.Scan(
-			&i.ID,
-			&i.Email,
-			&i.Username,
-			&i.HashedPassword,
-			&i.CreatedAt,
-			&i.UpdatedAt,
-			&i.Status,
-			&i.RBACRoles,
-			&i.LoginType,
-			&i.AvatarURL,
-			&i.Deleted,
-			&i.LastSeenAt,
-		); err != nil {
-			return nil, err
-		}
-		items = append(items, i)
-	}
-	if err := rows.Close(); err != nil {
-		return nil, err
-	}
-	if err := rows.Err(); err != nil {
-		return nil, err
-	}
-	return items, nil
-}
-
 const getGroupByID = `-- name: GetGroupByID :one
 SELECT
 	id, name, organization_id, avatar_url, quota_allowance

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
 	*

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.

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),

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)
 	})