-
Notifications
You must be signed in to change notification settings - Fork 401
MSC4155: Invite filtering #4155
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
base: main
Are you sure you want to change the base?
Changes from 23 commits
6adc165
43aef70
b8b226a
dcde420
04169f8
8c81fd5
d132a13
2345a5b
a558adb
a47dbce
e9478f0
29324e9
2f17aa7
3b248c5
230ed37
f916f98
44ecf7e
88fe9cc
be64f07
e526862
52c8bd5
0eee397
21521f0
643c5bc
5c3cab8
16dd841
e735ee8
f65b9b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,198 @@ | ||
# MSC4155: Invite filtering | ||
|
||
Matrix supports ignoring users via the eponymous [module] and the `m.ignored_user_list` account data | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
event. This is a nuclear option though and will suppress both invites and room events from the ignored | ||
users. Additionally, the `m.ignored_user_list` event only allows for block-list configurations that ignore | ||
specific users but doesn't have a mechanism to ignore entire servers. These shortcomings make the module | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
insufficient for use cases such as tightly locked down applications where ignoring needs to be the default | ||
behaviour. | ||
|
||
This proposal generalises the ignoring users [module] to allow filtering invites specifically. The scheme | ||
outlined below was conceptually borrowed from the [gematik specification]. | ||
|
||
|
||
## Proposal | ||
|
||
To allow users to configure which other users are allowed to invite them into rooms, a new account data | ||
event `m.invite_permission_config` is introduced. | ||
|
||
```json5 | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
// User-level settings | ||
"allowed_users": [ "@john:goodguys.org", ... ], | ||
"ignored_users": [ ... ], | ||
"blocked_users": [ ... ], | ||
// Server-level settings | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"allowed_servers": [ ... ], | ||
"ignored_servers": [ ... ], | ||
"blocked_servers": [ "*" ] // A feature of all the fields at this level, globs | ||
} | ||
} | ||
``` | ||
|
||
All properties in `content` are optional arrays. A missing or `null` property MUST be treated like an | ||
empty array. The array elements are [glob expressions]. Any `*_users` glob is to be matched against | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're explicitly extending to include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer only missing to be considered empty. What's the goal of including null here?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would clients/servers treat null differently to missing though? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is null valid in the grammar? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think
There's occasionally been complaints that the spec doesn't clearly describe the behavior for However, I suppose that's sort of obvious because how else would such a value be interpreted? I have removed the sentence entirely with 5c3cab8. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the options are that invalid data should be treated as empty or that it invalidates the whole object. 🤷 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this is something that could be defined spec-wide? FWIW I'm dealing with this when implementing element-hq/synapse#18195 as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may be a spec-wide problem, but being explicit in the MSC in the meantime helps ensure it's covered and not delayed behind yet more MSCs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, then I think anything that doesn't match the schema should lead to the whole object being discarded. This is because the different lists will commonly work together and using only some of them might lead to unexpected results.
How does that sound? |
||
full user IDs (localpart and domain). Any `*_servers` glob is to be matched against server names / | ||
domain parts of user IDs. | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
When evaluating an invite, implementations MUST first apply the existing `m.ignored_user_list` as per | ||
the current spec. If the invite didn't match, implementations MUST then apply `m.invite_permission_config`. | ||
The complete processing logic is as follows: | ||
|
||
1. Verify the invite against `m.ignored_user_list`: | ||
1. If it matches `ignored_users`, stop processing and ignore. | ||
2. Verify the invite against `m.invite_permission_config`: | ||
1. If it matches `allowed_users`, stop processing and allow. | ||
2. If it matches `ignored_users`, stop processing and ignore. | ||
3. If it matches `blocked_users`, stop processing and block. | ||
4. If it matches `allowed_servers`, stop processing and allow. | ||
5. If it matches `ignored_servers`, stop processing and ignore. | ||
6. If it matches `blocked_servers`, stop processing and block. | ||
3. Otherwise, allow. | ||
|
||
The semantics of "ignore" and "block" follow [MSC4283] which means ignoring hides the invite with no | ||
feedback to the inviter whereas blocking rejects (or refuses, in the case of servers) the invite. | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
When blocking an inviter, the server must respond to the following endpoints with an error: | ||
|
||
- `PUT /_matrix/federation/(v1|v2)/invite/{roomId}/{eventId}` | ||
- `POST /_matrix/client/v3/rooms/{roomId}/invite` | ||
- `POST /_matrix/client/v3/createRoom` (checking the `invite` list) | ||
- `PUT /_matrix/client/v3/rooms/{roomId}/state/m.room.member/{stateKey}` (for invite membership) | ||
|
||
The error SHOULD be `M_INVITE_BLOCKED` with a HTTP 403 status code. | ||
|
||
When ignoring an invite, these endpoints MUST handle an invite normally as if accepted. However, the server | ||
MUST not include the invite down client synchronization endpoints such as `GET /_matrix/client/v3/sync` or | ||
MSC4186's sliding sync endpoint. In addition, these invites MUST be ignored when calculating push notifications. | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Otherwise, all other endpoints (such as `GET /_matrix/client/v3/rooms/{roomId}/state`) should work as before. | ||
|
||
Servers are not expected to process these rules for appservice users when calculating events to send down | ||
`PUT /_matrix/app/v1/transactions`. Appservices are not expected to be run directly by client users, and | ||
should be able to handle their own spam prevention. | ||
|
||
The semantics and order of evaluation enable a number of use cases, for instance: | ||
|
||
```json5 | ||
// Invites from everyone | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { } | ||
} | ||
|
||
// No invites at all | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"blocked_servers": [ "*" ] | ||
} | ||
} | ||
|
||
// Only invites from goodguys.org | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"allowed_servers": [ "goodguys.org" ], | ||
"blocked_servers": [ "*" ] | ||
} | ||
} | ||
|
||
// Invites from everyone except badguys.org | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"blocked_servers": [ "badguys.org" ] | ||
} | ||
} | ||
|
||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Only invites from goodguys.org except for @notactuallyguy:goodguys.org | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"blocked_users": [ "@notactuallyguy:goodguys.org" ], | ||
"allowed_servers": [ "goodguys.org" ] | ||
"blocked_servers": [ "*" ] | ||
} | ||
} | ||
|
||
// No invites from badguys.org unless it's @goodguy:badguys.org | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"allowed_users": [ "@goodguy:badguys.org" ], | ||
"blocked_servers": [ "badguys.org" ] | ||
} | ||
} | ||
|
||
// Only invites from goodguys.org and don't provide feedback to reallybadguys.org | ||
{ | ||
"type": "m.invite_permission_config", | ||
"content": { | ||
"allowed_servers": [ "goodguys.org" ], | ||
"ignored_servers": [ "reallybadguys.org" ], | ||
"blocked_servers": [ "*" ] | ||
} | ||
} | ||
``` | ||
|
||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Servers MUST enforce `m.invite_permission_config` against incoming new invites. Additionally, Servers | ||
SHOULD apply the config against existing pending invites as well. | ||
|
||
|
||
## Potential issues | ||
|
||
Enforcing the permission configuration exclusively on the server means users have no way to review | ||
processed invites. This is desirable in most cases as a spam protection measure. It does mean, however, | ||
that if the user has accidentally blocked a good actor and is informed about it through a different | ||
communication channel, they'll have to update their permission configuration and request a re-invite. | ||
|
||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Alternatives | ||
|
||
The split between user settings and server settings is technically not needed because glob expressions | ||
are powerful enough to allow matching either. Splitting them is more explicit and prevents unintended | ||
globbing mistakes, however. The fact that a user glob and a server glob can overlap does not seem | ||
problematic because this proposal includes a deterministic processing order for all settings. | ||
|
||
A previous version of this proposal included a base setting of block / allow all with user and | ||
server exceptions applied on top. In this scheme, flipping the base setting also inverts the semantics | ||
of all exceptions, however, which makes changing the configuration quite complicated. | ||
|
||
A comprehensive comparison of existing invite filtering proposals may be found in [MSC4192]. The | ||
present proposal is functionally inferior to some of the alternatives outline there. It does, for | ||
instance, not cover the change history of the permission config or sharing the config among different | ||
users. The proposal is, however, a straightforward and easy to implement extension of the existing | ||
`m.ignored_user_list` mechanism. See also [this comment] for further details. This proposal is additionally | ||
extensible for further types of blocking in the future. For example, a future MSC could create definitions | ||
and behaviours to block/ignore/allow invites from contacts, of a particular type (DM, space, etc), | ||
to a particular room, or even with given keywords. | ||
|
||
|
||
## Security considerations | ||
|
||
None. | ||
|
||
|
||
## Unstable prefix | ||
|
||
Until this proposal is accepted into the spec, implementations should refer to `m.invite_permission_config` | ||
and `m.invite_permission_config_enforced` as `org.matrix.msc4155.invite_permission_config` and | ||
`org.matrix.msc4155.invite_permission_config_enforced`, respectively. Note that the [gematik specification], | ||
which predates this MSC, uses an event type of `de.gematik.tim.account.permissionconfig.v1` and | ||
a different event schema. | ||
|
||
The error `M_INVITE_BLOCKED` should be `ORG.MATRIX.MSC4155.M_INVITE_BLOCKED` until this proposal is accepted into the spec. | ||
## Dependencies | ||
|
||
This proposal loosely depends on [MSC4283] for the semantics of "ignore" and "block". | ||
|
||
|
||
[gematik specification]: https://github.com/gematik/api-ti-messenger/blob/9b9f21b87949e778de85dbbc19e25f53495871e2/src/schema/permissionConfig.json | ||
[glob expressions]: https://spec.matrix.org/v1.14/appendices/#glob-style-matching | ||
[MSC4192]: https://github.com/matrix-org/matrix-spec-proposals/pull/4192 | ||
[MSC4283]: https://github.com/matrix-org/matrix-spec-proposals/pull/4283 | ||
[module]: https://spec.matrix.org/v1.10/client-server-api/#ignoring-users | ||
[this comment]: https://github.com/matrix-org/matrix-spec-proposals/pull/4192#discussion_r2025188127 |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation requirements:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I believe all the gematik implementations will be closed source, I'll reference #3860 (comment) as an example for how cases like this were handled in the past. Thanks @clokep for digging it up.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element-hq/synapse#18288 is a serverside implementation, albeit with #4155 (comment) "corrected"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element-hq/element-web#29603 exposes the serverside settings in the client, but does no filtering of itself. @Johennes does this evoke any worries from you if the invite filtering is done server-side?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I this is fine and consistent with the proposal which allows but doesn't enforce the filtering on either the client or the server. Now that I think of it, we might need a capability so that the client knows when the server does not filter in which case the client needs to filter itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element-hq/synapse#18288 supports the updated proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With my SCT hat:
The following implementations demonstrate the desire for filtering, and experiment with different ordering, but all demonstrate that the code is more than possible:
The following implementations show that there's desire, but don't do much with the actual config:
Collectively, I feel this satisfies the implementation component, though we should monitor for user feedback if the rule processing order feels wrong. If so, we can correct it with a future MSC relatively easily (it'll suck for people on implementations which use the 'old' rules, but as those implementations update users will see the changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish I shared your confidence. Won't it (also) suck for people who have set up rules with the "old" system, which suddenly don't work because the ordering has changed? And then they'll change the rules and get something that works with their client but not their server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot think of a good way to prevent that other than blocking this MSC to let implementations try out the currently proposed ordering in practice. That'll involve exactly the same migration pain for users if the proposal changes, just that people will (hopefully consciously) have opted into an explicitly unstable implementation at their own risk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't disagree with you there. At some point we're going to have to just decide that we have enough confidence in the ordering that it's worth shipping. I just think it's worth having our eyes open to the fact that changing it later is going to be a bit of a buttpain, rather than "relatively easy" as Travis claimed.