[WIP] MSC4375: Admin Room Management

[nexy7574]

Rendered

N.B. Assuming MSC ID 4375 as #4374 was marked as spam.

---

Implementations:

Signed-off-by: Nexus [email protected]

[TheArcaneBrony]

Sorry Nex, got quite a bit of thoughts on a few points, especially relating to how i'm using the existing equivalents and findings from Rory&::MatrixUtils, hope you don't mind :)

[TheArcaneBrony]

Couldn't this be a single endpoint? I'm not sure I see much value in having these be separate, and feels like a lot of room for error. (ie. what happens if you call /evacuate and then DELETE immediately afterwards? what happens if you PUT /blocked on a room that isnt known (/anymore)? What happens if you delete a room without evacuation?)

[nexy7574]

A single endpoint doesn't feel very RESTful but there are definitely too many endpoints. I'll see if I can squash some of them down into /remove and /block

[TheArcaneBrony]

This feels kind of backwards. A common thing i do is explicitly list empty rooms, or ie. tombstoned rooms.

Could these be tristates instead? ie. empty=true/false or omitted, also merging un/encrypted and un/federated.

Another nice touch would be able to filter for tombstoned rooms in particular (something i do as a maintenance task from time to time)

[nexy7574]

tristates are ugly and i dont like them but at this point might be a necessary evil

[TheArcaneBrony]

This has been a huge annoyance with the synapse admin API here: can we make these consistent? It gets quite annoying when you have to remember which ones are backwards when you're explicitly exposing "Ascending"/"Descending" in your UI (directly mapped to dir=f/b).

[nexy7574]

Hm, I'll make room_version newest -> oldest, and change latest_event to last_event and make it newest -> oldest also. That good?

[TheArcaneBrony]

that would sure be an improvment - though that'd still have name be backwards, no?

[TheArcaneBrony]

This feels kind of low, personally. In RMU, I query in pages of 1000, used to be 250 but was changed at some point due to performance issues regarding how little data synapse actually returns)

[nexy7574]

I'm not sure if venturing past 100 is even something people are willing to accept. The original proposal did suggest a 1000 cap, but I'd like other's input on how much is acceptable

[TheArcaneBrony]

It might be worthwhile to allow larger page sizes, especially with sql based servers that would have to SELECT * FROM rooms ORDER BY property ASCENDING SKIP 15200 LIMIT 100 otherwise - that's a lot of overhead spent on just skipping rows

[TheArcaneBrony]

This seems horribly inefficient: for ~10k rooms, this would mean 22 list requests and 10k room details requests to render a table of rooms (without pagination, as done in Rory&::MatrixUtils), but most likely also common in ie. bots.

Could this endpoint return something more like a summary of each room, like how synapse does it, or whatever the room info endpoint returns?

[Gnuxie]

The fields I'd include would be the create event, the admin information about the room (block status etc), and then maybe some cached information. It'd be good to keep these under distinct properties as the data is from different sources and some of it is just derived from a cached summary of current room state.

{
  "room_id": "...",
  "admin_info": "...",
  "create_event": "...",
  "current_state": "..."
}

Each property in current_state may be undefined and unavailable. create_event and admin_info should be required. Because they should always be available before presenting a room to the API¹.

• invited: The number of m.room.member events with a membership of invite in the room. This is useful for spotting rooms with lots of invitations being sent to them.
• joined: The total number of m.room.member events with a membership of join. Useful for gauging the size of the room for triaging reports or action.
• join_rules: It may be important to determine whether the room is private or public
• m.room.name : Important to have access to the room name content. Included rather than the field in case the event changes later but probably doesn't matter here
• m.room.topic: Important to have access to the topic event content https://spec.matrix.org/latest/client-server-api/#mroomtopic. This one does need to be the content since now it is extensible and has rich text.
• m.room.avatar: https://spec.matrix.org/latest/client-server-api/#mroomavatar

Footnotes

1. Otherwise there are big security implications. ↩

[Gnuxie]

This might be better as "room_summary" for things like the invited and joined membership counts. And then just use current state as something that can be filled in with a filter.

[TheArcaneBrony]

For most rooms, wouldn't it make more sense to just make this return all room state instead? That seems more useful than just including members.

[TheArcaneBrony]

Requiring the user to be local kind of feels like a needless restriction - and is something that is used quite often on the synapse admin API.

Notably: I use this quite often through Draupnir, to gain temporary access to do x or y. This draupnir instance isn't always hosted on my homeserver, and this would be a major restriction for me.

[TheArcaneBrony]

/remove_admin endpoint?

[nexy7574]

Doesn't resolve the problem

[TheArcaneBrony]

Shouldn't it be advertised via /capabilities instead?

[nexy7574]

Not really, unlike the user suspend/lock endpoints, you either do or don't have access to these endpoints, not only some of them. Once it becomes stable you can just try the endpoints with the assumption they'll fail if you're not a server admin.
The only thing I can see this being useful for is allowing clients to pre-determine their admin status via capabilities for rendering, but rooms aren't usually managed via a client UI so this seems like more of an edge case.

The versions advertisement is only while it is unstable, I highlighted the "OPTIONALLY only to authenticated users." to remind people that the endpoint can return different information for authenticated users

[Gnuxie]

I strongly recommend against copying the synapse room list ordering because it is "gappy". Rooms discovered or created during pagination may be missed by the consumer. And if they want to keep looking for new rooms, they have to start over the entire stream again.

What I do recommend instead is to have the streaming token be based on "room encounter time". Where a room encounter is defined as when we have received the m.room.create PDU for for the first time. (Which in turn is triggered by things like receiving an event or an invitation from the room for the first time)

[Gnuxie]

Then consumers can just paginate the entire list, and then poll forwards until more rooms are discovered by the homeserver.

[Gnuxie]

It might still be possible to provide custom orders ontop of this if the canonical order still is the room encounter time. And you just paginate with a custom order within a limit. e.g. /rooms?dir=f&to=token_obtained_when_pagination_session_started&order_by=member_count

[Gnuxie]

That way they can follow up with another pagination session /rooms?dir=f&to=new_token_obtained_when_this_pagination_session_started&from=to_token_from_previous_session_example&order_by=member_count with no gaps.

[nexy7574]

In theory, the pagination token could just be a timestamp, which would still allow custom ordering 🤔

[Gnuxie]

So a distinct problem that is related to this is that ordering by mutable fields in pagination always leads to a problem where entries can disappear or appear more than once throughout the session. I think we just have to accept that and document the phenomenon as part of the endpoint. Most of the orders offered by the synapse admin api are on mutable fields and are unstable. So I will be happy with just a stable order by encounter time being offered for tools like Draupnir that need to be able to collect the entire list.

Or in other words, there's no need to rebase the other orders on encounter time, because that wouldn't provide stability anyway as they are mutable sorts.

[Gnuxie]

We might want to also be safer for pre-v12 rooms and reference rooms in terms of their m.room.create event id rather than their room id.

[TheArcaneBrony]

I suppose this does solve the issue of purging rooms with custom room IDs that tend to run into url encoding issues

[nexy7574]

This has the problem of admins possibly not knowing the create event ID,

That's kinda irrelevant if this proposal is updated to go using create event IDs instead of room IDs in the first place but I don't really fancy doing that

[Gnuxie]

I know Synapse has this, but I don't know if we need to provide parity here. We should wait until we have a more general way for server admins to communicate with users that can be utilised.

The drawback of doing this without another way of communicating the evacuation is users being confused why a room they were in no longer exists...

[Gnuxie]

I think the room summary and room pagination apis could be split out if it's found that the evacuate/block/takeover semantics are going to require more thought and work. I am willing to champion room pagination and summary.

[Gnuxie]

the-draupnir-project/Draupnir#989 room locking and suspension might be nice to have. Happy to follow up on these.

[jevolk]

It looks like you put a lot of hard work into this MSC already but I think the proper course here is to standardize the Synapse admin API. There are a few reasons for this. It is the tradition for standards development: take something that exists or is proven and stabilize, refine details if necessary, and standardize it. An ecosystem of software already exists for the Synapse API, including bots and even several GUI interfaces used by industry. It's much more economical to reuse rather than reinvent. I think the effort and energy is good here, but it should be directed at a hard target and not outer space.

Also one other nit, it was Construct that invented the admin room 😉

[Gnuxie]

The Synapse admin api is broken and an abomination. No.

[nexy7574]

stabilize, refine details if necessary, and standardize it.

That's effectively what I'm trying to do here. I took parts of the Synapse API I liked, parts of the Dendrite API that I liked, and the UX parts of Conduit's admin commands that I liked, and tried to combine them into a middle-ground collection of routes.

The idea behind this MSC, MSC4323, and a couple other MSCs I've got in the oven, is to move away from each implementation's own versions of these APIs, to bring one uniform, standardised API that anyone can both implement and use without needing to worry about the intricacies. Part of this includes building off of prior art, rather than simply standardising it. All of the implementations right now have issues, and this proposal no doubt also will (this first draft is also more of a rough sketch than anything meant to be final), but the great thing about the spec is that anyone can review proposals, and if they don't like things that get merged into spec, they can open up followup proposals to fix what they don't like, whereas implementation specific interfaces don't tend to get that same TLC.

An ecosystem of software already exists for the Synapse API, including bots and even several GUI interfaces used by industry. It's much more economical to reuse rather than reinvent

This would require cloning the API routes verbatim, which would mean they aren't even under /_matrix at root. Even if I were to just clone Synapse's API, there'd still need to be plenty of changes to make it spec-ready.

Also one other nit, it was Construct that invented the admin room 😉

Caught red handed not doing my homework :^)