Unfortunately this does not work.

By default, a member can only read themselves, not other org members. So if you were paginating 10 users at a time, and the user was the 12th. Your query would return 10 rows, all would fail the authz check, and you would get an empty set as your return.

You would need to offset = 2 to find yourself, but that is not intuitive.

Pagination is a general problem for authorization engines. The user is requesting "The next 10 elements I can access". If you cannot join the authz data into the query itself then:

• Query 10 items, if some are blocked by the authz, fetch some more. Repeat until you get 10 or exhaust the table.
• Fetch all elements, then filter (large table this is not doable)
• Overfetch by some, say 50 elements. Keep filtering and re-fetching until you get 10.

These solutions kinda suck. So we have 2 methods in our codebase to solve this.

Quick and dirty

The first is the quick solution. We just restrict access to this endpoint to users who can read the entire table (with organization_id = @org_id). This functionally changes the query to only usable by a certain class of users. Regular members now get a Forbidden instead of a list of ["me"]. If we go this route, we just need to fix the page that currently displays just yourself in these cases.

func (q *querier) PaginatedOrganizationMembers(ctx context.Context, arg database.PaginatedOrganizationMembersParams) ([]database.PaginatedOrganizationMembersRow, error) {
	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceOrganizationMember.InOrg(arg.OrganizationID)); err != nil {
		return nil, err
	}
	return q.db.PaginatedOrganizationMembers(ctx, arg)
}

The correct solution

The more correct solution is to properly filter the list, so if there exists a user who can read a subset of members, this query still works.

A good example is workspaces

The function prepareSQLFilter actually creates WHERE clause expressions that return the truthy value of an authorize() call for the given user. So we apply the filter in SQL, and you can paginate through the rows you can access.

There is some extra code required, but we have precedent to copy from if we go this route.

I think we should just go the quick and dirty route in this case. We don't have any roles that grant subset access to members. It is also strange to request organization members, and only return a subset.

For workspaces/templates this makes sense, as the resources have access control. However in this case, I think it makes more sense to return all or nothing. And we can update the UI to be in line with this decision.

Thanks for the feedback/explanation! I agree that from a product perspective the quick and dirty route makes the most sense

I think this different check means we also don't need this field any more

Accidentally pushed some test code in the test so not sure which one you were looking at but that's needed.

I think this needs to check if the limitOpt is not 0. If it is zero, skip this check. To match the SQL