Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC2432: Updated semantics for publishing room aliases #2432

Merged
merged 9 commits into from
Mar 25, 2020
225 changes: 225 additions & 0 deletions proposals/2432-revised-alias-publishing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
# MSC2432: Updated semantics for publishing room aliases

This MSC offers an alternative to [MSC2260](https://github.com/matrix-org/matrix-doc/issues/2260).

## Background

The [`m.room.aliases`](https://matrix.org/docs/spec/client_server/r0.6.0#m-room-aliases)
state event exists to list the available aliases for a given room. This serves
two purposes:

* It allows existing members of the room to discover alternative aliases,
which may be useful for them to pass this knowledge on to others trying to
join.

* Secondarily, it helps to educate users about how Matrix works by
illustrating multiple aliases per room and giving a perception of the size
of the network.

However, it has problems:

* Any user in the entire ecosystem can create aliases for rooms, which are
then unilaterally added to `m.room.aliases`, and room admins are unable to
remove them. This is an abuse
vector (https://github.com/matrix-org/matrix-doc/issues/625).

* For various reasons, the `m.room.aliases` event tends to get out of sync
with the actual aliases (https://github.com/matrix-org/matrix-doc/issues/2262).

## Proposal

We propose that that room moderators should be able to manually curate a list
of "official" aliases for their room, instead of matrix servers automatically
publishing lists of all room aliases into the room state. No particular
guarantees are offered that this alias list is entirely accurate: it becomes
room moderators' responsibilty to keep it so.
richvdh marked this conversation as resolved.
Show resolved Hide resolved

Meanwhile, the aliases that map to a given room on a given server become
the ultimate responsibility of the administrators of that server. We give them
tools to inspect the alias list and clean it up when necessary, in addition to
the current tools which allow restriction of who can create aliases in the
first place.

A detailed list of proposed modifications to the Matrix spec follows:

* `m.room.aliases` loses any special meaning within the spec. In particular:
richvdh marked this conversation as resolved.
Show resolved Hide resolved

* Clients should no longer format it specially in room timelines.

* Clients and servers should no longer consider `m.room.aliases` when
[calculating the display name for a
room](https://matrix.org/docs/spec/client_server/r0.6.0#calculating-the-display-name-for-a-room).

(Note: servers follow the room display-name algorithm when calculating
room names for certain types of push notification.)

* A future room version will remove the special [authorization
rules](https://matrix.org/docs/spec/rooms/v1#authorization-rules) and
[redaction rules](https://matrix.org/docs/spec/client_server/r0.6.0#redactions).

* [`m.room.canonical_alias`](https://matrix.org/docs/spec/client_server/r0.6.0#m-room-canonical-alias)
is extended to include a new `alt_aliases` property. This, if present,
should be a list of alternative aliases for the room. An example event might
look like:

```json
{
"content": {
"alias": "#somewhere:localhost",
"alt_aliases": [
"#somewhere:overthere.com",
"#somewhereelse:example.com"
]
},
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
"state_key": "",
"type": "m.room.canonical_alias"
}
```

It is valid for `alt_aliases` to be non-empty even if `alias` is absent or
empty. This means that no alias has been picked out as the 'main' alias.

(Note: although the spec currently claims that `alias` is mandatory, Synapse
generates `m.room.canonical` alias events with no `alias` property when the
richvdh marked this conversation as resolved.
Show resolved Hide resolved
main alias is deleted. This change would legitimise that behaviour.)

(For clarity: it is not proposed that the `alt_aliases` be considered when
calculating the displayname for a room.)

* [`PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}`](https://matrix.org/docs/spec/client_server/r0.6.0#put-matrix-client-r0-rooms-roomid-state-eventtype-statekey)
is extended to recommend that servers validate any *new* aliases added to
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: spec an error code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the synapse implementation (matrix-org/synapse#6971) I currently chose M_INVALID_PARAM if the form of the data is incorrect, and M_NOT_FOUND if the alias does not point to a room. Unfortunately M_NOT_FOUND is also used for if the alias itself does not exist.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M_NOT_FOUND doesn't feel like a good match to me. We don't actually know if the underlying resource exists or not (it probably does).

I'd go with something like M_BAD_ALIAS or M_ALIAS_IN_USE or maybe even abuse M_ROOM_IN_USE.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saying the M_ROOM_IN_USE doesn't quite seem to mean the same thing (it isn't a matter of it being used already, but by the data not matching). Adding an M_BAD_ALIAS likely makes more sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is M_INVALID_PARAM wrong for that purpose? If it's an m.room.canonical_alias event, there's not much room for ambiguity, while an additional error code means additional treatment (i.e. turning to a human-friendly message) in all the clients.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there was some confusion here over what the behaviour should be if the alias doesn't exist at all. I was envisaging it would be treated the same way as an alias which points at the wrong room (400 with M_BAD_ALIAS).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to copy what I had put somewhere else, my understanding from the above was we would do the following:

  1. M_INVALID_PARAM if the form of the alias is wrong.
  2. M_NOT_FOUND if the alias is completely unknown.
  3. M_BAD_ALIAS if the alias is found, but does not point to this room.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think of M_NOT_FOUND as a special case. I wouldn't tremendously care if the alias is misplaced or just doesn't exist; I'm happy to either have or not have M_NOT_FOUND for that.

Copy link
Member Author

@richvdh richvdh Mar 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly conscious that clients are going to forget to deal with 404s as well as 400s (as shown in riot at matrix-org/synapse#7105)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I have clarified this in 7917d08. If people have strong feelings that we should do something different, please say so.

`m.room.canonical_alias` by checking that it is a valid alias according to
the [syntax](https://matrix.org/docs/spec/appendices#room-aliases), and by
looking up the alias and and that it corresponds to the expected room ID.

(Note: Synapse currently implements this check on the main alias, though
this is unspecced.)

(TODO: what error code should be returned if the alias is invalid, or if it
points to the wrong room?)
richvdh marked this conversation as resolved.
Show resolved Hide resolved

* Currently, [`PUT /_matrix/client/r0/directory/room/{roomAlias}`](https://matrix.org/docs/spec/client_server/r0.6.0#put-matrix-client-r0-directory-room-roomalias)
attempts to send updated `m.room.aliases` events on the caller's
behalf. (This is implemented in Synapse but does not appear to be explicitly
specced.) This functionality should be removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably this enters the spec as "Do not send m.room.aliases events" for abundance of clarity. Likewise for endpoints below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, possibly, but if we've got this right, m.room.aliases should disappear from the spec altogether. It would make odd reading (to a reader who didn't know the history) if the only place it was mentioned was to say that you shouldn't send one.


* Currently, [`DELETE /_matrix/client/r0/directory/room/{roomAlias}`](https://matrix.org/docs/spec/client_server/r0.6.0#delete-matrix-client-r0-directory-room-roomalias),
attempts to send updated `m.room.aliases` and/or `m.room.canonical_alias`
events on the caller's behalf, removing any aliases which have been
deleted. (Again, this is implemented in Synapse but does not appear to be
explicitly specced.) The `m.room.aliases` functionality should be removed,
and the `m.room.canonical_alias` functionality should be extended to cover
`alt_aliases`. As today, no error occurs if the caller does not have
permission to send such an event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is largely an implementation detail anyways:

Servers may choose to implement additional access control checks here, for instance that room aliases can only be deleted by their creator or a server administrator.

I'd count "can you send the event" as additional access control.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeahhh, though I think it's worth emphasising that being able to delete an alias, but not update the event, is a case that server implementers should consider.

I've clarified this to leave the behaviour implementation-defined, but to make a recommendation. wdyt?


* A new api endpoint, `GET /_matrix/client/r0/rooms/{roomId}/aliases` is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future MSCs: we also need to define authorization and rate limiting requirements. Presumably this endpoint can be rate limited and requires auth.

added, which returns the list of aliases currently defined on the local
richvdh marked this conversation as resolved.
Show resolved Hide resolved
server for the given room. The response looks like:

```json
{
"aliases": [
"#somewhere:example.com",
"#somewhereelse:example.com",
"#special_alias:example.com"
]
}
```

This API can be called by any current member of the room (calls from other
richvdh marked this conversation as resolved.
Show resolved Hide resolved
users result in `M_FORBIDDEN`).

TODO: should this be tied in with `history_visibilty` to allow peeking from
richvdh marked this conversation as resolved.
Show resolved Hide resolved
non-members? On the one hand that feels like pointless clutter; on the other
it feels like it makes it more consistent with peekable rooms.

Various APIs are currently subject to implementation-defined access
restrictions. No change to the specification is changed in this regard
richvdh marked this conversation as resolved.
Show resolved Hide resolved
(implementations will continue to be free to impose their own
restrictions). Nevertheless as part of this MSC it is useful to consider some
proposed changes to Synapse's implementation:

* No change: `PUT /_matrix/client/r0/directory/room/{roomAlias}`: Synapse
only allows access to current members of the room, and also exposes some
configuration options which allow restriction of which users are allowed to
create aliases in general.

* `DELETE /_matrix/client/r0/directory/room/{roomAlias}`: in this case,
currently Synapse restricts its use to the user that created the alias, and
server admins.

It is proposed to extend this to local users who have a power-level
sufficient to send an `m.room.canonical_alias` event in the room that the
alias currently points to.

* [`PUT /_matrix/client/r0/directory/list/room/{roomId}`](https://matrix.org/docs/spec/client_server/r0.6.0#put-matrix-client-r0-directory-list-room-roomid)
and the corresponding unspecced `DELETE` api (both of which set the
visibility of a room in the public directory): currently Synapse restricts
their use to server admins and local users who have a PL sufficient to send
an `m.room.aliases` event in the room (ignoring the special auth
rules). This will be changed to check against the PL required to send an
`m.room.canonical_alias` event.

It is envisaged that Matrix clients will then change their "Room Settings" user
interface to display the aliases from `m.room.canonical_alias` instead of those
in `m.room.aliases`, as well as giving moderators the ability to update that
list. Clients might also wish to use the new `GET
/_matrix/client/r0/rooms/{roomId}/aliases` endpoint to obtain and display the
currently-available local aliases, though given that this list may be subject
to abuse, it should probably not be shown by default.

### Future work

This work isn't considered part of this MSC, but rather a potential extension
for the future.

* It may be useful to be able to query remote servers for their alias
list. This could be done by extending `GET
/_matrix/client/r0/rooms/{roomId}/aliases` to take a `server_name`
parameter, and defining an API in the server_server spec which will expose
the requested information, subject to the calling homeserver having at least
one user with a right to see it.

* Similarly, room moderators may wish to be able to delete aliases on a remote
server for their room. We could envisage a fedaration API which allows such
richvdh marked this conversation as resolved.
Show resolved Hide resolved
a request to be made, subject to the calling homeserver having at least one
moderator in the room.

## Potential issues

The biggest problem with this proposal is that existing clients, which rely on
`m.room.aliases` in one way or another, will lose functionality. In particular,
they may not know about aliases that exist, or they may look at outdated
`m.room.aliases` events that list aliases that no longer exist. However, since
`m.room.aliases` is best-effort anyway, these are both problems that exist to
some extent today.

## Alternatives

We considered continuing to use `m.room.aliases` to advertise room aliases
instead of `m.room.canonical_alias`, but the significant changes in semantics
made that seem inappropriate.

We also considered using separate state events for each advertised alias,
rather than listing them all in one event. This might increase the number of
aliases which can be advertised, and help to reduce races when editing the
list. However, the 64KB limit of an event still allows room for hundreds of
aliases of any sane length, and we don't expect the list to be changing
frequently enough for races to be a practical concern. Ultimately the added
complexity seemed redundant.

A previous suggestion was
[MSC2260](https://github.com/matrix-org/matrix-doc/issues/2260), which proposed
keeping `m.room.aliases` largely as-is, but giving room moderators tools to
control who can send them via room power-levels. We dismissed it for the
reasons set out at
https://github.com/matrix-org/matrix-doc/pull/2260#issuecomment-584207073.

## Security considerations

None currently identified.

## Unstable prefix

TBD.