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.

Sign up for GitHub

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

Jump to bottom

MSC2432: Updated semantics for publishing room aliases #2432

Merged
merged 9 commits into from
Mar 25, 2020
Merged

MSC2432: Updated semantics for publishing room aliases #2432

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6bd4b3c
MSC: Updated semantics for publishing room aliases
richvdh Feb 10, 2020
95ff266
Apply suggestions from code review
richvdh Feb 11, 2020
786772f
Update proposals/2432-revised-alias-publishing.md
richvdh Feb 18, 2020
645dbcc
make room alias lists peekable
richvdh Feb 19, 2020
30d762c
document unstable prefixes/feature flags
richvdh Feb 19, 2020
98a6cd0
fix table
richvdh Feb 19, 2020
8b9ea10
server admins can list aliases
richvdh Feb 19, 2020
4e123ca
Update proposals/2432-revised-alias-publishing.md
richvdh Feb 29, 2020
7917d08
clarifications
richvdh Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 247 additions & 0 deletions proposals/2432-revised-alias-publishing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
# 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' responsibility to keep it so.

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
main alias is deleted. This change would legitimise that behaviour.)
Comment on lines +83 to +85
Copy link
Member

Choose a reason for hiding this comment

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

This is a problem with all state events ftr. Realistically we should just drop the Required on all the content fields for state events, unless it's needed for auth rules (like membership).

Copy link
Member Author

Choose a reason for hiding this comment

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

probably. I don't think that matters here though.

Copy link
Member

Choose a reason for hiding this comment

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

We also have this in the spec, but for some reason have declared the field as required:

    A room with an ``m.room.canonical_alias`` event with an absent, null, or
    empty ``alias`` field should be treated the same as a room with no
    ``m.room.canonical_alias`` event.


(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 •
edited
Loading

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.)

The following error codes are specified:

* HTTP 400, with `errcode: M_INVALID_PARAMETER` if an attempt is made to add
an entry which is not a well-formed alias (examples: too long, doesn't
start with `#`, doesn't contain a `:`).

* HTTP 400, with `errcode: M_BAD_ALIAS` if an added alias does not point at
the given room (either because the alias doesn't exist, because it can't
be resolved due to an unreachable server, or because the alias points at a
different room).

* 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`.

The behaviour if the calling user has permission to delete the alias but
does not have permission to send `m.room.canonical_alias` events in the room
(for example, by virtue of being a "server administrator", or by being the
user that created the alias) is implementation-defined. It is *recommended*
that in this case, the alias is deleted anyway, and a successful response is
returned to the client.

* 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`). For rooms with `history_visibility` set to
`world_readable`, it can also be called by users outside the room.

Servers might also choose to allow access to other users such as server
administrators.

Various APIs are currently subject to implementation-defined access
restrictions. No change to the specification is introduced in this regard
(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 federation API which allows such
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

While this feature is in development, the following names will be in use:

| Proposed final name | Name while in development |
| --- | --- |
| `GET /_matrix/client/r0/rooms/{roomId}/aliases` | `GET /_matrix/client/unstable/org.matrix.msc2432/rooms/{roomId}/aliases` |

Servers will indicate support for the new endpoint via a non-empty value for feature flag
`org.matrix.msc2432` in `unstable_features` in the response to `GET
/_matrix/client/versions`.