From d1a78ba2a3272fc6e2b7fb9512431b31a6d3ed58 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 16:48:36 +0200 Subject: Implement restriction on public room creation --- synapse/third_party_rules/access_rules.py | 41 ++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index e3f97bdf3a..2cb77e8115 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -17,7 +17,7 @@ import email.utils from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.types import get_domain_from_id @@ -94,35 +94,52 @@ class RoomAccessRules(object): default rule to the initial state. """ is_direct = config.get("is_direct") - rule = None + preset = config.get("preset") + access_rule = None + join_rule = None # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - rule = event["content"].get("rule") + access_rule = event["content"].get("rule") # Make sure the event has a valid content. - if rule is None: + if access_rule is None: raise SynapseError(400, "Invalid access rule") # Make sure the rule name is valid. - if rule not in VALID_ACCESS_RULES: + if access_rule not in VALID_ACCESS_RULES: raise SynapseError(400, "Invalid access rule") # Make sure the rule is "direct" if the room is a direct chat. if ( - (is_direct and rule != ACCESS_RULE_DIRECT) - or (rule == ACCESS_RULE_DIRECT and not is_direct) + (is_direct and access_rule != ACCESS_RULE_DIRECT) + or (access_rule == ACCESS_RULE_DIRECT and not is_direct) ): raise SynapseError(400, "Invalid access rule") + if event["type"] == EventTypes.JoinRules: + join_rule = event["content"].get("join_rule") + + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # If there's no rules event in the initial state, create one with the default # setting. - if not rule: + if not access_rule: if is_direct: default_rule = ACCESS_RULE_DIRECT else: + # If the default value for non-direct chat changes, we should make another + # case here for rooms created with either a "public" join_rule or the + # "public_chat" preset to make sure those keep defaulting to "restricted" default_rule = ACCESS_RULE_RESTRICTED if not config.get("initial_state"): @@ -136,11 +153,11 @@ class RoomAccessRules(object): } }) - rule = default_rule + access_rule = default_rule # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( - config.get("power_level_content_override", {}), rule, + config.get("power_level_content_override", {}), access_rule, ) if not allowed: raise SynapseError(400, "Invalid power levels content override") @@ -148,7 +165,9 @@ class RoomAccessRules(object): # Second loop for events we need to know the current rule to process. for event in config.get("initial_state", []): if event["type"] == EventTypes.PowerLevels: - allowed = self._is_power_level_content_allowed(event["content"], rule) + allowed = self._is_power_level_content_allowed( + event["content"], access_rule + ) if not allowed: raise SynapseError(400, "Invalid power levels content") -- cgit 1.5.1 From ea5f86304e6be03ce32df16cf1996eb41da93b01 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 17:27:07 +0200 Subject: Implement restrictions on new events --- synapse/third_party_rules/access_rules.py | 41 ++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 2cb77e8115..a7fd9eab2e 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -232,6 +232,9 @@ class RoomAccessRules(object): if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: return self._on_membership_or_invite(event, rule, state_events) + if event.type == EventTypes.JoinRules: + return self._on_join_rule_change(event, rule) + return True def _on_rules_change(self, event, state_events): @@ -251,6 +254,12 @@ class RoomAccessRules(object): if new_rule not in VALID_ACCESS_RULES: return False + # We must not allow rooms with the "public" join rule to be given any other access + # rule than "restricted". + join_rule = self._get_join_rule_from_state(state_events) + if join_rule == JoinRules.PUBLIC and new_rule != ACCESS_RULE_RESTRICTED: + return False + # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( @@ -400,7 +409,6 @@ class RoomAccessRules(object): access_rule (str): The access rule in place in this room. Returns: bool, True if the event can be allowed, False otherwise. - """ # Check if we need to apply the restrictions with the current rule. if access_rule not in RULES_WITH_RESTRICTED_POWER_LEVELS: @@ -424,6 +432,22 @@ class RoomAccessRules(object): return True + def _on_join_rule_change(self, event, rule): + """Check whether a join rule change is allowed. A join rule change is always + allowed unless the new join rule is "public" and the current access rule isn't + "restricted". + + Args: + event (synapse.events.EventBase): The event to check. + rule (str): The name of the rule to apply. + Returns: + bool, True if the event can be allowed, False otherwise. + """ + if event.content.get('join_rule') == JoinRules.PUBLIC: + return rule == ACCESS_RULE_RESTRICTED + + return True + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. @@ -441,6 +465,21 @@ class RoomAccessRules(object): rule = access_rules.content.get("rule") return rule + @staticmethod + def _get_join_rule_from_state(state_events): + """Extract the room's join rule from the given set of state events. + + Args: + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + str, the name of the join rule (either "public", or "invite") + """ + join_rule_event = state_events.get((EventTypes.JoinRules, "")) + if join_rule_event is None: + return "" + return join_rule_event.content.get("join_rule") + @staticmethod def _get_members_and_tokens_from_state(state_events): """Retrieves from a list of state events the list of users that have a -- cgit 1.5.1 From dd92685179655ac555d40390dcab8970261d19da Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:03:36 +0200 Subject: Only check the join rule on room creation if an access rule is also provided --- synapse/third_party_rules/access_rules.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index a7fd9eab2e..786f3d9ad3 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,18 +122,18 @@ class RoomAccessRules(object): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - - # If there's no rules event in the initial state, create one with the default - # setting. - if not access_rule: + if access_rule: + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + else: + # If there's no rules event in the initial state, create one with the default + # setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: -- cgit 1.5.1 From aea03c9d734d3dd5f0650b9d127bc9026266505c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:14:41 +0200 Subject: Doc --- synapse/third_party_rules/access_rules.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 786f3d9ad3..07b449ab32 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -123,6 +123,11 @@ class RoomAccessRules(object): join_rule = event["content"].get("join_rule") if access_rule: + # If there's an access rules event in the initial state, check if the prefix + # or the join rule in use is compatible (i.e. if it involves a "public" join + # rule, the access rule must be "restricted"). We don't need to check that if + # there's no access rule provided, as in this case the access rule will + # default to "restricted", with which any join rule is allowed. if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: raise SynapseError(400, "Invalid access rule") @@ -132,8 +137,8 @@ class RoomAccessRules(object): ): raise SynapseError(400, "Invalid access rule") else: - # If there's no rules event in the initial state, create one with the default - # setting. + # If there's no access rules event in the initial state, create one with the + # default setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: @@ -437,6 +442,13 @@ class RoomAccessRules(object): allowed unless the new join rule is "public" and the current access rule isn't "restricted". + Note that we currently rely on the default access rule being "restricted": during + room creation, the m.room.join_rules event will be sent *before* the + im.vector.room.access_rules one, so the access rule that will be considered here + in this case will be the default "restricted" one. This is fine since the + "restricted" access rule allows any value for the join rule, but we should keep + that in mind if we need to change the default access rule in the future. + Args: event (synapse.events.EventBase): The event to check. rule (str): The name of the rule to apply. -- cgit 1.5.1 From 2526b79ce6923bcf681ecd846b718269833e5a7e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:15:44 +0200 Subject: Merge ifs --- synapse/third_party_rules/access_rules.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 07b449ab32..c37737ea60 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -128,12 +128,11 @@ class RoomAccessRules(object): # rule, the access rule must be "restricted"). We don't need to check that if # there's no access rule provided, as in this case the access rule will # default to "restricted", with which any join rule is allowed. - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED ): raise SynapseError(400, "Invalid access rule") else: -- cgit 1.5.1 From d2bb51080e344b179d5d5e3789e00480b8f5286a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 16:15:01 +0200 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index c37737ea60..56527d6365 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,20 +122,7 @@ class RoomAccessRules(object): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if access_rule: - # If there's an access rules event in the initial state, check if the prefix - # or the join rule in use is compatible (i.e. if it involves a "public" join - # rule, the access rule must be "restricted"). We don't need to check that if - # there's no access rule provided, as in this case the access rule will - # default to "restricted", with which any join rule is allowed. - if ( - ( - join_rule == JoinRules.PUBLIC - or preset == RoomCreationPreset.PUBLIC_CHAT - ) and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - else: + if access_rule is None: # If there's no access rules event in the initial state, create one with the # default setting. if is_direct: @@ -159,6 +146,17 @@ class RoomAccessRules(object): access_rule = default_rule + # Check that the preset or the join rule in use is compatible with the access + # rule, whether it's a user-defined one or the default one (i.e. if it involves + # a "public" join rule, the access rule must be "restricted"). + if ( + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( config.get("power_level_content_override", {}), access_rule, @@ -488,7 +486,7 @@ class RoomAccessRules(object): """ join_rule_event = state_events.get((EventTypes.JoinRules, "")) if join_rule_event is None: - return "" + return None return join_rule_event.content.get("join_rule") @staticmethod -- cgit 1.5.1 From 0c6500a08bbaac34b7630d66339c03dc076b2dbe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 10:19:04 +0200 Subject: Explain rationale --- synapse/third_party_rules/access_rules.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 56527d6365..1a295ea7ce 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -438,6 +438,10 @@ class RoomAccessRules(object): """Check whether a join rule change is allowed. A join rule change is always allowed unless the new join rule is "public" and the current access rule isn't "restricted". + The rationale is that external users (those whose server would be denied access + to rooms enforcing the "restricted" access rule) should always rely on non- + external users for access to rooms, therefore they shouldn't be able to access + rooms that don't require an invite to be joined. Note that we currently rely on the default access rule being "restricted": during room creation, the m.room.join_rules event will be sent *before* the -- cgit 1.5.1