diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8fa9d0893421c..3c51e33e3e5a4 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10175,6 +10175,10 @@ const docTemplate = `{ }, "source": { "$ref": "#/definitions/codersdk.GroupSource" + }, + "total_member_count": { + "description": "How many members are in this group. Shows the total count,\neven if the user is not authorized to read group member details.\nMay be greater than ` + "`" + `len(Group.Members)` + "`" + `.", + "type": "integer" } } }, @@ -11566,6 +11570,7 @@ const docTemplate = `{ "deployment_stats", "file", "group", + "group_member", "license", "notification_preference", "notification_template", @@ -11596,6 +11601,7 @@ const docTemplate = `{ "ResourceDeploymentStats", "ResourceFile", "ResourceGroup", + "ResourceGroupMember", "ResourceLicense", "ResourceNotificationPreference", "ResourceNotificationTemplate", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 4fbb0f86e195a..bf4dc370aa3be 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9133,6 +9133,10 @@ }, "source": { "$ref": "#/definitions/codersdk.GroupSource" + }, + "total_member_count": { + "description": "How many members are in this group. Shows the total count,\neven if the user is not authorized to read group member details.\nMay be greater than `len(Group.Members)`.", + "type": "integer" } } }, @@ -10436,6 +10440,7 @@ "deployment_stats", "file", "group", + "group_member", "license", "notification_preference", "notification_template", @@ -10466,6 +10471,7 @@ "ResourceDeploymentStats", "ResourceFile", "ResourceGroup", + "ResourceGroupMember", "ResourceLicense", "ResourceNotificationPreference", "ResourceNotificationTemplate", diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 818793182e468..1d513e75aff47 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -159,6 +159,35 @@ func ReducedUser(user database.User) codersdk.ReducedUser { } } +func UserFromGroupMember(member database.GroupMember) database.User { + return database.User{ + ID: member.UserID, + Email: member.UserEmail, + Username: member.UserUsername, + HashedPassword: member.UserHashedPassword, + CreatedAt: member.UserCreatedAt, + UpdatedAt: member.UserUpdatedAt, + Status: member.UserStatus, + RBACRoles: member.UserRbacRoles, + LoginType: member.UserLoginType, + AvatarURL: member.UserAvatarUrl, + Deleted: member.UserDeleted, + LastSeenAt: member.UserLastSeenAt, + QuietHoursSchedule: member.UserQuietHoursSchedule, + ThemePreference: member.UserThemePreference, + Name: member.UserName, + GithubComUserID: member.UserGithubComUserID, + } +} + +func ReducedUserFromGroupMember(member database.GroupMember) codersdk.ReducedUser { + return ReducedUser(UserFromGroupMember(member)) +} + +func ReducedUsersFromGroupMembers(members []database.GroupMember) []codersdk.ReducedUser { + return List(members, ReducedUserFromGroupMember) +} + func ReducedUsers(users []database.User) []codersdk.ReducedUser { return List(users, ReducedUser) } @@ -179,16 +208,17 @@ func Users(users []database.User, organizationIDs map[uuid.UUID][]uuid.UUID) []c }) } -func Group(group database.Group, members []database.User) codersdk.Group { +func Group(group database.Group, members []database.GroupMember, totalMemberCount int) codersdk.Group { return codersdk.Group{ - ID: group.ID, - Name: group.Name, - DisplayName: group.DisplayName, - OrganizationID: group.OrganizationID, - AvatarURL: group.AvatarURL, - Members: ReducedUsers(members), - QuotaAllowance: int(group.QuotaAllowance), - Source: codersdk.GroupSource(group.Source), + ID: group.ID, + Name: group.Name, + DisplayName: group.DisplayName, + OrganizationID: group.OrganizationID, + AvatarURL: group.AvatarURL, + Members: ReducedUsersFromGroupMembers(members), + TotalMemberCount: totalMemberCount, + QuotaAllowance: int(group.QuotaAllowance), + Source: codersdk.GroupSource(group.Source), } } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 1ab1183985098..282de1faccc22 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1396,11 +1396,19 @@ func (q *querier) GetGroupMembers(ctx context.Context) ([]database.GroupMember, return q.db.GetGroupMembers(ctx) } -func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([]database.User, error) { - if _, err := q.GetGroupByID(ctx, id); err != nil { // AuthZ check - return nil, err +func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([]database.GroupMember, error) { + return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetGroupMembersByGroupID)(ctx, id) +} + +func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) { + if _, err := q.GetGroupByID(ctx, groupID); err != nil { // AuthZ check + return 0, err + } + memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, groupID) + if err != nil { + return 0, err } - return q.db.GetGroupMembersByGroupID(ctx, id) + return memberCount, nil } func (q *querier) GetGroups(ctx context.Context) ([]database.Group, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 4eb764fde57b1..d186b789eabf1 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -305,8 +305,10 @@ func (s *MethodTestSuite) TestGroup() { })) s.Run("DeleteGroupMemberFromGroup", s.Subtest(func(db database.Store, check *expects) { g := dbgen.Group(s.T(), db, database.Group{}) - m := dbgen.GroupMember(s.T(), db, database.GroupMember{ + u := dbgen.User(s.T(), db, database.User{}) + m := dbgen.GroupMember(s.T(), db, database.GroupMemberTable{ GroupID: g.ID, + UserID: u.ID, }) check.Args(database.DeleteGroupMemberFromGroupParams{ UserID: m.UserID, @@ -326,11 +328,18 @@ func (s *MethodTestSuite) TestGroup() { })) s.Run("GetGroupMembersByGroupID", s.Subtest(func(db database.Store, check *expects) { g := dbgen.Group(s.T(), db, database.Group{}) - _ = dbgen.GroupMember(s.T(), db, database.GroupMember{}) + u := dbgen.User(s.T(), db, database.User{}) + gm := dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID}) + check.Args(g.ID).Asserts(gm, policy.ActionRead) + })) + s.Run("GetGroupMembersCountByGroupID", s.Subtest(func(db database.Store, check *expects) { + g := dbgen.Group(s.T(), db, database.Group{}) check.Args(g.ID).Asserts(g, policy.ActionRead) })) s.Run("GetGroupMembers", s.Subtest(func(db database.Store, check *expects) { - _ = dbgen.GroupMember(s.T(), db, database.GroupMember{}) + g := dbgen.Group(s.T(), db, database.Group{}) + u := dbgen.User(s.T(), db, database.User{}) + dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID}) check.Asserts(rbac.ResourceSystem, policy.ActionRead) })) s.Run("GetGroups", s.Subtest(func(db database.Store, check *expects) { @@ -339,7 +348,8 @@ func (s *MethodTestSuite) TestGroup() { })) s.Run("GetGroupsByOrganizationAndUserID", s.Subtest(func(db database.Store, check *expects) { g := dbgen.Group(s.T(), db, database.Group{}) - gm := dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g.ID}) + u := dbgen.User(s.T(), db, database.User{}) + gm := dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID}) check.Args(database.GetGroupsByOrganizationAndUserIDParams{ OrganizationID: g.OrganizationID, UserID: gm.UserID, @@ -368,7 +378,7 @@ func (s *MethodTestSuite) TestGroup() { u1 := dbgen.User(s.T(), db, database.User{}) g1 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID}) g2 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID}) - _ = dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g1.ID, UserID: u1.ID}) + _ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g1.ID, UserID: u1.ID}) check.Args(database.InsertUserGroupsByNameParams{ OrganizationID: o.ID, UserID: u1.ID, @@ -380,8 +390,8 @@ func (s *MethodTestSuite) TestGroup() { u1 := dbgen.User(s.T(), db, database.User{}) g1 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID}) g2 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID}) - _ = dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g1.ID, UserID: u1.ID}) - _ = dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g2.ID, UserID: u1.ID}) + _ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g1.ID, UserID: u1.ID}) + _ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g2.ID, UserID: u1.ID}) check.Args(u1.ID).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns() })) s.Run("UpdateGroupByID", s.Subtest(func(db database.Store, check *expects) { diff --git a/coderd/database/dbauthz/groupsauth_test.go b/coderd/database/dbauthz/groupsauth_test.go index d742e15148fa9..a72c4db3af38a 100644 --- a/coderd/database/dbauthz/groupsauth_test.go +++ b/coderd/database/dbauthz/groupsauth_test.go @@ -115,18 +115,15 @@ func TestGroupsAuth(t *testing.T) { Name: "GroupMember", Subject: rbac.Subject{ ID: users[0].ID.String(), - Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.ScopedRoleOrgMember(org.ID)}.Expand())), + Roles: rbac.Roles(must(rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(org.ID)}.Expand())), Groups: []string{ - group.Name, + group.ID.String(), }, Scope: rbac.ExpandableScope(rbac.ScopeAll), }, - // TODO: currently group members cannot see their own groups. - // If this is fixed, these booleans should be flipped to true. - ReadGroup: false, - ReadMembers: false, - // TODO: If fixed, they should only be able to see themselves - // MembersExpected: 1, + ReadGroup: true, + ReadMembers: true, + MembersExpected: 1, }, { // Org admin in the incorrect organization @@ -160,8 +157,7 @@ func TestGroupsAuth(t *testing.T) { require.NoError(t, err, "member read") require.Len(t, members, tc.MembersExpected, "member count found does not match") } else { - require.Error(t, err, "member read") - require.True(t, dbauthz.IsNotAuthorizedError(err), "not authorized error") + require.Len(t, members, 0, "member count is not 0") } }) } diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index a6ca57662e28d..ab7cb9bf1b5a3 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -6,6 +6,7 @@ import ( "database/sql" "encoding/hex" "encoding/json" + "errors" "fmt" "net" "strings" @@ -374,18 +375,61 @@ func Group(t testing.TB, db database.Store, orig database.Group) database.Group return group } -func GroupMember(t testing.TB, db database.Store, orig database.GroupMember) database.GroupMember { - member := database.GroupMember{ - UserID: takeFirst(orig.UserID, uuid.New()), - GroupID: takeFirst(orig.GroupID, uuid.New()), - } +// GroupMember requires a user + group to already exist. +// Example for creating a group member for a random group + user. +// +// GroupMember(t, db, database.GroupMemberTable{ +// UserID: User(t, db, database.User{}).ID, +// GroupID: Group(t, db, database.Group{ +// OrganizationID: must(db.GetDefaultOrganization(genCtx)).ID, +// }).ID, +// }) +func GroupMember(t testing.TB, db database.Store, member database.GroupMemberTable) database.GroupMember { + require.NotEqual(t, member.UserID, uuid.Nil, "A user id is required to use 'dbgen.GroupMember', use 'dbgen.User'.") + require.NotEqual(t, member.GroupID, uuid.Nil, "A group id is required to use 'dbgen.GroupMember', use 'dbgen.Group'.") + //nolint:gosimple err := db.InsertGroupMember(genCtx, database.InsertGroupMemberParams{ UserID: member.UserID, GroupID: member.GroupID, }) require.NoError(t, err, "insert group member") - return member + + user, err := db.GetUserByID(genCtx, member.UserID) + if errors.Is(err, sql.ErrNoRows) { + require.NoErrorf(t, err, "'dbgen.GroupMember' failed as the user with id %s does not exist. A user is required to use this function, use 'dbgen.User'.", member.UserID) + } + require.NoError(t, err, "get user by id") + + group, err := db.GetGroupByID(genCtx, member.GroupID) + if errors.Is(err, sql.ErrNoRows) { + require.NoErrorf(t, err, "'dbgen.GroupMember' failed as the group with id %s does not exist. A group is required to use this function, use 'dbgen.Group'.", member.GroupID) + } + require.NoError(t, err, "get group by id") + + groupMember := database.GroupMember{ + UserID: user.ID, + UserEmail: user.Email, + UserUsername: user.Username, + UserHashedPassword: user.HashedPassword, + UserCreatedAt: user.CreatedAt, + UserUpdatedAt: user.UpdatedAt, + UserStatus: user.Status, + UserRbacRoles: user.RBACRoles, + UserLoginType: user.LoginType, + UserAvatarUrl: user.AvatarURL, + UserDeleted: user.Deleted, + UserLastSeenAt: user.LastSeenAt, + UserQuietHoursSchedule: user.QuietHoursSchedule, + UserThemePreference: user.ThemePreference, + UserName: user.Name, + UserGithubComUserID: user.GithubComUserID, + OrganizationID: group.OrganizationID, + GroupName: group.Name, + GroupID: group.ID, + } + + return groupMember } // ProvisionerJob is a bit more involved to get the values such as "completedAt", "startedAt", "cancelledAt" set. ps diff --git a/coderd/database/dbgen/dbgen_test.go b/coderd/database/dbgen/dbgen_test.go index 5f9c235f312db..04f6d38d70d00 100644 --- a/coderd/database/dbgen/dbgen_test.go +++ b/coderd/database/dbgen/dbgen_test.go @@ -102,8 +102,8 @@ func TestGenerator(t *testing.T) { db := dbmem.New() g := dbgen.Group(t, db, database.Group{}) u := dbgen.User(t, db, database.User{}) - exp := []database.User{u} - dbgen.GroupMember(t, db, database.GroupMember{GroupID: g.ID, UserID: u.ID}) + gm := dbgen.GroupMember(t, db, database.GroupMemberTable{GroupID: g.ID, UserID: u.ID}) + exp := []database.GroupMember{gm} require.Equal(t, exp, must(db.GetGroupMembersByGroupID(context.Background(), g.ID))) }) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 2ad54acd21473..9bbc531dfda9e 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -60,7 +60,7 @@ func New() database.Store { dbcryptKeys: make([]database.DBCryptKey, 0), externalAuthLinks: make([]database.ExternalAuthLink, 0), groups: make([]database.Group, 0), - groupMembers: make([]database.GroupMember, 0), + groupMembers: make([]database.GroupMemberTable, 0), auditLogs: make([]database.AuditLog, 0), files: make([]database.File, 0), gitSSHKey: make([]database.GitSSHKey, 0), @@ -156,7 +156,7 @@ type data struct { files []database.File externalAuthLinks []database.ExternalAuthLink gitSSHKey []database.GitSSHKey - groupMembers []database.GroupMember + groupMembers []database.GroupMemberTable groups []database.Group jfrogXRayScans []database.JfrogXrayScan licenses []database.License @@ -723,18 +723,68 @@ func (q *FakeQuerier) getOrganizationMemberNoLock(orgID uuid.UUID) []database.Or return members } +var errUserDeleted = xerrors.New("user deleted") + +// getGroupMemberNoLock fetches a group member by user ID and group ID. +func (q *FakeQuerier) getGroupMemberNoLock(ctx context.Context, userID, groupID uuid.UUID) (database.GroupMember, error) { + groupName := "Everyone" + orgID := groupID + groupIsEveryone := q.isEveryoneGroup(groupID) + if !groupIsEveryone { + group, err := q.getGroupByIDNoLock(ctx, groupID) + if err != nil { + return database.GroupMember{}, err + } + groupName = group.Name + orgID = group.OrganizationID + } + + user, err := q.getUserByIDNoLock(userID) + if err != nil { + return database.GroupMember{}, err + } + if user.Deleted { + return database.GroupMember{}, errUserDeleted + } + + return database.GroupMember{ + UserID: user.ID, + UserEmail: user.Email, + UserUsername: user.Username, + UserHashedPassword: user.HashedPassword, + UserCreatedAt: user.CreatedAt, + UserUpdatedAt: user.UpdatedAt, + UserStatus: user.Status, + UserRbacRoles: user.RBACRoles, + UserLoginType: user.LoginType, + UserAvatarUrl: user.AvatarURL, + UserDeleted: user.Deleted, + UserLastSeenAt: user.LastSeenAt, + UserQuietHoursSchedule: user.QuietHoursSchedule, + UserThemePreference: user.ThemePreference, + UserName: user.Name, + UserGithubComUserID: user.GithubComUserID, + OrganizationID: orgID, + GroupName: groupName, + GroupID: groupID, + }, nil +} + // getEveryoneGroupMembersNoLock fetches all the users in an organization. -func (q *FakeQuerier) getEveryoneGroupMembersNoLock(orgID uuid.UUID) []database.User { +func (q *FakeQuerier) getEveryoneGroupMembersNoLock(ctx context.Context, orgID uuid.UUID) []database.GroupMember { var ( - everyone []database.User + everyone []database.GroupMember orgMembers = q.getOrganizationMemberNoLock(orgID) ) for _, member := range orgMembers { - user, err := q.getUserByIDNoLock(member.UserID) + groupMember, err := q.getGroupMemberNoLock(ctx, member.UserID, orgID) + if errors.Is(err, errUserDeleted) { + continue + } if err != nil { return nil } - everyone = append(everyone, user) + everyone = append(everyone, groupMember) } return everyone } @@ -2486,42 +2536,67 @@ func (q *FakeQuerier) GetGroupByOrgAndName(_ context.Context, arg database.GetGr return database.Group{}, sql.ErrNoRows } -func (q *FakeQuerier) GetGroupMembers(_ context.Context) ([]database.GroupMember, error) { +func (q *FakeQuerier) GetGroupMembers(ctx context.Context) ([]database.GroupMember, error) { q.mutex.RLock() defer q.mutex.RUnlock() - out := make([]database.GroupMember, len(q.groupMembers)) - copy(out, q.groupMembers) - return out, nil + members := make([]database.GroupMemberTable, 0, len(q.groupMembers)) + members = append(members, q.groupMembers...) + for _, org := range q.organizations { + for _, user := range q.users { + members = append(members, database.GroupMemberTable{ + UserID: user.ID, + GroupID: org.ID, + }) + } + } + + var groupMembers []database.GroupMember + for _, member := range members { + groupMember, err := q.getGroupMemberNoLock(ctx, member.UserID, member.GroupID) + if errors.Is(err, errUserDeleted) { + continue + } + if err != nil { + return nil, err + } + groupMembers = append(groupMembers, groupMember) + } + + return groupMembers, nil } -func (q *FakeQuerier) GetGroupMembersByGroupID(_ context.Context, id uuid.UUID) ([]database.User, error) { +func (q *FakeQuerier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([]database.GroupMember, error) { q.mutex.RLock() defer q.mutex.RUnlock() if q.isEveryoneGroup(id) { - return q.getEveryoneGroupMembersNoLock(id), nil + return q.getEveryoneGroupMembersNoLock(ctx, id), nil } - var members []database.GroupMember + var groupMembers []database.GroupMember for _, member := range q.groupMembers { + groupMember, err := q.getGroupMemberNoLock(ctx, member.UserID, member.GroupID) + if errors.Is(err, errUserDeleted) { + continue + } + if err != nil { + return nil, err + } if member.GroupID == id { - members = append(members, member) + groupMembers = append(groupMembers, groupMember) } } - users := make([]database.User, 0, len(members)) + return groupMembers, nil +} - for _, member := range members { - for _, user := range q.users { - if user.ID == member.UserID && !user.Deleted { - users = append(users, user) - break - } - } +func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) { + users, err := q.GetGroupMembersByGroupID(ctx, groupID) + if err != nil { + return 0, err } - - return users, nil + return int64(len(users)), nil } func (q *FakeQuerier) GetGroups(_ context.Context) ([]database.Group, error) { @@ -2541,15 +2616,15 @@ func (q *FakeQuerier) GetGroupsByOrganizationAndUserID(_ context.Context, arg da q.mutex.RLock() defer q.mutex.RUnlock() - var groupIds []uuid.UUID + var groupIDs []uuid.UUID for _, member := range q.groupMembers { if member.UserID == arg.UserID { - groupIds = append(groupIds, member.GroupID) + groupIDs = append(groupIDs, member.GroupID) } } groups := []database.Group{} for _, group := range q.groups { - if slices.Contains(groupIds, group.ID) && group.OrganizationID == arg.OrganizationID { + if slices.Contains(groupIDs, group.ID) && group.OrganizationID == arg.OrganizationID { groups = append(groups, group) } } @@ -6234,7 +6309,7 @@ func (q *FakeQuerier) InsertGroupMember(_ context.Context, arg database.InsertGr } //nolint:gosimple - q.groupMembers = append(q.groupMembers, database.GroupMember{ + q.groupMembers = append(q.groupMembers, database.GroupMemberTable{ GroupID: arg.GroupID, UserID: arg.UserID, }) @@ -6774,7 +6849,7 @@ func (q *FakeQuerier) InsertUserGroupsByName(_ context.Context, arg database.Ins } for _, groupID := range groupIDs { - q.groupMembers = append(q.groupMembers, database.GroupMember{ + q.groupMembers = append(q.groupMembers, database.GroupMemberTable{ UserID: arg.UserID, GroupID: groupID, }) diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 207f02d241e99..eb62316be1902 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -648,13 +648,20 @@ func (m metricsStore) GetGroupMembers(ctx context.Context) ([]database.GroupMemb return r0, r1 } -func (m metricsStore) GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]database.User, error) { +func (m metricsStore) GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]database.GroupMember, error) { start := time.Now() users, err := m.s.GetGroupMembersByGroupID(ctx, groupID) m.queryLatencies.WithLabelValues("GetGroupMembersByGroupID").Observe(time.Since(start).Seconds()) return users, err } +func (m metricsStore) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) { + start := time.Now() + r0, r1 := m.s.GetGroupMembersCountByGroupID(ctx, groupID) + m.queryLatencies.WithLabelValues("GetGroupMembersCountByGroupID").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetGroups(ctx context.Context) ([]database.Group, error) { start := time.Now() r0, r1 := m.s.GetGroups(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 11c1dcd86c831..9920dafada324 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1285,10 +1285,10 @@ func (mr *MockStoreMockRecorder) GetGroupMembers(arg0 any) *gomock.Call { } // GetGroupMembersByGroupID mocks base method. -func (m *MockStore) GetGroupMembersByGroupID(arg0 context.Context, arg1 uuid.UUID) ([]database.User, error) { +func (m *MockStore) GetGroupMembersByGroupID(arg0 context.Context, arg1 uuid.UUID) ([]database.GroupMember, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetGroupMembersByGroupID", arg0, arg1) - ret0, _ := ret[0].([]database.User) + ret0, _ := ret[0].([]database.GroupMember) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -1299,6 +1299,21 @@ func (mr *MockStoreMockRecorder) GetGroupMembersByGroupID(arg0, arg1 any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroupMembersByGroupID", reflect.TypeOf((*MockStore)(nil).GetGroupMembersByGroupID), arg0, arg1) } +// GetGroupMembersCountByGroupID mocks base method. +func (m *MockStore) GetGroupMembersCountByGroupID(arg0 context.Context, arg1 uuid.UUID) (int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGroupMembersCountByGroupID", arg0, arg1) + ret0, _ := ret[0].(int64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGroupMembersCountByGroupID indicates an expected call of GetGroupMembersCountByGroupID. +func (mr *MockStoreMockRecorder) GetGroupMembersCountByGroupID(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGroupMembersCountByGroupID", reflect.TypeOf((*MockStore)(nil).GetGroupMembersCountByGroupID), arg0, arg1) +} + // GetGroups mocks base method. func (m *MockStore) GetGroups(arg0 context.Context) ([]database.Group, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index a16d4bc31847b..c8d360bdf4cae 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -563,6 +563,77 @@ COMMENT ON COLUMN groups.display_name IS 'Display name is a custom, human-friend COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like OIDC group sync.'; +CREATE TABLE organization_members ( + user_id uuid NOT NULL, + organization_id uuid NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + roles text[] DEFAULT '{}'::text[] NOT NULL +); + +CREATE TABLE users ( + id uuid NOT NULL, + email text NOT NULL, + username text DEFAULT ''::text NOT NULL, + hashed_password bytea NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + status user_status DEFAULT 'dormant'::user_status NOT NULL, + rbac_roles text[] DEFAULT '{}'::text[] NOT NULL, + login_type login_type DEFAULT 'password'::login_type NOT NULL, + avatar_url text DEFAULT ''::text NOT NULL, + deleted boolean DEFAULT false NOT NULL, + last_seen_at timestamp without time zone DEFAULT '0001-01-01 00:00:00'::timestamp without time zone NOT NULL, + quiet_hours_schedule text DEFAULT ''::text NOT NULL, + theme_preference text DEFAULT ''::text NOT NULL, + name text DEFAULT ''::text NOT NULL, + github_com_user_id bigint +); + +COMMENT ON COLUMN users.quiet_hours_schedule IS 'Daily (!) cron schedule (with optional CRON_TZ) signifying the start of the user''s quiet hours. If empty, the default quiet hours on the instance is used instead.'; + +COMMENT ON COLUMN users.theme_preference IS '"" can be interpreted as "the user does not care", falling back to the default theme'; + +COMMENT ON COLUMN users.name IS 'Name of the Coder user'; + +COMMENT ON COLUMN users.github_com_user_id IS 'The GitHub.com numerical user ID. At time of implementation, this is used to check if the user has starred the Coder repository.'; + +CREATE VIEW group_members_expanded AS + WITH all_members AS ( + SELECT group_members.user_id, + group_members.group_id + FROM group_members + UNION + SELECT organization_members.user_id, + organization_members.organization_id AS group_id + FROM organization_members + ) + SELECT users.id AS user_id, + users.email AS user_email, + users.username AS user_username, + users.hashed_password AS user_hashed_password, + users.created_at AS user_created_at, + users.updated_at AS user_updated_at, + users.status AS user_status, + users.rbac_roles AS user_rbac_roles, + users.login_type AS user_login_type, + users.avatar_url AS user_avatar_url, + users.deleted AS user_deleted, + users.last_seen_at AS user_last_seen_at, + users.quiet_hours_schedule AS user_quiet_hours_schedule, + users.theme_preference AS user_theme_preference, + users.name AS user_name, + users.github_com_user_id AS user_github_com_user_id, + groups.organization_id, + groups.name AS group_name, + all_members.group_id + FROM ((all_members + JOIN users ON ((users.id = all_members.user_id))) + JOIN groups ON ((groups.id = all_members.group_id))) + WHERE (users.deleted = false); + +COMMENT ON VIEW group_members_expanded IS 'Joins group members with user information, organization ID, group name. Includes both regular group members and organization members (as part of the "Everyone" group).'; + CREATE TABLE jfrog_xray_scans ( agent_id uuid NOT NULL, workspace_id uuid NOT NULL, @@ -680,14 +751,6 @@ CREATE TABLE oauth2_provider_apps ( COMMENT ON TABLE oauth2_provider_apps IS 'A table used to configure apps that can use Coder as an OAuth2 provider, the reverse of what we are calling external authentication.'; -CREATE TABLE organization_members ( - user_id uuid NOT NULL, - organization_id uuid NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - roles text[] DEFAULT '{}'::text[] NOT NULL -); - CREATE TABLE organizations ( id uuid NOT NULL, name text NOT NULL, @@ -1014,33 +1077,6 @@ COMMENT ON COLUMN template_versions.external_auth_providers IS 'IDs of External COMMENT ON COLUMN template_versions.message IS 'Message describing the changes in this version of the template, similar to a Git commit message. Like a commit message, this should be a short, high-level description of the changes in this version of the template. This message is immutable and should not be updated after the fact.'; -CREATE TABLE users ( - id uuid NOT NULL, - email text NOT NULL, - username text DEFAULT ''::text NOT NULL, - hashed_password bytea NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - status user_status DEFAULT 'dormant'::user_status NOT NULL, - rbac_roles text[] DEFAULT '{}'::text[] NOT NULL, - login_type login_type DEFAULT 'password'::login_type NOT NULL, - avatar_url text DEFAULT ''::text NOT NULL, - deleted boolean DEFAULT false NOT NULL, - last_seen_at timestamp without time zone DEFAULT '0001-01-01 00:00:00'::timestamp without time zone NOT NULL, - quiet_hours_schedule text DEFAULT ''::text NOT NULL, - theme_preference text DEFAULT ''::text NOT NULL, - name text DEFAULT ''::text NOT NULL, - github_com_user_id bigint -); - -COMMENT ON COLUMN users.quiet_hours_schedule IS 'Daily (!) cron schedule (with optional CRON_TZ) signifying the start of the user''s quiet hours. If empty, the default quiet hours on the instance is used instead.'; - -COMMENT ON COLUMN users.theme_preference IS '"" can be interpreted as "the user does not care", falling back to the default theme'; - -COMMENT ON COLUMN users.name IS 'Name of the Coder user'; - -COMMENT ON COLUMN users.github_com_user_id IS 'The GitHub.com numerical user ID. At time of implementation, this is used to check if the user has starred the Coder repository.'; - CREATE VIEW visible_users AS SELECT users.id, users.username, diff --git a/coderd/database/migrations/000242_group_members_view.down.sql b/coderd/database/migrations/000242_group_members_view.down.sql new file mode 100644 index 0000000000000..99d64047d1211 --- /dev/null +++ b/coderd/database/migrations/000242_group_members_view.down.sql @@ -0,0 +1 @@ +DROP VIEW group_members_expanded; diff --git a/coderd/database/migrations/000242_group_members_view.up.sql b/coderd/database/migrations/000242_group_members_view.up.sql new file mode 100644 index 0000000000000..bbc664f6dc6cb --- /dev/null +++ b/coderd/database/migrations/000242_group_members_view.up.sql @@ -0,0 +1,40 @@ +CREATE VIEW + group_members_expanded +AS +-- If the group is a user made group, then we need to check the group_members table. +-- If it is the "Everyone" group, then we need to check the organization_members table. +WITH all_members AS ( + SELECT user_id, group_id FROM group_members + UNION + SELECT user_id, organization_id AS group_id FROM organization_members +) +SELECT + users.id AS user_id, + users.email AS user_email, + users.username AS user_username, + users.hashed_password AS user_hashed_password, + users.created_at AS user_created_at, + users.updated_at AS user_updated_at, + users.status AS user_status, + users.rbac_roles AS user_rbac_roles, + users.login_type AS user_login_type, + users.avatar_url AS user_avatar_url, + users.deleted AS user_deleted, + users.last_seen_at AS user_last_seen_at, + users.quiet_hours_schedule AS user_quiet_hours_schedule, + users.theme_preference AS user_theme_preference, + users.name AS user_name, + users.github_com_user_id AS user_github_com_user_id, + groups.organization_id AS organization_id, + groups.name AS group_name, + all_members.group_id AS group_id +FROM + all_members +JOIN + users ON users.id = all_members.user_id +JOIN + groups ON groups.id = all_members.group_id +WHERE + users.deleted = 'false'; + +COMMENT ON VIEW group_members_expanded IS 'Joins group members with user information, organization ID, group name. Includes both regular group members and organization members (as part of the "Everyone" group).'; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 775000ac6ba05..ac6d7e656d4d6 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -12,6 +12,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" ) type WorkspaceStatus string @@ -75,18 +76,18 @@ func (m OrganizationMember) Auditable(username string) AuditableOrganizationMemb type AuditableGroup struct { Group - Members []GroupMember `json:"members"` + Members []GroupMemberTable `json:"members"` } // Auditable returns an object that can be used in audit logs. // Covers both group and group member changes. -func (g Group) Auditable(users []User) AuditableGroup { - members := make([]GroupMember, 0, len(users)) - for _, u := range users { - members = append(members, GroupMember{ - UserID: u.ID, - GroupID: g.ID, - }) +func (g Group) Auditable(members []GroupMember) AuditableGroup { + membersTable := make([]GroupMemberTable, len(members)) + for i, member := range members { + membersTable[i] = GroupMemberTable{ + UserID: member.UserID, + GroupID: member.GroupID, + } } // consistent ordering @@ -96,7 +97,7 @@ func (g Group) Auditable(users []User) AuditableGroup { return AuditableGroup{ Group: g, - Members: members, + Members: membersTable, } } @@ -173,7 +174,17 @@ func (v TemplateVersion) RBACObjectNoTemplate() rbac.Object { func (g Group) RBACObject() rbac.Object { return rbac.ResourceGroup.WithID(g.ID). - InOrg(g.OrganizationID) + InOrg(g.OrganizationID). + // Group members can read the group. + WithGroupACL(map[string][]policy.Action{ + g.ID.String(): { + policy.ActionRead, + }, + }) +} + +func (gm GroupMember) RBACObject() rbac.Object { + return rbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String()) } func (w GetWorkspaceByAgentIDRow) RBACObject() rbac.Object { diff --git a/coderd/database/models.go b/coderd/database/models.go index 4bd012e258fbd..a120d16cf9dc8 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2054,7 +2054,30 @@ type Group struct { Source GroupSource `db:"source" json:"source"` } +// Joins group members with user information, organization ID, group name. Includes both regular group members and organization members (as part of the "Everyone" group). type GroupMember struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + UserEmail string `db:"user_email" json:"user_email"` + UserUsername string `db:"user_username" json:"user_username"` + UserHashedPassword []byte `db:"user_hashed_password" json:"user_hashed_password"` + UserCreatedAt time.Time `db:"user_created_at" json:"user_created_at"` + UserUpdatedAt time.Time `db:"user_updated_at" json:"user_updated_at"` + UserStatus UserStatus `db:"user_status" json:"user_status"` + UserRbacRoles []string `db:"user_rbac_roles" json:"user_rbac_roles"` + UserLoginType LoginType `db:"user_login_type" json:"user_login_type"` + UserAvatarUrl string `db:"user_avatar_url" json:"user_avatar_url"` + UserDeleted bool `db:"user_deleted" json:"user_deleted"` + UserLastSeenAt time.Time `db:"user_last_seen_at" json:"user_last_seen_at"` + UserQuietHoursSchedule string `db:"user_quiet_hours_schedule" json:"user_quiet_hours_schedule"` + UserThemePreference string `db:"user_theme_preference" json:"user_theme_preference"` + UserName string `db:"user_name" json:"user_name"` + UserGithubComUserID sql.NullInt64 `db:"user_github_com_user_id" json:"user_github_com_user_id"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + GroupName string `db:"group_name" json:"group_name"` + GroupID uuid.UUID `db:"group_id" json:"group_id"` +} + +type GroupMemberTable struct { UserID uuid.UUID `db:"user_id" json:"user_id"` GroupID uuid.UUID `db:"group_id" json:"group_id"` } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 9feb5266cc2a2..9ddbf2a74ddf6 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -146,9 +146,11 @@ type sqlcQuerier interface { GetGroupByID(ctx context.Context, id uuid.UUID) (Group, error) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrgAndNameParams) (Group, error) GetGroupMembers(ctx context.Context) ([]GroupMember, error) - // If the group is a user made group, then we need to check the group_members table. - // If it is the "Everyone" group, then we need to check the organization_members table. - GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]User, error) + GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]GroupMember, error) + // Returns the total count of members in a group. Shows the total + // count even if the caller does not have read access to ResourceGroupMember. + // They only need ResourceGroup read access. + GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) GetGroups(ctx context.Context) ([]Group, error) GetGroupsByOrganizationAndUserID(ctx context.Context, arg GetGroupsByOrganizationAndUserIDParams) ([]Group, error) GetGroupsByOrganizationID(ctx context.Context, organizationID uuid.UUID) ([]Group, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ba8129584ccda..2de986f9471f9 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1322,7 +1322,7 @@ func (q *sqlQuerier) DeleteGroupMemberFromGroup(ctx context.Context, arg DeleteG } const getGroupMembers = `-- name: GetGroupMembers :many -SELECT user_id, group_id FROM group_members +SELECT user_id, user_email, user_username, user_hashed_password, user_created_at, user_updated_at, user_status, user_rbac_roles, user_login_type, user_avatar_url, user_deleted, user_last_seen_at, user_quiet_hours_schedule, user_theme_preference, user_name, user_github_com_user_id, organization_id, group_name, group_id FROM group_members_expanded ` func (q *sqlQuerier) GetGroupMembers(ctx context.Context) ([]GroupMember, error) { @@ -1334,7 +1334,27 @@ func (q *sqlQuerier) GetGroupMembers(ctx context.Context) ([]GroupMember, error) var items []GroupMember for rows.Next() { var i GroupMember - if err := rows.Scan(&i.UserID, &i.GroupID); err != nil { + if err := rows.Scan( + &i.UserID, + &i.UserEmail, + &i.UserUsername, + &i.UserHashedPassword, + &i.UserCreatedAt, + &i.UserUpdatedAt, + &i.UserStatus, + pq.Array(&i.UserRbacRoles), + &i.UserLoginType, + &i.UserAvatarUrl, + &i.UserDeleted, + &i.UserLastSeenAt, + &i.UserQuietHoursSchedule, + &i.UserThemePreference, + &i.UserName, + &i.UserGithubComUserID, + &i.OrganizationID, + &i.GroupName, + &i.GroupID, + ); err != nil { return nil, err } items = append(items, i) @@ -1349,57 +1369,38 @@ func (q *sqlQuerier) GetGroupMembers(ctx context.Context) ([]GroupMember, error) } const getGroupMembersByGroupID = `-- name: GetGroupMembersByGroupID :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, users.quiet_hours_schedule, users.theme_preference, users.name, users.github_com_user_id -FROM - users -LEFT JOIN - group_members -ON - group_members.user_id = users.id AND - group_members.group_id = $1 -LEFT JOIN - organization_members -ON - organization_members.user_id = users.id AND - organization_members.organization_id = $1 -WHERE - -- In either case, the group_id will only match an org or a group. - (group_members.group_id = $1 - OR - organization_members.organization_id = $1) -AND - users.deleted = 'false' +SELECT user_id, user_email, user_username, user_hashed_password, user_created_at, user_updated_at, user_status, user_rbac_roles, user_login_type, user_avatar_url, user_deleted, user_last_seen_at, user_quiet_hours_schedule, user_theme_preference, user_name, user_github_com_user_id, organization_id, group_name, group_id FROM group_members_expanded WHERE group_id = $1 ` -// If the group is a user made group, then we need to check the group_members table. -// If it is the "Everyone" group, then we need to check the organization_members table. -func (q *sqlQuerier) GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]User, error) { +func (q *sqlQuerier) GetGroupMembersByGroupID(ctx context.Context, groupID uuid.UUID) ([]GroupMember, error) { rows, err := q.db.QueryContext(ctx, getGroupMembersByGroupID, groupID) if err != nil { return nil, err } defer rows.Close() - var items []User + var items []GroupMember for rows.Next() { - var i User + var i GroupMember 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, - &i.QuietHoursSchedule, - &i.ThemePreference, - &i.Name, - &i.GithubComUserID, + &i.UserID, + &i.UserEmail, + &i.UserUsername, + &i.UserHashedPassword, + &i.UserCreatedAt, + &i.UserUpdatedAt, + &i.UserStatus, + pq.Array(&i.UserRbacRoles), + &i.UserLoginType, + &i.UserAvatarUrl, + &i.UserDeleted, + &i.UserLastSeenAt, + &i.UserQuietHoursSchedule, + &i.UserThemePreference, + &i.UserName, + &i.UserGithubComUserID, + &i.OrganizationID, + &i.GroupName, + &i.GroupID, ); err != nil { return nil, err } @@ -1414,6 +1415,20 @@ func (q *sqlQuerier) GetGroupMembersByGroupID(ctx context.Context, groupID uuid. return items, nil } +const getGroupMembersCountByGroupID = `-- name: GetGroupMembersCountByGroupID :one +SELECT COUNT(*) FROM group_members_expanded WHERE group_id = $1 +` + +// Returns the total count of members in a group. Shows the total +// count even if the caller does not have read access to ResourceGroupMember. +// They only need ResourceGroup read access. +func (q *sqlQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) { + row := q.db.QueryRowContext(ctx, getGroupMembersCountByGroupID, groupID) + var count int64 + err := row.Scan(&count) + return count, err +} + const insertGroupMember = `-- name: InsertGroupMember :exec INSERT INTO group_members (user_id, group_id) @@ -1585,24 +1600,17 @@ SELECT groups.id, groups.name, groups.organization_id, groups.avatar_url, groups.quota_allowance, groups.display_name, groups.source FROM groups - -- If the group is a user made group, then we need to check the group_members table. -LEFT JOIN - group_members -ON - group_members.group_id = groups.id AND - group_members.user_id = $1 - -- If it is the "Everyone" group, then we need to check the organization_members table. -LEFT JOIN - organization_members -ON - organization_members.organization_id = groups.id AND - organization_members.user_id = $1 WHERE - -- In either case, the group_id will only match an org or a group. - (group_members.user_id = $1 OR organization_members.user_id = $1) -AND - -- Ensure the group or organization is the specified organization. - groups.organization_id = $2 + groups.id IN ( + SELECT + group_id + FROM + group_members_expanded gme + WHERE + gme.user_id = $1 + AND + gme.organization_id = $2 + ) ` type GetGroupsByOrganizationAndUserIDParams struct { diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql index 8f4770eff112e..0ef2c72323cc9 100644 --- a/coderd/database/queries/groupmembers.sql +++ b/coderd/database/queries/groupmembers.sql @@ -1,30 +1,14 @@ -- name: GetGroupMembers :many -SELECT * FROM group_members; +SELECT * FROM group_members_expanded; -- name: GetGroupMembersByGroupID :many -SELECT - users.* -FROM - users --- If the group is a user made group, then we need to check the group_members table. -LEFT JOIN - group_members -ON - group_members.user_id = users.id AND - group_members.group_id = @group_id --- If it is the "Everyone" group, then we need to check the organization_members table. -LEFT JOIN - organization_members -ON - organization_members.user_id = users.id AND - organization_members.organization_id = @group_id -WHERE - -- In either case, the group_id will only match an org or a group. - (group_members.group_id = @group_id - OR - organization_members.organization_id = @group_id) -AND - users.deleted = 'false'; +SELECT * FROM group_members_expanded WHERE group_id = @group_id; + +-- name: GetGroupMembersCountByGroupID :one +-- Returns the total count of members in a group. Shows the total +-- count even if the caller does not have read access to ResourceGroupMember. +-- They only need ResourceGroup read access. +SELECT COUNT(*) FROM group_members_expanded WHERE group_id = @group_id; -- InsertUserGroupsByName adds a user to all provided groups, if they exist. -- name: InsertUserGroupsByName :exec diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index 9dea20f0fa6e6..edd8448eacb35 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -36,25 +36,17 @@ SELECT groups.* FROM groups - -- If the group is a user made group, then we need to check the group_members table. -LEFT JOIN - group_members -ON - group_members.group_id = groups.id AND - group_members.user_id = @user_id - -- If it is the "Everyone" group, then we need to check the organization_members table. -LEFT JOIN - organization_members -ON - organization_members.organization_id = groups.id AND - organization_members.user_id = @user_id WHERE - -- In either case, the group_id will only match an org or a group. - (group_members.user_id = @user_id OR organization_members.user_id = @user_id) -AND - -- Ensure the group or organization is the specified organization. - groups.organization_id = @organization_id; - + groups.id IN ( + SELECT + group_id + FROM + group_members_expanded gme + WHERE + gme.user_id = @user_id + AND + gme.organization_id = @organization_id + ); -- name: InsertGroup :one INSERT INTO groups ( diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 2896e7035fcfa..9eeecd6b21d00 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -74,6 +74,8 @@ sql: go_type: type: "[]byte" rename: + group_member: GroupMemberTable + group_members_expanded: GroupMember template: TemplateTable template_with_name: Template workspace_build: WorkspaceBuildTable diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 7645f65c5c502..e75dd76292edb 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -93,6 +93,13 @@ var ( Type: "group", } + // ResourceGroupMember + // Valid Actions + // - "ActionRead" :: read group members + ResourceGroupMember = Object{ + Type: "group_member", + } + // ResourceLicense // Valid Actions // - "ActionCreate" :: create a license @@ -287,6 +294,7 @@ func AllResources() []Objecter { ResourceDeploymentStats, ResourceFile, ResourceGroup, + ResourceGroupMember, ResourceLicense, ResourceNotificationPreference, ResourceNotificationTemplate, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 54dcbe358007b..398cec2c829b0 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -149,6 +149,11 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionUpdate: actDef("update a group"), }, }, + "group_member": { + Actions: map[Action]ActionDefinition{ + ActionRead: actDef("read group members"), + }, + }, "file": { Actions: map[Action]ActionDefinition{ ActionCreate: actDef("create a file"), diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 4511111feded6..14f797ca0b4ee 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -301,10 +301,11 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Site: Permissions(map[string][]policy.Action{ // Should be able to read all template details, even in orgs they // are not in. - ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights}, - ResourceAuditLog.Type: {policy.ActionRead}, - ResourceUser.Type: {policy.ActionRead}, - ResourceGroup.Type: {policy.ActionRead}, + ResourceTemplate.Type: {policy.ActionRead, policy.ActionViewInsights}, + ResourceAuditLog.Type: {policy.ActionRead}, + ResourceUser.Type: {policy.ActionRead}, + ResourceGroup.Type: {policy.ActionRead}, + ResourceGroupMember.Type: {policy.ActionRead}, // Allow auditors to query deployment stats and insights. ResourceDeploymentStats.Type: {policy.ActionRead}, ResourceDeploymentConfig.Type: {policy.ActionRead}, @@ -329,6 +330,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceOrganization.Type: {policy.ActionRead}, ResourceUser.Type: {policy.ActionRead}, ResourceGroup.Type: {policy.ActionRead}, + ResourceGroupMember.Type: {policy.ActionRead}, // Org roles are not really used yet, so grant the perm at the site level. ResourceOrganizationMember.Type: {policy.ActionRead}, }), @@ -351,6 +353,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Full perms to manage org members ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroup.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + ResourceGroupMember.Type: {policy.ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -461,6 +464,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceAssignOrgRole.Type: {policy.ActionAssign, policy.ActionDelete, policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, ResourceGroup.Type: ResourceGroup.AvailableActions(), + ResourceGroupMember.Type: ResourceGroupMember.AvailableActions(), }), }, User: []Permission{}, @@ -480,6 +484,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Assigning template perms requires this permission. ResourceOrganizationMember.Type: {policy.ActionRead}, ResourceGroup.Type: {policy.ActionRead}, + ResourceGroupMember.Type: {policy.ActionRead}, }), }, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 9d71629d95d9f..b3c81564ff941 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -112,6 +112,7 @@ func TestRolePermissions(t *testing.T) { // Subjects to user memberMe := authSubject{Name: "member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember()}}} orgMemberMe := authSubject{Name: "org_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}}} + groupMemberMe := authSubject{Name: "group_member_me", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgMember(orgID)}, Groups: []string{groupID.String()}}} owner := authSubject{Name: "owner", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleOwner()}}} templateAdmin := authSubject{Name: "template-admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleTemplateAdmin()}}} @@ -382,21 +383,47 @@ func TestRolePermissions(t *testing.T) { }, }, { - Name: "Groups", - Actions: []policy.Action{policy.ActionCreate, policy.ActionDelete, policy.ActionUpdate}, - Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID), + Name: "Groups", + Actions: []policy.Action{policy.ActionCreate, policy.ActionDelete, policy.ActionUpdate}, + Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID).WithGroupACL(map[string][]policy.Action{ + groupID.String(): { + policy.ActionRead, + }, + }), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, orgUserAdmin}, - false: {setOtherOrg, memberMe, orgMemberMe, templateAdmin, orgTemplateAdmin, orgAuditor}, + false: {setOtherOrg, memberMe, orgMemberMe, templateAdmin, orgTemplateAdmin, orgAuditor, groupMemberMe}, + }, + }, + { + Name: "GroupsRead", + Actions: []policy.Action{policy.ActionRead}, + Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID).WithGroupACL(map[string][]policy.Action{ + groupID.String(): { + policy.ActionRead, + }, + }), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, groupMemberMe}, + false: {setOtherOrg, memberMe, orgMemberMe, orgAuditor}, }, }, { - Name: "GroupsRead", + Name: "GroupMemberMeRead", Actions: []policy.Action{policy.ActionRead}, - Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID), + Resource: rbac.ResourceGroupMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgMemberMe, groupMemberMe}, + false: {setOtherOrg, memberMe, orgAuditor}, + }, + }, + { + Name: "GroupMemberOtherRead", + Actions: []policy.Action{policy.ActionRead}, + Resource: rbac.ResourceGroupMember.WithID(adminID).InOrg(orgID).WithOwner(adminID.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin}, - false: {setOtherOrg, memberMe, orgMemberMe, orgAuditor}, + false: {setOtherOrg, memberMe, orgAuditor, orgMemberMe, groupMemberMe}, }, }, { diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 2eff919ddc63d..fd9f4752bff51 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -49,14 +49,14 @@ func TestTelemetry(t *testing.T) { Provisioner: database.ProvisionerTypeTerraform, }) _ = dbgen.TemplateVersion(t, db, database.TemplateVersion{}) - _ = dbgen.User(t, db, database.User{}) + user := dbgen.User(t, db, database.User{}) _ = dbgen.Workspace(t, db, database.Workspace{}) _ = dbgen.WorkspaceApp(t, db, database.WorkspaceApp{ SharingLevel: database.AppSharingLevelOwner, Health: database.WorkspaceAppHealthDisabled, }) - _ = dbgen.Group(t, db, database.Group{}) - _ = dbgen.GroupMember(t, db, database.GroupMember{}) + group := dbgen.Group(t, db, database.Group{}) + _ = dbgen.GroupMember(t, db, database.GroupMemberTable{UserID: user.ID, GroupID: group.ID}) wsagent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{}) // Update the workspace agent to have a valid subsystem. err = db.UpdateWorkspaceAgentStartupByID(ctx, database.UpdateWorkspaceAgentStartupByIDParams{ @@ -94,7 +94,8 @@ func TestTelemetry(t *testing.T) { require.Len(t, snapshot.TemplateVersions, 1) require.Len(t, snapshot.Users, 1) require.Len(t, snapshot.Groups, 2) - require.Len(t, snapshot.GroupMembers, 1) + // 1 member in the everyone group + 1 member in the custom group + require.Len(t, snapshot.GroupMembers, 2) require.Len(t, snapshot.Workspaces, 1) require.Len(t, snapshot.WorkspaceApps, 1) require.Len(t, snapshot.WorkspaceAgents, 1) diff --git a/codersdk/groups.go b/codersdk/groups.go index 4b5b8f5a5f4e6..fdd7217200738 100644 --- a/codersdk/groups.go +++ b/codersdk/groups.go @@ -30,9 +30,13 @@ type Group struct { DisplayName string `json:"display_name"` OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` Members []ReducedUser `json:"members"` - AvatarURL string `json:"avatar_url"` - QuotaAllowance int `json:"quota_allowance"` - Source GroupSource `json:"source"` + // How many members are in this group. Shows the total count, + // even if the user is not authorized to read group member details. + // May be greater than `len(Group.Members)`. + TotalMemberCount int `json:"total_member_count"` + AvatarURL string `json:"avatar_url"` + QuotaAllowance int `json:"quota_allowance"` + Source GroupSource `json:"source"` } func (g Group) IsEveryone() bool { diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 788cab912643b..67a24bf73e04a 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -14,6 +14,7 @@ const ( ResourceDeploymentStats RBACResource = "deployment_stats" ResourceFile RBACResource = "file" ResourceGroup RBACResource = "group" + ResourceGroupMember RBACResource = "group_member" ResourceLicense RBACResource = "license" ResourceNotificationPreference RBACResource = "notification_preference" ResourceNotificationTemplate RBACResource = "notification_template" @@ -65,6 +66,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceDeploymentStats: {ActionRead}, ResourceFile: {ActionCreate, ActionRead}, ResourceGroup: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceGroupMember: {ActionRead}, ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, ResourceNotificationPreference: {ActionRead, ActionUpdate}, ResourceNotificationTemplate: {ActionRead, ActionUpdate}, diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index be30b790d4aef..c0d7ff7a852c7 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -219,7 +219,8 @@ curl -X GET http://coder-server:8080/api/v2/groups/{group} \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` @@ -277,7 +278,8 @@ curl -X DELETE http://coder-server:8080/api/v2/groups/{group} \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` @@ -350,7 +352,8 @@ curl -X PATCH http://coder-server:8080/api/v2/groups/{group} \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` @@ -1108,7 +1111,8 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ] ``` @@ -1123,28 +1127,29 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups Status Code **200** -| Name | Type | Required | Restrictions | Description | -| --------------------- | ------------------------------------------------------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» avatar_url` | string | false | | | -| `» display_name` | string | false | | | -| `» id` | string(uuid) | false | | | -| `» members` | array | false | | | -| `»» avatar_url` | string(uri) | false | | | -| `»» created_at` | string(date-time) | true | | | -| `»» email` | string(email) | true | | | -| `»» id` | string(uuid) | true | | | -| `»» last_seen_at` | string(date-time) | false | | | -| `»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | -| `»» name` | string | false | | | -| `»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | -| `»» theme_preference` | string | false | | | -| `»» updated_at` | string(date-time) | false | | | -| `»» username` | string | true | | | -| `» name` | string | false | | | -| `» organization_id` | string(uuid) | false | | | -| `» quota_allowance` | integer | false | | | -| `» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | +| Name | Type | Required | Restrictions | Description | +| ---------------------- | ------------------------------------------------------ | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `[array item]` | array | false | | | +| `» avatar_url` | string | false | | | +| `» display_name` | string | false | | | +| `» id` | string(uuid) | false | | | +| `» members` | array | false | | | +| `»» avatar_url` | string(uri) | false | | | +| `»» created_at` | string(date-time) | true | | | +| `»» email` | string(email) | true | | | +| `»» id` | string(uuid) | true | | | +| `»» last_seen_at` | string(date-time) | false | | | +| `»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | +| `»» name` | string | false | | | +| `»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | +| `»» theme_preference` | string | false | | | +| `»» updated_at` | string(date-time) | false | | | +| `»» username` | string | true | | | +| `» name` | string | false | | | +| `» organization_id` | string(uuid) | false | | | +| `» quota_allowance` | integer | false | | | +| `» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | +| `» total_member_count` | integer | false | | How many members are in this group. Shows the total count, even if the user is not authorized to read group member details. May be greater than `len(Group.Members)`. | #### Enumerated Values @@ -1222,7 +1227,8 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/groups "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` @@ -1281,7 +1287,8 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups/ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` @@ -1987,7 +1994,8 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ], "users": [ @@ -2019,30 +2027,31 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| ---------------------- | ------------------------------------------------------ | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» groups` | array | false | | | -| `»» avatar_url` | string | false | | | -| `»» display_name` | string | false | | | -| `»» id` | string(uuid) | false | | | -| `»» members` | array | false | | | -| `»»» avatar_url` | string(uri) | false | | | -| `»»» created_at` | string(date-time) | true | | | -| `»»» email` | string(email) | true | | | -| `»»» id` | string(uuid) | true | | | -| `»»» last_seen_at` | string(date-time) | false | | | -| `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | -| `»»» name` | string | false | | | -| `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | -| `»»» theme_preference` | string | false | | | -| `»»» updated_at` | string(date-time) | false | | | -| `»»» username` | string | true | | | -| `»» name` | string | false | | | -| `»» organization_id` | string(uuid) | false | | | -| `»» quota_allowance` | integer | false | | | -| `»» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | -| `» users` | array | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------------- | ------------------------------------------------------ | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `[array item]` | array | false | | | +| `» groups` | array | false | | | +| `»» avatar_url` | string | false | | | +| `»» display_name` | string | false | | | +| `»» id` | string(uuid) | false | | | +| `»» members` | array | false | | | +| `»»» avatar_url` | string(uri) | false | | | +| `»»» created_at` | string(date-time) | true | | | +| `»»» email` | string(email) | true | | | +| `»»» id` | string(uuid) | true | | | +| `»»» last_seen_at` | string(date-time) | false | | | +| `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | +| `»»» name` | string | false | | | +| `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | +| `»»» theme_preference` | string | false | | | +| `»»» updated_at` | string(date-time) | false | | | +| `»»» username` | string | true | | | +| `»» name` | string | false | | | +| `»» organization_id` | string(uuid) | false | | | +| `»» quota_allowance` | integer | false | | | +| `»» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | +| `»» total_member_count` | integer | false | | How many members are in this group. Shows the total count, even if the user is not authorized to read group member details. May be greater than `len(Group.Members)`. | +| `» users` | array | false | | | #### Enumerated Values diff --git a/docs/api/members.md b/docs/api/members.md index 472d3dcc90d43..cecb22340fe99 100644 --- a/docs/api/members.md +++ b/docs/api/members.md @@ -189,6 +189,7 @@ Status Code **200** | `resource_type` | `deployment_stats` | | `resource_type` | `file` | | `resource_type` | `group` | +| `resource_type` | `group_member` | | `resource_type` | `license` | | `resource_type` | `notification_preference` | | `resource_type` | `notification_template` | @@ -346,6 +347,7 @@ Status Code **200** | `resource_type` | `deployment_stats` | | `resource_type` | `file` | | `resource_type` | `group` | +| `resource_type` | `group_member` | | `resource_type` | `license` | | `resource_type` | `notification_preference` | | `resource_type` | `notification_template` | @@ -472,6 +474,7 @@ Status Code **200** | `resource_type` | `deployment_stats` | | `resource_type` | `file` | | `resource_type` | `group` | +| `resource_type` | `group_member` | | `resource_type` | `license` | | `resource_type` | `notification_preference` | | `resource_type` | `notification_template` | @@ -728,6 +731,7 @@ Status Code **200** | `resource_type` | `deployment_stats` | | `resource_type` | `file` | | `resource_type` | `group` | +| `resource_type` | `group_member` | | `resource_type` | `license` | | `resource_type` | `notification_preference` | | `resource_type` | `notification_template` | diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 1ece64c0c6a40..a7e5120a1b338 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -246,7 +246,8 @@ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ], "users": [ @@ -2806,22 +2807,24 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "quota_allowance": 0, - "source": "user" + "source": "user", + "total_member_count": 0 } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | ----------------------------------------------------- | -------- | ------------ | ----------- | -| `avatar_url` | string | false | | | -| `display_name` | string | false | | | -| `id` | string | false | | | -| `members` | array of [codersdk.ReducedUser](#codersdkreduceduser) | false | | | -| `name` | string | false | | | -| `organization_id` | string | false | | | -| `quota_allowance` | integer | false | | | -| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | +| Name | Type | Required | Restrictions | Description | +| -------------------- | ----------------------------------------------------- | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `avatar_url` | string | false | | | +| `display_name` | string | false | | | +| `id` | string | false | | | +| `members` | array of [codersdk.ReducedUser](#codersdkreduceduser) | false | | | +| `name` | string | false | | | +| `organization_id` | string | false | | | +| `quota_allowance` | integer | false | | | +| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | +| `total_member_count` | integer | false | | How many members are in this group. Shows the total count, even if the user is not authorized to read group member details. May be greater than `len(Group.Members)`. | ## codersdk.GroupSource @@ -4267,6 +4270,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `deployment_stats` | | `file` | | `group` | +| `group_member` | | `license` | | `notification_preference` | | `notification_template` | diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 0b027f21ff2e0..c6fdd67cd029b 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -77,10 +77,10 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) return } - var emptyUsers []database.User - aReq.New = group.Auditable(emptyUsers) + var emptyMembers []database.GroupMember + aReq.New = group.Auditable(emptyMembers) - httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Group(group, nil)) + httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Group(group, nil, 0)) } // @Summary Update group by name @@ -285,7 +285,13 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { aReq.New = group.Auditable(patchedMembers) - httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers)) + memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers, int(memberCount))) } // @Summary Delete group by name @@ -370,7 +376,13 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users)) + memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users, int(memberCount))) } // @Summary Get groups by organization @@ -414,8 +426,13 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { httpapi.InternalServerError(rw, err) return } + memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } - resp = append(resp, db2sdk.Group(group, members)) + resp = append(resp, db2sdk.Group(group, members, int(memberCount))) } httpapi.Write(ctx, rw, http.StatusOK, resp) diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 4d84a24601b1a..73e114f6239d3 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -2,6 +2,7 @@ package coderd_test import ( "net/http" + "sort" "testing" "github.com/google/uuid" @@ -567,6 +568,12 @@ func TestPatchGroup(t *testing.T) { }) } +func sortGroupMembers(group *codersdk.Group) { + sort.Slice(group.Members, func(i, j int) bool { + return group.Members[i].ID.String() < group.Members[j].ID.String() + }) +} + // TODO: test auth. func TestGroup(t *testing.T) { t.Parallel() @@ -638,6 +645,9 @@ func TestGroup(t *testing.T) { ggroup, err := userAdminClient.Group(ctx, group.ID) require.NoError(t, err) + sortGroupMembers(&group) + sortGroupMembers(&ggroup) + require.Equal(t, group, ggroup) }) @@ -820,6 +830,14 @@ func TestGroups(t *testing.T) { groups, err := userAdminClient.GroupsByOrganization(ctx, user.OrganizationID) require.NoError(t, err) + + // sort group members so we can compare them + allGroups := append([]codersdk.Group{}, groups...) + allGroups = append(allGroups, group1, group2) + for i := range allGroups { + sortGroupMembers(&allGroups[i]) + } + // 'Everyone' group + 2 custom groups. require.Len(t, groups, 3) require.Contains(t, groups, group1) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 9531125d7ceb1..fb61f0b3c494d 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -64,8 +64,13 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req httpapi.InternalServerError(rw, err) return } + memberCount, err := api.Database.GetGroupMembersCountByGroupID(ctx, group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } - sdkGroups = append(sdkGroups, db2sdk.Group(group, members)) + sdkGroups = append(sdkGroups, db2sdk.Group(group, members, int(memberCount))) } httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{ @@ -121,7 +126,7 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { groups := make([]codersdk.TemplateGroup, 0, len(dbGroups)) for _, group := range dbGroups { - var members []database.User + var members []database.GroupMember // This is a bit of a hack. The caller might not have permission to do this, // but they can read the acl list if the function got this far. So we let @@ -133,8 +138,14 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { httpapi.InternalServerError(rw, err) return } + // nolint:gocritic + memberCount, err := api.Database.GetGroupMembersCountByGroupID(dbauthz.AsSystemRestricted(ctx), group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } groups = append(groups, codersdk.TemplateGroup{ - Group: db2sdk.Group(group.Group, members), + Group: db2sdk.Group(group.Group, members, int(memberCount)), Role: convertToTemplateRole(group.Actions), }) } diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index 6cee389dfbc7a..0f9578aa226bf 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -50,6 +50,9 @@ export const RBACResourceActions: Partial< read: "read groups", update: "update a group", }, + group_member: { + read: "read group members", + }, license: { create: "create a license", delete: "delete license", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 27d310cb83515..370b4f1f90c76 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -613,6 +613,7 @@ export interface Group { readonly display_name: string; readonly organization_id: string; readonly members: readonly ReducedUser[]; + readonly total_member_count: number; readonly avatar_url: string; readonly quota_allowance: number; readonly source: GroupSource; @@ -2302,6 +2303,7 @@ export type RBACResource = | "deployment_stats" | "file" | "group" + | "group_member" | "license" | "notification_preference" | "notification_template" @@ -2331,6 +2333,7 @@ export const RBACResources: RBACResource[] = [ "deployment_stats", "file", "group", + "group_member", "license", "notification_preference", "notification_template", diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.tsx index bc544fe1fab30..daccaa2dc01b8 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountUserGroups.tsx @@ -57,8 +57,8 @@ export const AccountUserGroups: FC = ({ header={group.display_name || group.name} subtitle={ <> - {group.members.length} member - {group.members.length !== 1 && "s"} + {group.total_member_count} member + {group.total_member_count !== 1 && "s"} } /> diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 288bd3d708fb7..ab665709fed45 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -2499,6 +2499,7 @@ export const MockGroup: TypesGen.Group = { members: [MockUser, MockUser2], quota_allowance: 5, source: "user", + total_member_count: 2, }; const everyOneGroup = (organizationId: string): TypesGen.Group => ({ @@ -2510,6 +2511,7 @@ const everyOneGroup = (organizationId: string): TypesGen.Group => ({ avatar_url: "", quota_allowance: 0, source: "user", + total_member_count: 0, }); export const MockTemplateACL: TypesGen.TemplateACL = {