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

Jump to bottom

MSC2346: Bridge information state event #2346

Open
wants to merge 23 commits into
base: old_master
Choose a base branch
from
Open

MSC2346: Bridge information state event #2346

Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fa30188
Initial WIP of proposal
Half-Shot Nov 4, 2019
e5daeb9
2346
Half-Shot Nov 5, 2019
e13cbec
Various updates to take this out of WIP
Half-Shot Dec 2, 2019
e887a08
Backticks around location header
Half-Shot Dec 2, 2019
dfba42d
Clarify fields
Half-Shot Dec 3, 2019
38e635a
Add some example event bodys
Half-Shot Dec 3, 2019
0103c30
Small fixes
Half-Shot Dec 3, 2019
ffc98ce
C&P fail
Half-Shot Dec 3, 2019
ef21d6b
Fix content in mastodon example.
Half-Shot Dec 3, 2019
542dfa7
@uhoreg nit picks
Half-Shot Dec 4, 2019
4f05bb3
Remove `status` key
Half-Shot Dec 8, 2019
8deecda
Update 2346-bridge-info-state-event.md
Half-Shot Dec 8, 2019
899f4b1
Update key info for dispn/avatar
Half-Shot Dec 8, 2019
d21d25b
Update MSC to include escaping information
Half-Shot Dec 9, 2019
e133838
Add Implementation notes
Half-Shot Jan 9, 2020
6e72d10
Apply suggestions from code review
Half-Shot Feb 5, 2020
9104740
Update proposals/2346-bridge-info-state-event.md
Half-Shot Feb 5, 2020
a0b98d7
Some feedback changes
Half-Shot Feb 7, 2020
d5dd039
Merge branch 'hs/msc-bridge-inf' of github.com:matrix-org/matrix-doc …
Half-Shot Feb 7, 2020
5251039
Add bridgebot
Half-Shot Feb 7, 2020
74bc2d9
Remove thirdparty/location statement
Half-Shot Feb 7, 2020
380b54d
"avatar_url" is now the standard for avatars
Half-Shot May 22, 2020
62afef8
Update proposals/2346-bridge-info-state-event.md
Half-Shot Jul 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 91 additions & 0 deletions proposals/2346-bridge-info-state-event.md
View file
Open in desktop
@@ -0,0 +1,91 @@
### MSC 2346: Bridge information state event

Many rooms across the matrix network are currently bridged into other networks, using our bridging API.
However the specification does not contain a method to determine which networks are bridged into a given
room. Many users have taken to peeking at the list of aliases for a giveaway alias like `#freenode_` or
looking for bridge bots or users with a `@_discord_` prefix.

This is an unacceptable situation. Users must know beforehand the names of IRC networks or protocols in order
to determine if a given ID is part of a bridge. The bridge cannot give away any infomation about itself
via Matrix. And blah blah blah. It alos allows bridges to be re-usable between different servers should we
want to go for the decentralised route one day.

This proposal stands to fix, or partially fix the following issues:
- a bunch of riot projects that depend upon this
- a bunch of spec items too

This proposal is heavily based upon my previous attempt [#1410](https://github.com/matrix-org/matrix-doc/issues/1410)
albeit with a notably reduced set of features. The aim of this proposal is to offer information about the
bridged network and nothing more.

## Proposal

This proposal uses

It should be noted that this MSC is intended to provide the baseline needed to display information about
a bridge. Future MSCs should be written to expand this event as the need arises.

### `org.matrix.bridge`

*The name of the event type should eventually be `m.bridge` but has been prefixed until the spec is ready.*
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved

```js
{
"state_key": "org.matrix.appservice-irc://network_id/channel_id",
"type": "org.matrix.bridge"
"content": {
"creator": "@alice:matrix.org", // Optional
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this? Widgets already suffer from this being unreliable and unhelpful, to the point of us ignoring it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to the point of us ignoring it.

image

How's that going for you?

This is actually the same use case that widgets are using it for right now, it's just sugar to point at whoever added the bridge, if it was added by a user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you may have been referring to it duplicating the sender field. This is intentional, for the plumbing use case. I don't see any reason why this would be unreliable though.

Copy link
Member

Choose a reason for hiding this comment

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

It's not really trusted information, though I guess if somehow the bridge is limited to being able to send it then it can be trusted. The reason we don't use creatorUserId for widgets, even if someone else edits the widget, is because it is displayed so prominently and can cause lies to be shown to the user.

Copy link
Member

Choose a reason for hiding this comment

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

The major concern here is essentially a room admin/mod hopping into the room and changing the state event to say that @bob:example.org is responsible for the bridge, who may or may not even be a real user ID. One of the downsides for Bob in that case would be spam about bridging to somewhere controversial. In a general scenario though, as an informational thing it's probably okay, but it would be nice to acknowledge the privacy/social risks in the security section.

"status": "active" // Optional, will default to active. One of "active", "disabled".
"network": {
"id": "freenode",
"displayname": "Freenode", // Optional
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
turt2live marked this conversation as resolved.
Show resolved Hide resolved
"avatar": "mxc://foo/bar" // Optinal
},
"channel": {
Copy link

Choose a reason for hiding this comment

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

Nit: Why are we calling this a channel? In Matrix we call these rooms so doesn't it make sense to use the local terminology. Basically "the thing in the source protocol which maps to a room in Matrix". For example if the remote network has groups and Matrix has rooms why are we calling the field channel?

"id": "#friends",
"displayname": "Friends" // Optional
},
// Custom vendor-specific keys
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
"org.matrix.appservice-irc.room_mode": "+sn",
},
"sender": "@appservice-irc:matrix.org"
}
```

The `state_key` must be comprised of the bridge's prefix, followed by the `network.id`, followed by the `channel.id`.
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved

The `sender` should be the MXID of the bridge bot.
richvdh marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to have a dedicated field for the identity of the bridge bot rather than it having to be the sender. My main motivation for this is potential future uses of this type of event to request provisioning of bridges (i.e. I send this event into the room, invite the bridge bot user, the bridge reads the state event and sets up the bridge).

While I appreciate that this isn't really the scope of this MSC (which is fine), standardising on the only entity to send these state events is the bridge bot seems very limiting for future iterations and extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might also help solve #2346 (comment) as we don't assume the sender is the bridge.


The `creator` field is the name of the *user* which provisioned the bridge. In the case of alias based bridges, where the
creator is not known -- it may be omitted.

The `status` field informs the client if the bridge is still active, or has been disabled. The meaning may be different
depending on the context of the network.

The `network` field should be information about the specific network the bridge is connected to. This may be "Freenode" for IRC,
or "Discord" if the network is global. (What does this mean?).

Copy link
Member

Choose a reason for hiding this comment

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

Can we have examples of the protocol.id/network.id/channel.id part of the state key? e.g. for XMPP, would it be XMPP/xsf@muc.xmpp.org or XMPP//xsf@muc.xmpp.org (i.e. do we drop a / if there's no network field? What if there are more levels (e.g. protocol, network, community, room)? Is the network supposed to be human-readable like "Freenode" or something more like "freenode.org"? Maybe it would be better to just say that it's a path representing the hierarchy starting from the protocol and ending in the room? And then instead of hard-coding protocol, network, and channel as keys in the content, make it an array where the first element is the protocol and the last element is the channel?

Also, if a protocol/network name has a / in it, does it get escaped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added several examples of the event body to the description that might help make this more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And then instead of hard-coding protocol, network, and channel as keys in the content, make it an array where the first element is the protocol and the last element is the channel.

Potentially, though I am worried this might make it harder for clients to show a sensible UI. How would riot render:

["Github", "matrix-org", "matrix-doc", "2346"]
vs
{"protocol": "GitHub", "network": "matrix-org/matrix-doc", "channel": "2346"}

(Simplified for readability, and using a deliberately complex example).

In the first example, there are 4 keys and it's hard for a client to decide how to format this in a settings page. Joining them with a delimiter is too ugly (to me). There are probably examples which are restricted by the 3 component limit, but I am struggling to come up with any?

Copy link
Member

Choose a reason for hiding this comment

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

Does the client actually need to care about the state key? The information embedded into the state key is already available in the body, and clients would want to use a more friendly name anyways. I think we can just use an empty state key and let clients figure it out.

If you're running multiple bridges off the same bridge bot, don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you're running multiple bridges off the same bridge bot, don't.

Or if you are running several bridges off different bridge bots, you will still need different state events and therefore different keys.

Why can't we use user_ids? We could, but that does forever tie your bridge to using one user_id for life when the actual thing the bridge is "keyed" off is the protocol,network and channel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The client doesn't need to give a heck about the state key, other than it being more readable for those who want to work on it. Given there isn't really a downside to having a schema for the state key, and it gives more readability, I don't see why not.

Copy link

Choose a reason for hiding this comment

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

I would much rather that we don't prescribe the state key. Just say that it needs to start with the bridge ID (unique prefix) and the rest is implementation defined. Any meaningful values should be delegated to the context of the state object.

The event may contain information specific to the bridge in question, such as the mode for the room in IRC. These keys
should be prefixed by the bridge's name. Clients may be capable of displaying this extra information and are free to do so.
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved

## Potential issues
Copy link
Member

Choose a reason for hiding this comment

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

Missing: what if the bridge doesn't have permission to send state events? (a completely valid thing to do)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Such is life. I don't think we can engineer around users not giving their bridges permission to add state. It's not great, but it's better than adding lots of sketchy outside-of-the-room data(though matrix.org bridges in both portal and plumbed rooms have PL50 by default, so this is a relatively unlikely).

Copy link
Member

Choose a reason for hiding this comment

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

I'm a lot more concerned with the bridges that aren't on matrix.org, given the popularity of bridges in the last year alone (and the year prior to this thread). Most bridges don't ask for permissions, and a quick poll shows that most non-matrix.org bridges appear to be running without appropriate permissions in those rooms.

This can be considered a room configuration error, but it's still a valid issue that this MSC needs to acknowledge. There's a point where we can't just write off issues as "users should be better".

Copy link
Member

Choose a reason for hiding this comment

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

Missing: what if the bridge doesn't have a bridge bot? (puppet bridges, transparent bridges, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, this is more of a "well, meh". There may be a way to solve this in a future MSC, but let's leave it for then.

Copy link
Member

Choose a reason for hiding this comment

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

not sure that's the best option: can we figure out a way to publish this info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Either a user the bridge has ownership over publishes the information, or a user publishes the information on behalf of the bridge. I don't think it's worth speccing this though, as it's down to implementation how they want to insert the event.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't have to be a state event. Could start publishing this over EDUs or some other DAG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But having it in the room dag and a state event allows us to reuse pretty much all logic across the homeservers/clients/bridges. This is important, because we can not only reuse the same API routes when synchronizing the information across clients and bridges, but also re-use the same access control semantics as with other information in the room like names and topics (using PL events).

I would like to hear from others if they also think supporting the use case of a bridgebot-less bridge is important and requires us to invent our own non-room dag or EDU structure. Personally, it feels like a lot of faff for little gain.

Copy link
Member

Choose a reason for hiding this comment

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

A note in the MSC about how the non-bridge-bot user can publish this would be great.

Half-Shot marked this conversation as resolved.
Show resolved Hide resolved

This proposal knowingly discards some information present in #1410. These were removed to allow the MSC to pass quicker
through the process, as well as some of the information having limited value to clients.

The proposal intentionally sidesteps the 'bridge type' problem: Codifing what a portal, plumbed and gatewayed bridge look
like in Matrix. For the time being, the event will not contain information about the type of bridge in a room but merely
information about what is is connected to.

## Alternatives

Some thoughts have been thought on using the third party bridge routes in the AS api to get bridge info,
by calling a specalised endpoint. There are many issues with this, such as the routes not working presently
over federation, as well as requring the bridge to be online. Using a state event ensures the data is scoped
per room, and can be synchronised and updated over federation.

## Security considerations

Anybody with the correct PLs to post state events will be able to manipulate a room by sending a bridge
event into a room , even if the bridge is not present or does not exist. It goes without saying that if
you let people modify your room state, you need to trust them not to mess around.