Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MSC3386: Unified Join Rules #3386

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

MSC3386: Unified Join Rules #3386

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c60061
Create Unified Join Rules MSC.
kevincox Sep 7, 2021
9155595
Update with MSC ID.
kevincox Sep 7, 2021
0794346
Link to Pull Request instead of resulting document.
kevincox Sep 7, 2021
b971838
Use room IDs in join rules rather than aliases.
kevincox Sep 8, 2021
0a6f8c1
Spelling fix.
kevincox Sep 8, 2021
f4097da
Remove backwards compatibility.
kevincox Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 251 additions & 0 deletions proposals/3386-unified-join-rules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
# MSC3386: Unified Join Rules
kevincox marked this conversation as resolved.
Show resolved Hide resolved

[MSC2403 Knock](https://github.com/matrix-org/matrix-doc/pull/2403) and [MSC3083 Restricting room membership based on membership in other rooms](https://github.com/matrix-org/matrix-doc/pull/3083) both update the join rules of a room to allow a new feature. The former defines `join_rule: knock` which allows anyone to knock to enter a room and the latter defines `join_rule: restricted` to restrict who can join a room based on a set of rules. Unfortunately these features can not be used together as a room can only have one `join_rule`.

This MSC aims to solve the proliferation of `join_rule`s and allow these features to interoperate.

## Proposal

In a future room version the meaning of the `m.room.join_rules` state event will be changed to the following.

### `join_rule`

The `join_rule` key is removed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@tulir tulir Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


### `allow_join`

`allow` will be renamed to `allow_join`. Otherwise its meaning is unchanged.

The `allow_join` key may be absent in which case it is treated as if it was set to `[]` (the empty list). In this case no one is allowed to join without an invite.

### `allow_knock`

An `allow_knock` key will be allowed. This functions identically to the `allow_join` key except that it controls who can knock, instead of controlling who can join. Please see [MSC3083 Restricting room membership based on membership in other rooms](https://github.com/matrix-org/matrix-doc/pull/3083) for how the rules are evaluated and [MSC2403 Knock](https://github.com/matrix-org/matrix-doc/pull/2403) for what it means to knock.

The `allow_knock` key may be absent in which case it is treated as if it was set to `[]` (the empty list). In this case no one is allowed to knock.

### `m.any`

The `m.any` allow type will be defined. This type allows any user to perform the relevant action. The may be used in both the `allow_join` and `allow_knock` fields.

### Example: Anyone can knock

This shows an example of a room where you can join if you are a member of `!users:example.org` otherwise you can only knock.

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

### Example: Restricted knock

This shows an example of a room where anyone in the `!users:example.org` room (or space) can knock, but anyone in `!mods:example.org` can join directly.

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

### Conversion
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"
		}]
	}
}


When upgrading a room the following rules can be used to generate a new `join_rules` that matches the previous `join_rules`.

#### `invite`

A `join_rules` state event with `join_rule: invite` can be replaced by the following `join_rules`.

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {}
}
```

#### `public`

A `join_rules` state event with `join_rule: public` can be replaced by the following `join_rules`.

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_join": [{
"type": "m.any"
}]
}
}
```

#### `knock`

A `join_rules` state event with `join_rule: knock` can be replaced by the following `join_rules`.

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_knock": [{
"type": "m.any"
}]
}
}
```

#### `restricted`

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

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_join": // Value from `allow` attribute of previous `join_rules`.
}
}
```

For example the following `join_rules`...

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

...becomes...

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

## Potential issues

### Useless `allow_knock` entires.

It is possible that entries in `allow_knock` are redundant because they are also included in `allow_join` so could simply join directly. While this is unsightly it is non-harmful and will not affect users or servers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_join": [{
"type": "m.any"
}]
// This is irrelevant because anyone can directly join.
"allow_knock": [{
"type": "m.room_membership"
"room_id": "!users:example.org"
}]
}
}
```

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_join": [{
"type": "m.room_membership",
"room_id": "!users:example.org"
}]
// This is irrelevant because everyone in this room can join directly.
kevincox marked this conversation as resolved.
Show resolved Hide resolved
"allow_knock": [{
"type": "m.room_membership"
"room_id": "!users:example.org"
}]
}
}
```

```json5
{
"type": "m.room.join_rules"
"state_key": ""
"content": {
"allow_join": [{
"type": "m.any"
}, {
// This is irrelevant because of the m.any rule.
kevincox marked this conversation as resolved.
Show resolved Hide resolved
"type": "m.room_membership",
"room_id": "!users:example.org"
}]
}
}
```

Clients may consider helping users to clean up unnecessary elements from the `allow_join` and `allow_knock` lists.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we lay this over on homeservers instead, actually?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


## Alternatives

### Introduce a new mechanism for knock.

Knock is arguably not a join rule, it is a rule for who may request an invite. It could be moved to a separate mechanism to avoid the conflict with `join_rule: restricted`.

This MSC is a better alternative because:

- Knock does affect how people may join a room so it makes sense to handle it in the same location as other ways to join.
kevincox marked this conversation as resolved.
Show resolved Hide resolved
- Allowing `m.room_membership` restricted knocking is a useful feature that appears "for free" with this unification.
kevincox marked this conversation as resolved.
Show resolved Hide resolved

## Security considerations

Other than the general risk of adding complexity to `join_rules` there are no further security implications.

It is worth noting the [Security considerations of `m.room_membership`](./3083-restricted-rooms.md#security-considerations) as this MSC does not improve them and the same concerns apply (although with less consequences) to `allow_knock`.

## Unstable prefix

Until this specification is approved please the follow changes should be used for implementation:
- Use `ca.kevincox.allow_knock.v1` instead of the `allow_knock` attribute.
- use `ca.kevincox.any.v1` instead of `type: m.any`.