MSC4155: Invite filtering

[Johennes]

Rendered

---

In line with matrix-org/matrix-spec#1700, the following disclosure applies:

I am a Systems Architect at gematik, Software Engineer at Unomed, Matrix community member and former Element employee. This proposal was written and published with my gematik hat on.

---

SCT stuff:

MSC checklist

FCP tickyboxes

[turt2live]

Implementation requirements:

• Client supporting the feature, and using it

[Johennes]

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.

[Half-Shot]

element-hq/synapse#18288 is a serverside implementation, albeit with #4155 (comment) "corrected"

[Half-Shot]

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?

[Johennes]

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.

[Half-Shot]

Well, I still need to get all this stuff approved but hopefully yes :). EWeb has lots of nice safety features landing :)

[deepbluev7]

Similar implementations:

• Implementing the gematik spec: https://github.com/famedly/synapse-invite-checker/blob/5608f40ffeaef1320edef8554cb432fa2c881fdd/synapse_invite_checker/types.py#L71
• Nheko (missing UI, using a slightly different format): Nheko-Reborn/mtxclient@d6a0a4e and Nheko-Reborn/nheko@978174a

[Johennes]

Partial implementation as a synapse module: https://github.com/matrix-org/msc4155-synapse-invite-filter

[Half-Shot]

element-hq/synapse#18288 supports the updated proposal.

[turt2live]

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:

• Add support for MSC4155 Invite filtering element-hq/synapse#18288
• Nheko-Reborn/mtxclient@d6a0a4e
• Nheko-Reborn/nheko@978174a
• https://github.com/famedly/synapse-invite-checker/blob/5608f40ffeaef1320edef8554cb432fa2c881fdd/synapse_invite_checker/types.py#L71
• https://github.com/matrix-org/msc4155-synapse-invite-filter

The following implementations show that there's desire, but don't do much with the actual config:

• Implement MSC4155: Invite filtering element-hq/element-web#29603

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

[Half-Shot]

I think this comes with a problem of data loss. If you flip the "default" here, you end up with one of two problems. Either:

1. The exemptions are now blocked/allowed, so if I start with my invites allowed but block @badguy:scam.org and then flip the default then the only person who can talk to me is bad guy!
2. If the client flips the default and removes the exceptions then that data is lost forever and I need to reconstruct my list.

Could we make an easy change here where the exceptions are explicit about their behavior by including the block/allow for each key. While this looks redundant, it means flipping bits won't cause unexpected behavior here.

[Johennes]

Hm, this is a problem indeed. Adding the block / allow behavior into each exception sounds reasonable to me.

It might be worth pointing out that this would make the proposal somewhat equivalent to the combination of #3847 and #4150. The only significant difference is storing the rules in a single account data event vs. as multiple policy events in a room.

I won't add this into the proposal yet because I'm hoping the SCT can provide some guidance on #4192 so that we don't spend our time in vein on the different options.

[Half-Shot]

I'm looking to work on an experimental implementation of this, for the current T&S safety sprint I'm doing so I'm just doing some review on that basis.

#4192 felt a bit heavy a quick experiment, so I want to give this one a fair tryout.

[Johennes]

Oh, I see! And sorry, I wasn't aware you're actually part of T&S. If you want to go ahead with putting the type into the exceptions, that sounds reasonable to me, like I said. I'll happily add this into the proposal if it turns out worthwhile in the implementation.

Also, since we have this in the gematik specification (though with a different event type), there must already be implementations of this somewhere, just apparently not in open source clients, sadly. 😕

[turt2live]

I think this would be relatively easy to handle with just prefixing fields for now:

{
  "allowed_users": [...],
  "allowed_servers": [...],
  "denied_users": [...],
  "denied_servers": [...]
}

Though verbose, it carries the same meaning. If we consider #4283 then we might extend this to:

allowed_users
ignored_users
blocked_users

allowed_servers
ignored_servers
blocked_servers

A later MSC (namely #3847) would handle change history of the rules more easily.

[turt2live]

https://github.com/matrix-org/msc4155-synapse-invite-filter is a partial implementation of the variant.

[Johennes]

Thanks for expanding. I have updated proposal text accordingly.

[grinapo]

I'd like to keep it separate for now, for ease of use by developers. Having too many 'tricks' like this makes the protocol feel more complicated than it is, sometimes.

So basically it's because "you like it this way"? I can accept that but there should be some consesus instead of your sole opinion, ain't it?

Seems like a clear duplicate functionality, and I guess the code would look like matching the sender of the invite with the list entries, so in this case two lists instead of one, using the same matching. I don't see how this would make developers' life simpler.

There were mentions elsewhere about ordering (name matches first, server later, to make it possible to disable everone but allow someone), which is not mentioned here as a pro, but feels like it would be a mess anyway people guessing the double ordering (first names, then servers).

Using common globs (servers and users) does not feel complex to me, and clinet developers can create separate UI for servers if they feel like, I'm sure it's not very complex to understand/remember that *:servername is a "server" entry.

[Johennes]

I had the same question initially but I think splitting users and servers is actually helpful. Having dedicated fields for servers prevents unintended globbing issues (server names can contain : as well) and is more explicit and easier to understand. Furhtermore, overlapping user / server globs don't strike me as problematic because we have a defined evaluation order for all settings.

I'm reopening this in case other people have thoughts on this.

[turt2live]

(not considering this thread as a blocker against the checklist in this case)

[turt2live]

other than the confusion of block versus ignore, this looks good to me - thank you!

If we do switch to using split out fields, supporting globs would be ideal.

[turt2live]

thanks for updating this! I'll do some implementation review then hopefully send this for FCP (with the assumption that the listed suggestions aren't too controversial 😇 )

[grinapo]

Would be nice to get more exmplanation why it's out of scope...

[turt2live]

@mscbot resolve Rule order

[turt2live]

@mscbot concern Clarify ignore behaviour (preferred: send an event, but don't deliver, like normal user ignores)

[clokep]

@mscbot concern Confusing behavior of whether enforced by clients or servers

[Half-Shot]

@mscbot concern Confusing behavior of whether enforced by clients or servers

Sounds resolved now clients are not involved.

[clokep]

@mscbot resolve Confusing behavior of whether enforced by clients or servers

[turt2live]

@mscbot resolve Clarify ignore behaviour (preferred: send an event, but don't deliver, like normal user ignores)
@mscbot resolve Checklist incomplete

[Gnuxie]

The allow semantic here is in the reverse order to server ACL, and the implication is that an allow rules overrule ban rules. This will cause problems later if an allow policy rule recommendation is created, because it will have different semantics based on context.

To be explicit, for it to be consistent with server ACL, the allow rule would have to be applied first and processing can only continue if there is a match.