MSC3386: Unified Join Rules

[kevincox]

Rendered

Signed-off-by: Kevin Cox kevincox@kevincox.ca

Fixes: https://github.com/matrix-org/matrix-doc/issues/3297

[turt2live]

Please line wrap to roughly 100 characters. Links to MSCs should be github links rather than the markdown documents to ensure conversation context and threads are maintained.

[kevincox]

Will do.

[ShadowJonathan]

@kevincox You forgot to add a "rendered" link

[Dominaezzz]

Please, either don't remove the join_rule key or create a new state event.

[Dominaezzz]

Redundant is a strong word I think. People are currently able to send invites to public rooms, so knocking on public rooms seems perfectly fine, at least we already have this "problem".
In fact, one may want to explicitly prevent knocking in their public rooms to prevent spam or something.

[kevincox]

I can re-word this section to be a bit softer. I still think it makes a "rarely useful" configuration possible but I don't think it is very harmful anyways.

[Dominaezzz]

For the sake of client side backwards compatibility and spec consistency, please don't remove this key.

Can this instead be a new join_rule, like "custom", "granular", "msc3386", etc ?

{
	"type": "m.room.join_rules"
	"state_key": ""
	"content": {
		"join_rule": "custom",
		"allow_join": [{
			"type": "m.room_membership"
			"room_id": "!users:example.org"
		}]
		"allow_knock": [{
			"type": "m.any"
		}],
	},
}

In addition to this, the allow_ prefixes now seem unnecessary.

[tulir]

There's no need for the join rule key so some random static value like that seems pretty pointless.

Adding a fake join_rule key (either in the actual federated event, or inject it server-locally before serving the event to clients) that corresponds best to the rules in the room would probably be best for backwards compatibility. For example, set "join_rule": "public" if the room has allow_join with m.any. It wouldn't do anything in the protocol/server, but old clients that don't know better would use it and display something reasonably correct. Including the fake keys could then be phased out eventually.

[Dominaezzz]

I'm not suggesting a random static value, I'm suggesting a new join rule. This would allow old style join rules to still work. So older clients would still be update the join rules. This seems to me, a low price for some backwards compat.

I think it's better to create a separate m.room.knock_rules than to break this schema anyway IMO.

[kevincox]

This was also discussed here which is how the MSC got to the current state: #3386 (comment)

Picking a new name for the join rule may make it less error-prone on clients. Originally it was kept the same because the rules were mostly-compatible. But since compatibility has largely been dropped picking a new name makes more sense.

[jcgruenhage]

@kevincox looks good to me, and looking forward to this one! One small nitpick: Is this ready for review? The PR title still says WIP, while it seems that you marked this ready for review 15h ago

[kevincox]

Thanks. Updated the title. I thought I removed this from draft weeks ago 😅

[KitsuneRal]

I really like where this is going; but a few more eyeballs on it would be great.

[KitsuneRal]

Can we lay this over on homeservers instead, actually?

[kevincox]

We could but I was thinking it would be best to keep the initial version of this small. There is no reason that we couldn't require homeservers to do particular cleanups in the future.

[Xiretza]

As noted in another comment thread, having overlap between different join rules may be intentional and should be allowed, rather than rejected/automatically "corrected" by the homeserver.

[Xiretza]

I'd love to see this, since it paves the way for better and more fine-grained join rules in future MSCs.

[Xiretza]

This could use an update for knock_restricted:

knock_restricted

A join_rules state event with join_rule: knock_restricted can be replaced by an event with the following template. Substitute the previous elements from the allow attribute into the allow_join attribute.

{
	"type": "m.room.join_rules"
	"state_key": ""
	"content": {
		"allow_knock": [{
			"type": "m.any"
		}],
		"allow_join": // Value from `allow` attribute of previous `join_rules`.
	}
}

For example the following join_rules...

{
	"type": "m.room.join_rules"
	"state_key": ""
	"content": {
		"join_rule": "knock_restricted"
		"allow": [ {
			"type": "m.room_membership"
			"room_id": "!mods:example.org"
		}, {
			"type": "m.room_membership"
			"room_id": "!users:example.org"
		}]
	}
}

...becomes...

{
	"type": "m.room.join_rules"
	"state_key": ""
	"content": {
		"allow_knock": [{
			"type": "m.any"
		}],
		"allow_join": [{
			"type": "m.room_membership"
			"room_id": "!mods:example.org"
		}, {
			"type": "m.room_membership"
			"room_id": "!users:example.org"
		}]
	}
}

[Xiretza]

As noted in another comment thread, having overlap between different join rules may be intentional and should be allowed, rather than rejected/automatically "corrected" by the homeserver.