-
Notifications
You must be signed in to change notification settings - Fork 399
MSC3905: Application services should only be interested in local users #3905
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
Merged
turt2live
merged 10 commits into
main
from
madlittlemods/appservice-interested-in-local-users
Oct 24, 2022
Merged
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
baba3ac
MSC so appservices only interested in local users
MadLittleMods a1ad534
Add MSC number
MadLittleMods 48684a6
Fix typo
MadLittleMods 60f3232
Fix title
MadLittleMods b36dee4
Add example implementation
MadLittleMods 74f750c
`rooms` and `aliases` not affected
MadLittleMods 7b5b8ad
Add historical context
MadLittleMods a280c24
Document how to do unstable
MadLittleMods 0b9fdb0
fix some typos
anoadragon453 dbe5bde
Add link to spec
MadLittleMods File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
98 changes: 98 additions & 0 deletions
98
proposals/3905-appservice-only-interested-in-local-users.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
# MSC3905: Application services should only be interested in local users | ||
|
||
Application services receive events that they are "interested" in. | ||
|
||
The current language in the spec describes it like this: | ||
|
||
> An application service is said to be "interested" in a given event if one of the IDs | ||
> in the event match the regular expression provided by the application service | ||
> [registration]. [...] the application service is said to be interested in a given | ||
> event if one of the application service's namespaced users is the target of the event, | ||
> or is a joined member of the room where the event occurred. | ||
|
||
This language is ambiguous around which users it should match against though. Should it | ||
be all members of the room including remote users from other homeservers or just the | ||
local users where the application service lives? It could be assumed either way and | ||
naively applied to all members of the room which is still valid with the current | ||
language and even what Synapse does. | ||
|
||
But matching against remote users is merely a footgun because an application service may | ||
assume that it'll receive all events sent by that remote user, even though it will only | ||
receive events in rooms that are shared with a local user. This leaves us with a | ||
behavior mismatch between remote and local users. | ||
|
||
|
||
## Proposal | ||
|
||
Therefore the proposal is that the `users` namespace regex should only be applied | ||
against local users of the homeserver. | ||
MadLittleMods marked this conversation as resolved.
Show resolved
Hide resolved
MadLittleMods marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
A basic implementation of this would look like: | ||
|
||
```js | ||
const isLocalUser = sender.endsWith(":" + homeserver.domain); | ||
const isInterestingUser = isLocalUser && sender.matches(regex); | ||
``` | ||
|
||
```js | ||
const localRoomMembers = getLocalRoomMembers(roomId); | ||
const interestingUsers = localRoomMembers.filter((localRoomMember) => localRoomMember.matches(regex)); | ||
``` | ||
|
||
--- | ||
|
||
To avoid confusion, please note that the `rooms` and `aliases` namespaces are not | ||
affected. You can still match whatever rooms and aliases to listen to all events | ||
that occur in them. | ||
|
||
|
||
## Potential issues | ||
|
||
There are use cases like moderation where an application service wants to hear all | ||
messages from remote users in rooms but these are also covered by the `rooms` | ||
namespace where all events in a matched room are considered "interesting". | ||
|
||
|
||
|
||
## Alternatives | ||
|
||
The alternative is to clarify that the `users` namespace should be matched against all | ||
users (local and remote). This still leaves us with the behavior-difference footgun. | ||
|
||
|
||
|
||
## Security considerations | ||
|
||
Since we're reducing the surface area, there doesn't seem to be any additional security | ||
considerations introduced. | ||
|
||
With this MSC, an application service will be receiving less events than before. | ||
|
||
|
||
## Historical context | ||
|
||
According to @turt2live (SCT member), "the spec intended to [originally] say the | ||
namespace can make an appservice interested in remote users, though there's obviously no | ||
ability for the server to call `/user` on remote users (it's not like the appservice can | ||
create them)." (https://github.com/matrix-org/synapse/pull/13958#discussion_r988369446) | ||
|
||
This intention goes back further than `r0` (or `v1.0` in marketing versions speak) but | ||
this history is lost to time since there isn't really anything concrete to point to | ||
beyond the original spec | ||
[issue](https://github.com/matrix-org/matrix-spec-proposals/issues/1307) and | ||
[PR](https://github.com/matrix-org/matrix-spec-proposals/pull/1533) which don't mention | ||
these details. | ||
|
||
Since we're unable to come up with any valid use cases nowadays, it's unclear to | ||
outsiders from that time whether the original intention is actually true. In any case, | ||
we're clarifying it here and making an MSC to change it explicitly. | ||
|
||
|
||
## Unstable prefix | ||
|
||
While this MSC is not considered stable, appservices can opt-in to this behaviour in | ||
either of two ways: | ||
|
||
1. Specifying a regex which includes the local homeserver's domain in the regex (eg: | ||
`@_irc.*:example.org`) | ||
1. Adding `"org.matrix.msc3905": true` to the root level of their registration file |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.