mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: allow group members to read group information (#14200)
* - allow group members to read basic Group info - allow group members to see they are part of the group, but not see that information about other members - add a GetGroupMembersCountByGroupID SQL query, which allows group members to see members count without revealing other information about the members - add the group_members_expanded db view - rewrite group member queries to use the group_members_expanded view - add the RBAC ResourceGroupMember and add it to relevant roles - rewrite GetGroupMembersByGroupID permission checks - make the GroupMember type contain all user fields - fix type issues coming from replacing User with GroupMember in group member queries - add the MemberTotalCount field to codersdk.Group - display `group.total_member_count` instead of `group.members.length` on the account page
This commit is contained in:
@@ -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
|
||||
}
|
||||
return q.db.GetGroupMembersByGroupID(ctx, id)
|
||||
memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, groupID)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
return memberCount, nil
|
||||
}
|
||||
|
||||
func (q *querier) GetGroups(ctx context.Context) ([]database.Group, error) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user