simplify RBAC check on GetGroupMembersCountByGroupID

hugodutka · hugodutka · commit 3930c6b32ae5 · 2024-08-09T17:36:28.000Z
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -1401,20 +1401,13 @@ func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([
 }
 
 func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
-	group, err := q.GetGroupByID(ctx, groupID)
-	if err != nil {
+	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
 	}
-	if err := q.authorizeContext(ctx, policy.ActionRead, database.GroupMembersCountRBACHelper{
-		GroupID:        groupID,
-		OrganizationID: group.OrganizationID,
-	}); err != nil {
-		return 0, err
-	}
 	return memberCount, nil
 }
 
diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -184,22 +184,20 @@ func groupRBACObject(groupID, organizationID uuid.UUID) rbac.Object {
 }
 
 func (g Group) RBACObject() rbac.Object {
-	return groupRBACObject(g.ID, g.OrganizationID)
+	return rbac.ResourceGroup.WithID(g.ID).
+		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())
 }
 
-type GroupMembersCountRBACHelper struct {
-	GroupID        uuid.UUID
-	OrganizationID uuid.UUID
-}
-
-func (r GroupMembersCountRBACHelper) RBACObject() rbac.Object {
-	return groupRBACObject(r.GroupID, r.OrganizationID)
-}
-
 func (w GetWorkspaceByAgentIDRow) RBACObject() rbac.Object {
 	return w.Workspace.RBACObject()
 }

Original file line number	Diff line number	Diff line change
`@@ -1401,20 +1401,13 @@ func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([`
`1401`	`1401`	`}`
`1402`	`1402`
`1403`	`1403`	`func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {`
`1404`		`- group, err := q.GetGroupByID(ctx, groupID)`
`1405`		`- if err != nil {`
	`1404`	`+ if _, err := q.GetGroupByID(ctx, groupID); err != nil { // AuthZ check`
`1406`	`1405`	`return 0, err`
`1407`	`1406`	`}`
`1408`	`1407`	`memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, groupID)`
`1409`	`1408`	`if err != nil {`
`1410`	`1409`	`return 0, err`
`1411`	`1410`	`}`
`1412`		`- if err := q.authorizeContext(ctx, policy.ActionRead, database.GroupMembersCountRBACHelper{`
`1413`		`- GroupID: groupID,`
`1414`		`- OrganizationID: group.OrganizationID,`
`1415`		`- }); err != nil {`
`1416`		`- return 0, err`
`1417`		`- }`
`1418`	`1411`	`return memberCount, nil`
`1419`	`1412`	`}`
`1420`	`1413`