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.

[Johennes]

Since I believe all the gematik implementations will be closed source,

Since it sounds like the biggest concern is whether a reasonable UI can be implemented, maybe a screenshot would suffice?

The gematik implementations are based on the initial version of this proposal where we had a base setting of either "allow all" or "block all" and exceptions for users and servers applied on top of it. It also didn't include any globbing.

FWIW, this is how my own health insurance has implemented it:

Main screen with radio buttons for the base setting at the top ("Alle" = "(Allow) everyone" / "Niemand" = "(Allow) noone"). Below that are two buttons to add exceptions for users ("Benutzer") and servers.

For adding user exceptions, they pop up a bottom sheet with a few options ("Teilnehmende hinzufügen"). The first two are for obtaining the MXID by searching gematik-specific directories. The third option scans the MXID from a QR code. And the last one lets you input it manually.

For server exceptions, the only option is to enter the server name manually.

Once entered, the exceptions show in a list at the bottom of the main screen.

[richvdh]

Thanks @Johennes, that's extremely helpful. I'm concerned, however, that it only (appears to) offer controls for blocked_users and blocked_servers (i.e, it does not expose allowed_users, ignored_users, allowed_servers or ignored_servers), and nor does it expose globbing.

It is the interplay between these settings that I think makes this proposal complicated to implement in a client, and my concern remains that we don't yet have a viable client implementation.

[Johennes]

[...] it only (appears to) offer controls for blocked_users and blocked_servers (i.e, it does not expose allowed_users, ignored_users, allowed_servers or ignored_servers), and nor does it expose globbing.

Yes, the gematik implementations are based on the initial version of this MSC which used the following scheme:

{
  "type": "m.invite_permission_config",
  "content": {
    "default": "allow | block",
    "user_exceptions": {
      "@someone:example.org": {},
      ...
    },
    "server_exceptions": {
      "example.org": {},
      ...
    }
  }
}

I think the only apparent UX problem this has is that when you flip default, your previous exceptions suddenly apply the other way around.

[richvdh]

I suppose a client can help manage that UX problem by clearing user_exceptions and server_exceptions at the same time as changing default? That doesn't feel too bad to me, tbh.

(I'm sorry you've been sent so far around the houses on this, @Johennes. For now I'm going to focus on landing something in the spec, and then we can come back here and see if we can figure out a plausible way forward.)

[Johennes]

No worries at all. 👍

[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 😇 )

[clokep]

@mscbot concern Dependency on MSC4283

[uhoreg]

A couple of nits and a minor concern, but otherwise looks reasonable.

[richvdh]

I notice this thread hasn't been addressed. For my part, I'm not sold on the importance of the ordering here matching that in server ACLs, but we need to agree it one way or the other.

(Further, this has been in production on matrix.org for months, much to my irritation, so it's a bit late to start changing it now :/)

[richvdh]

Consider the following issue1.

1. `@alice:example.com` is added to `allowed_users`.
2. `example.com` becomes hostile.
3. `example.com` is added to `blocked_servers`.
4. `@alice:example.com` sends malicious invitations.

When example.com is added to blocked_servers, the entry for each user from that server in allowed_users becomes stale.

That looks like an expected consequence of the decision to let user-level rules override server-level rules (#4155 (comment)). It is also called out under "Potential issues". No changes are needed here IMHO.

[richvdh]

this does not match the current implementation in Synapse, which matches the entire domain.

[richvdh]

AFAICT, enabled does not exist in the sample implementation in synapse

[richvdh]

I remain somewhat concerned that nobody has implemented a useful UI for all these settings. Element Web has a very coarse "Allow invites: Y/N" which toggles whether * appears in the blocked_users list, but that's a far cry from a full UI.

As an example of the sort of things I'm worried about: what happens if one client decides to implement "block invites" by setting blocked_servers: ["*"], while EW continues to do it via blocked_users? The two will be out of sync and give a very poor experience.

Honestly, if we're not expecting clients to implement a full UI, then I think we should scale back the specced proposal so that it only implements the bits that we do expect clients to implement.

[richvdh]

@mscbot concern does not match sample implementation
@mscbot concern lacks appropriate client-side sample implementation

[richvdh]

Following up on @turt2live's outstanding concerns:

@mscbot concern Discussion on forward compatability ordering needs resolution (and possible implementation)

I think this is referring to #4155 (review), though I wouldn't characterise that thread as being primarily about forward compatibility: indeed I'm not sure we have any practical proposals to provide forward compatibility.

@mscbot concern Discussion on global enable/disable thread needs resolution (and possible implementation)

I think this is referring to #4155 (comment); the proposal has been updated, but the sample implementation is still lacking support.

[richvdh]

I think this maybe needs more clarification as to what exactly "deactivate the configuration" means.

After discussion, and reading the associated thread, it seems like the intention is that enabled: false should block all invites. (i.e. it is equivalent to blocked_users: *, but at higher priority than allowed_users)

[richvdh]

Also (h/t @clokep here), could we have a more descriptive name? block_all, maybe?

[Johennes]

Oh! I had interpreted it exactly the opposite way. This is why the proposal text says

If enabled is false, stop processing and allow.`

Upon re-reading the thread, I think your interpretation is correct though. 🤦‍♂️

Putting this into perspective with your concern about the UI in clients, I wonder if we should drop enabled as well given that you can do the same thing with blocked_users: *? The only downside is that you have to empty your allowed_users setting at the same time. We may be able to avoid this if we switched to the processing order used in server ACLs (as suggested in #4155 (comment)). I think you could just temporarily prepend * to your blocked_users then without touching the rest of your configuration.

[richvdh]

@Half-Shot, @turt2live: as those arguing for enabled: false to be a high-level "block all invites": can you weigh in on this please?

[Half-Shot]

Coming back to this months later, I think the block_all was intended for clients who didn't support the full filtering spec to still have a block/unblock toggle in settings.

Element Web at the moment implements this by adding * to blocked_users (https://github.com/element-hq/element-web/blob/develop/src/settings/controllers/InviteRulesConfigController.ts#L77). I'm unconvinced that block_all is necessary.

[richvdh]

It's absolutely a problem, because presenting a meaningful UX for multiple overlapping settings is very difficult.

AFAICT, nobody has yet done so, which does little to persuade me it's possible.

[turt2live]

This MSC appears to be stuck in a cycle of debate, so to allow those conversations to happen without the pressure of FCP looming overhead, I'm pulling this back to the in-review stages.

@mscbot fcp cancel

[richvdh]

I don't feel like this MSC is ready for FCP. We are still trying to figure out what we really want from it, and we don't have viable client-side implementations of the proposal as it stands.

I don't think that minor alterations are going to change this. @Johennes previously suggested removing the *_servers fields from the proposal, but even that I don't think goes far enough: we still have the problem that nobody is implementing a UX that exposes the full power of all the *_users fields.

Meanwhile, we have the problem that this has been (prematurely) rolled out on matrix.org (with a very limited UX on Element Web). My instinct is to rip that out again, on the basis that if you were relying on it, you should have specced it properly, but it appears that the T&S team require block_all functionality to exist.

My solution, therefore, is that I'm going to write yet another MSC to add to the long list on MSC4192, that just exposes the block_all proposal from this MSC. We can then consider potential extensions in a later MSC.

[richvdh]

My solution, therefore, is that I'm going to write yet another MSC to add to the long list on MSC4192, that just exposes the block_all proposal from this MSC.

That MSC is now MSC4380. I'll do my best to implement it quickly.