Really unfortunate we cannot just use the/members endpoint without it being a breaking change.

Since organizations are new, I think it's worth considering.

I was originally going to to augment the existing route to be paginated but@aslilac brought up the issue of backwards compatibility. I can circle back with her, and other stakeholders on this

if we were having this discussion like two weeks ago I think I'd agree with you@Emyrk, but 2.20 is out now and marketing organizations as stable. :\

Unfortunately this does not work.

By default, amember 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 theauthz check, and you would get an empty set as your return.

You would need tooffset = 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 anddirty

The first is the quick solution. We just restrict access to this endpoint to users who can read the entire table (withorganization_id = @org_id). This functionally changes the query to only usable by a certain class of users. Regular members now get aForbidden 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) {iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceOrganizationMember.InOrg(arg.OrganizationID));err!=nil {returnnil,err}returnq.db.PaginatedOrganizationMembers(ctx,arg)}

Thecorrect 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 functionprepareSQLFilter actually createsWHERE clause expressions that return the truthy value of anauthorize() 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 wouldn't bother with the subrouter here

I saw this was a pattern we were doing in other places so copied that. Will change

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.

Just throw a comment

Can you add back the org member in the setup? And check it in theReturns? And set the limit to 0.

That was breaking in thedbmem becauselimit = 0 was not being treated as "no limit"

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

nice catch 😂