diff --git a/proposals/3613-combined-join-rules.md b/proposals/3613-combined-join-rules.md new file mode 100644 index 00000000000..b0d09db36ab --- /dev/null +++ b/proposals/3613-combined-join-rules.md @@ -0,0 +1,239 @@ +# MSC3613: Combinatorial join rules + +[Join rules](https://spec.matrix.org/v1.1/client-server-api/#mroomjoin_rules) define what conditions +a user must meet in order to, well, join a room. With the current setup it is impossible to have more +than one rule active at a time, which makes it difficult to allow anyone from room X to join, but also +join if their knock was accepted. The room would instead have to choose which condition it favoured. + +This proposal aims to introduce the smallest possible baseline for a slightly improved join rules +system, allowing for combinations of join rules to take effect. Alternative MSCs, like +[MSC3386](https://github.com/matrix-org/matrix-doc/pull/3386) instead seek to redesign the join rules +structure entirely. This proposal is intended to fill the gap while the more complex MSCs work out +their specifics. + +## Proposal + +In a new room version, we simply add an optional array to the +[`m.room.join_rules`](https://spec.matrix.org/v1.1/client-server-api/#mroomjoin_rules) event. The +existing behaviour of the `join_rule` key is kept, though has slightly modified behaviour per below. + +The new array, `join_rules`, is simply an array which contains objects representing the `m.room.join_rules` +event content (minus the array itself - this does not recurse). The array acts as a "first match wins" +setup, or effectively a bunch of `OR` conditions. + +A sample would be: + +*Unrelated keys removed to reduce noise.* + +```json5 +{ + "type": "m.room.join_rules", + "state_key": "", + "content": { + "join_rule": "knock", + "join_rules": [ + { + "join_rule": "restricted", + "allow": [ + { + "type": "m.room_membership", + "room_id": "!VzMVEOJOrVWFENLtnO:localhost" + } + ] + }, + { + "join_rule": "knock" + } + ] + } +} +``` + +The sample effectively means a user can join the room if: +* They receive an explicit invite. +* They have their `knock` approved (which is an invite). +* They are a member of `!VzMVEOJOrVWFENLtnO:localhost`. + +A setup similar to this is expected to be the most common setup. + +Note how the `join_rule` persists in the protocol, even with this MSC's introduction. This is to +ensure backwards compatibility with clients and various places in the protocol which aren't aware +of the new rules. While servers will be required to parse the event as described below, areas such +as the room creation presets, older clients, room directory, etc are not easily made fully aware +of a new array. + +The `join_rule` must always be specified, even alongside `join_rules`. The `join_rule` should represent +the most semantically relevant join rule for the room - it does not need to be listed in the array, +but should best represent the room's access control. The sample chose `knock`, but a different scenario +may very well choose `restricted` or even `private`. + +Whenever the server (or client) is checking against the join rules it would now check the `join_rules` +array if present. If not present, and only when not present, the `join_rule` string is checked instead. + +The specifics for how a `restricted` join rule (and its `allow` partner key) are handled are below. + +For clarity, the server could implement a check similar to: + +```python +def is_join_rule(room_version: RoomVersion, event: EventBase, expected_rule: JoinRules) -> bool: + """Returns whether the join rule event matches the expected join rule. + + Args: + room_version: The RoomVersion the event is in + event: The join rules event + expected_rule: The anticipated rule + + Returns: + bool: True if the join rule is as expected. + """ + if not event: + return expected_rule == JoinRules.INVITE + + if room_version.msc3613_simplified_join_rules: + arr = event.content.get("join_rules", []) + if arr and isinstance(arr, list): + return expected_rule in list(r.get("join_rule", None) for r in arr) + + return event.content.get("join_rule", None) == expected_rule +``` + +As implied, if the `join_rules` array is not an array then the server falls back to `join_rule`. + +Finally, the `join_rules` key is to be protected from redaction. This has a drawback of not being +able to easily remove abuse within the objects, however is simpler on the redaction algorithm itself. + +### Restricted join rule handling specifics + +To mirror the existing condition where a `join_rule` of `restricted` has an `allow` key at the same +level, all instances within the new event structure must have the same condition: + +```json5 +{ + "type": "m.room.join_rules", + "state_key": "", + "content": { + "join_rule": "restricted", + + // Required because the `join_rule` is `restricted`. No change from previous spec on the matter. + // Note that if the sender decided that `join_rule` was best represented as some other condition + // then they might not need to supply an `allow` key at this specific level. + "allow": ["!room:example.org"], + + "join_rules": [ + { + "join_rule": "restricted", + + // Also required for the same reasons as the backwards-compatible version up above. + "allow": ["!room:example.org"] + }, + { + // Some other join rule that isn't `restricted` - doesn't require/use an `allow` key. + "join_rule": "..." + } + ] + } +} +``` + +When `join_rules` is present, even if of the wrong type (ie: not an array), the upper level of +`allow` is *not* checked when `join_rule` is `restricted`. This is to honour the behaviour described +earlier where the presence of `join_rules` makes the value of `join_rule` irrelevant, thus making +the `allow` partner key also irrelevant. + +## Potential issues + +This representation isn't perfect and can be a bit confusing. As acknowledged in the introduction, +this proposal is meant to introduce the mixed functionality of join rules without re-architecting +the system. Other MSCs are welcome/encouraged to explore what a new join rules system might be. + +This proposal allows room admins to make bad decisions, though this proposal also does not have an +interest in preventing bad decisions from happening. For example, a room might decide to mix two +incompatible rules: `public` and `invite`. If this happens, the room is effectively public as the +`invite` rule wouldn't be serving a practical purpose (remember: the first passing join rule wins). +Room admins are recommended by the proposal author to not make bad decisions. + +Through effort, room admins can additionally break out their join rules nonsensically. Again however, +this proposal doesn't have an interest in patching bad decisions. An example would be: + +```json5 +[ + { + "join_rule": "restricted", + "allow": [ + {"type": "m.room_membership", "room_id": "!a:example.org"} + ] + }, + { + "join_rule": "restricted", + "allow": [ + {"type": "m.room_membership", "room_id": "!b:example.org"} + ] + }, + { + "join_rule": "restricted", + "allow": [ + {"type": "m.room_membership", "room_id": "!c:example.org"} + ] + } +] +``` + +This could be condensed into a singular `restricted` rule, but the room admin is legal in splitting +the rule out. + +This proposal also does not attempt to fix how the room directory, space hierarchy, etc all work +to return a single `join_rule`. Given the `join_rule` key is maintained in the event content, and +that the field should be set to a semantically relevant value, it is assumed that servers can/should +continue using the single `join_rule` key instead of needing to populate the `PublicRoomsChunk` with +the array of possible join rules. When servers are calculating stats and such on the room, they should +also be okay to continue using the single field (unless it is otherwise easier to check the array). + +Finally, this proposal favours backwards compatibility to its own detriment. This is intentional to +give clients (and other areas of the protocol/ecosystem) time to consider what their project might +look like if the join rules system were to change completely. This acts more like an easing function +than a complete refactoring by design. + +## Alternatives + +[MSC3386](https://github.com/matrix-org/matrix-doc/pull/3386) and similar ideas favour a complete +rebuild of the join rules system. While a potentially great change for Matrix, it doesn't feel proper +for the short term. + +## Security considerations + +Room admins can specify insane or otherwise lengthy lists of join rules. Servers should be wary of +these sorts of events when considering their loop usage, however this MSC doesn't forbid using all +65kb of an event to specify join rules. + +As mentioned in the potential issues, room admins can additionally make bad decisions about security. +Clients are encouraged to design and build a user interface which reduces the chances of a `public` +and `invite` rule set. Such an interface might be: + +``` +This room is: +[ ] Public +[x] Private + +Join rules (disabled when Public): +[x] Allow members of `#space:example.org` to join +[x] Allow knocking +``` + +## Unstable prefix + +While this MSC is not considered stable, implementations should use an `org.matrix.msc3613` room version +which uses room version 9 as a base. + +## Dependencies + +No relevant dependencies. + +## Considered but not included + +During a trial implementation of this proposal there were authorization rules added to validate that +a `m.room.join_rules` event is properly structured. While an initial reaction is to include auth rules +for the event, it ultimately doesn't appear important for the protocol. For example, the damage of a +user sending an empty join rules event (or one with unspecified rules) is naturally self-healing by the +room becoming invite only in conditions where the join rules don't match exactly. + +As such, the changes to auth rules are excluded from this proposal.