From 110608d1d6f5aec1610284e65a07a64cfda0978a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 14 Jun 2019 14:43:12 +0100 Subject: Base --- synapse/tchap/__init__.py | 14 ++++++++++++++ synapse/tchap/event_rules.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 synapse/tchap/__init__.py create mode 100644 synapse/tchap/event_rules.py diff --git a/synapse/tchap/__init__.py b/synapse/tchap/__init__.py new file mode 100644 index 0000000000..1453d04571 --- /dev/null +++ b/synapse/tchap/__init__.py @@ -0,0 +1,14 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py new file mode 100644 index 0000000000..f82883c642 --- /dev/null +++ b/synapse/tchap/event_rules.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from twisted.internet import defer + + +class TchapEventRules(object): + + def __init__(self, config): + # We don't have a config yet. + pass + + @staticmethod + def parse_config(config): + return config + + @defer.inlineCallbacks + def check_event_allowed(self, event, context): + return True -- cgit 1.5.1 From bd8448ccb26e54291cc754f29a0fca88fa9b0515 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 14 Jun 2019 17:26:37 +0100 Subject: Backbone --- synapse/tchap/event_rules.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index f82883c642..e8acad7577 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -18,6 +18,8 @@ from twisted.internet import defer class TchapEventRules(object): + ACESS_RULES_TYPE = "im.vector.room.access_rules" + def __init__(self, config): # We don't have a config yet. pass @@ -29,3 +31,12 @@ class TchapEventRules(object): @defer.inlineCallbacks def check_event_allowed(self, event, context): return True + + def _apply_restricted(self): + pass + + def _apply_unrestricted(self): + pass + + def _apply_direct(self): + pass -- cgit 1.5.1 From 8b2f655589e992c7e60a0846e005cb380c723f96 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 15:13:27 +0100 Subject: Implement rules for direct --- synapse/tchap/event_rules.py | 112 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 12 deletions(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index e8acad7577..fecfbefede 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -15,28 +15,116 @@ from twisted.internet import defer +from synapse.api.constants import EventTypes, Membership +from synapse.config._base import ConfigError -class TchapEventRules(object): +ACESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULE_RESTRICTED = "restricted" +ACCESS_RULE_UNRESTRICTED = "unrestricted" +ACCESS_RULE_DIRECT = "direct" - ACESS_RULES_TYPE = "im.vector.room.access_rules" +class TchapEventRules(object): def __init__(self, config): - # We don't have a config yet. - pass + self.id_server = config["id_server"] @staticmethod def parse_config(config): - return config + if "id_server" in config: + return config + else: + raise ConfigError("No IS for event rules TchapEventRules") + + def check_event_allowed(self, event, state_events): + # TODO: add rules for sending the access rules event + access_rules = state_events.get((ACESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + + if rule == ACCESS_RULE_RESTRICTED: + ret = self._apply_restricted(event, state_events) + elif rule == ACCESS_RULE_UNRESTRICTED: + ret = self._apply_unrestricted(event, state_events) + elif rule == ACCESS_RULE_DIRECT: + ret = self._apply_direct(event, state_events) + else: + # We currently apply the default (restricted) if we don't know the rule, we + # might want to change that in the future. + ret = self._apply_restricted(event, state_events) + + return ret @defer.inlineCallbacks - def check_event_allowed(self, event, context): + def _apply_restricted(self, event, state_events): return True - def _apply_restricted(self): - pass + def _apply_unrestricted(self, event, state_events): + # "unrestricted" currently means that every event is allowed. + return True + + def _apply_direct(self, event, state_events): + # "direct" currently means that no member is allowed apart from the two initial + # members the room was created for (i.e. the room's creator and their first + # invitee). + # TODO: figure out how to know if the room was created with "is_direct". + + if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + return True + + # Get the m.room.member and m.room.third_party_invite events from the room's + # state. + member_events = [] + threepid_invite_events = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events.append(event) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_events.append(event) - def _apply_unrestricted(self): - pass + # There should never be more than one 3PID invite in the room state: + if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + # If we already have a 3PID invite in flight, don't accept another one. + return False + + if len(member_events) == 2: + # If the user was within the two initial user of the room, Synapse would have + # looked it up successfully and thus sent a m.room.member here instead of + # m.room.third_party_invite. + if event.type == EventTypes.ThirdPartyInvite: + return False + + # We can only have m.room.member events here. The rule in this case is to only + # allow the event if its target is one of the initial two members in the room, + # i.e. the state key of one of the two m.room.member states in the room. + target = event.state_key + for e in member_events: + if e.state_key == target: + return True + + return False + + # We're alone in the room (and always have been) and there's one 3PID invite in + # flight. + if len(member_events) == 1 and len(threepid_invite_events) == 1: + # We can only have m.room.member events here. In this case, we can only allow + # the event if it's either a m.room.member from the joined user (we can assume + # that the only m.room.member event is a join otherwise we wouldn't be able to + # send an event to the room) or an a m.room.member with the "invite" + # membership which target is the invited user. + target = event.state_key + is_from_threepid_invite = self._is_invite_from_threepid( + event, threepid_invite_events[0], + + ) + if is_from_threepid_invite or target == member_events[0].state_key: + return True + + return False + + return True - def _apply_direct(self): - pass + def _is_invite_from_threepid(self, invite, threepid_invite): + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite.state_key -- cgit 1.5.1 From 53aff08ebe3d1ba07ee7a1a117fc3d33e14cc644 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 17:57:59 +0100 Subject: Implement restricted rules and room creation hook --- synapse/tchap/event_rules.py | 54 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index fecfbefede..471083fb1a 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -17,8 +17,9 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership from synapse.config._base import ConfigError +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker -ACESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULES_TYPE = "im.vector.room.access_rules" ACCESS_RULE_RESTRICTED = "restricted" ACCESS_RULE_UNRESTRICTED = "unrestricted" ACCESS_RULE_DIRECT = "direct" @@ -27,6 +28,9 @@ ACCESS_RULE_DIRECT = "direct" class TchapEventRules(object): def __init__(self, config): self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( + "domains_forbidden_when_restricted", [], + ) @staticmethod def parse_config(config): @@ -35,18 +39,45 @@ class TchapEventRules(object): else: raise ConfigError("No IS for event rules TchapEventRules") + def on_create_room(self, requester, config, is_requester_admin): + for event in config.get("initial_state", []): + if event["type"] == ACCESS_RULES_TYPE: + # If there's already a rules event in the initial state, check if it + # breaks the rules for "direct", and if not don't do anything else. + if ( + not config.get("is_direct") + or event["content"]["rule"] != ACCESS_RULE_DIRECT + ): + return + + # Append an access rules event to be sent once every other event in initial_state + # has been sent. If "is_direct" exists and is set to True, the rule needs to be + # "direct", and "restricted" otherwise. + if config.get("is_direct"): + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": default_rule, + } + }) + def check_event_allowed(self, event, state_events): # TODO: add rules for sending the access rules event - access_rules = state_events.get((ACESS_RULES_TYPE, "")) + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) if access_rules is None: rule = ACCESS_RULE_RESTRICTED else: rule = access_rules.content.get("rule") if rule == ACCESS_RULE_RESTRICTED: - ret = self._apply_restricted(event, state_events) + ret = self._apply_restricted(event) elif rule == ACCESS_RULE_UNRESTRICTED: - ret = self._apply_unrestricted(event, state_events) + ret = self._apply_unrestricted() elif rule == ACCESS_RULE_DIRECT: ret = self._apply_direct(event, state_events) else: @@ -56,11 +87,13 @@ class TchapEventRules(object): return ret - @defer.inlineCallbacks - def _apply_restricted(self, event, state_events): - return True + def _apply_restricted(self, event): + # "restricted" currently means that users can only invite users if their server is + # included in a limited list of domains. + invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + return invitee_domain not in self.domains_forbidden_when_restricted - def _apply_unrestricted(self, event, state_events): + def _apply_unrestricted(self): # "unrestricted" currently means that every event is allowed. return True @@ -68,8 +101,6 @@ class TchapEventRules(object): # "direct" currently means that no member is allowed apart from the two initial # members the room was created for (i.e. the room's creator and their first # invitee). - # TODO: figure out how to know if the room was created with "is_direct". - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True @@ -125,6 +156,7 @@ class TchapEventRules(object): return True - def _is_invite_from_threepid(self, invite, threepid_invite): + @staticmethod + def _is_invite_from_threepid(invite, threepid_invite): token = invite.content.get("third_party_signed", {}).get("token", "") return token == threepid_invite.state_key -- cgit 1.5.1 From 112a48a5aa53c49794de988066f846ba4ae5fec1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 18:53:13 +0100 Subject: Implement 3PID invite hook --- synapse/tchap/event_rules.py | 56 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 471083fb1a..624336dab3 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -13,9 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import email.utils + from twisted.internet import defer -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EventTypes from synapse.config._base import ConfigError from synapse.rulecheck.domain_rule_checker import DomainRuleChecker @@ -26,7 +28,8 @@ ACCESS_RULE_DIRECT = "direct" class TchapEventRules(object): - def __init__(self, config): + def __init__(self, config, http_client): + self.http_client = http_client self.id_server = config["id_server"] self.domains_forbidden_when_restricted = config.get( "domains_forbidden_when_restricted", [], @@ -66,13 +69,43 @@ class TchapEventRules(object): } }) + @defer.inlineCallbacks + def check_threepid_can_be_invited(self, medium, address, state_events): + rule = self._get_rule_from_state(state_events) + + if medium != "email": + defer.returnValue(False) + + if rule != ACCESS_RULE_RESTRICTED: + # Only "restricted" requires filtering 3PID invites. + defer.returnValue(True) + + parsed_address = email.utils.parseaddr(address)[1] + if parsed_address != address: + # Avoid reproducing the security issue described here: + # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 + # It's probably not worth it but let's just be overly safe here. + defer.returnValue(False) + + res = yield self.http_client.get_json( + "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), + { + "medium": medium, + "address": address, + } + ) + + # Look for a domain that's not forbidden from being invited. + if not res.get("hs"): + defer.returnValue(False) + if res.get("hs") in self.domains_forbidden_when_restricted: + defer.returnValue(False) + + defer.returnValue(True) + def check_event_allowed(self, event, state_events): # TODO: add rules for sending the access rules event - access_rules = state_events.get((ACCESS_RULES_TYPE, "")) - if access_rules is None: - rule = ACCESS_RULE_RESTRICTED - else: - rule = access_rules.content.get("rule") + rule = self._get_rule_from_state(state_events) if rule == ACCESS_RULE_RESTRICTED: ret = self._apply_restricted(event) @@ -156,6 +189,15 @@ class TchapEventRules(object): return True + @staticmethod + def _get_rule_from_state(state_events): + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + return rule + @staticmethod def _is_invite_from_threepid(invite, threepid_invite): token = invite.content.get("third_party_signed", {}).get("token", "") -- cgit 1.5.1 From 00736c8935b544112f61b5cdc283569ad76136a3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:21:09 +0100 Subject: Improve doc --- synapse/tchap/event_rules.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 624336dab3..7a2bec8072 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -87,6 +87,7 @@ class TchapEventRules(object): # It's probably not worth it but let's just be overly safe here. defer.returnValue(False) + # Get the HS this address belongs to from the identity server. res = yield self.http_client.get_json( "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), { @@ -147,7 +148,11 @@ class TchapEventRules(object): if key[0] == EventTypes.ThirdPartyInvite: threepid_invite_events.append(event) - # There should never be more than one 3PID invite in the room state: + # There should never be more than one 3PID invite in the room state: if the second + # original user came and left, and we're inviting them using their email address, + # given we know they have a Matrix account binded to the address (so they could + # join the first time), Synapse will successfully look it up before attempting to + # store an invite on the IS. if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: # If we already have a 3PID invite in flight, don't accept another one. return False @@ -175,8 +180,8 @@ class TchapEventRules(object): # We can only have m.room.member events here. In this case, we can only allow # the event if it's either a m.room.member from the joined user (we can assume # that the only m.room.member event is a join otherwise we wouldn't be able to - # send an event to the room) or an a m.room.member with the "invite" - # membership which target is the invited user. + # send an event to the room) or an an invite event which target is the invited + # user. target = event.state_key is_from_threepid_invite = self._is_invite_from_threepid( event, threepid_invite_events[0], -- cgit 1.5.1 From 073dd7778e0f9fdfdfcce53ef4c9d8a96cffeb63 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:21:58 +0100 Subject: Implement rules change --- synapse/tchap/event_rules.py | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 7a2bec8072..18d6d3bd6f 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -105,7 +105,10 @@ class TchapEventRules(object): defer.returnValue(True) def check_event_allowed(self, event, state_events): - # TODO: add rules for sending the access rules event + # Special-case the access rules event. + if event.type == ACCESS_RULES_TYPE: + return self._on_rules_change(event, state_events) + rule = self._get_rule_from_state(state_events) if rule == ACCESS_RULE_RESTRICTED: @@ -121,6 +124,42 @@ class TchapEventRules(object): return ret + def _on_rules_change(self, event, state_events): + new_rule = event.content.get("rule") + + # Check for invalid values. + if ( + new_rule != ACCESS_RULE_DIRECT + and new_rule != ACCESS_RULE_RESTRICTED + and new_rule != ACCESS_RULE_UNRESTRICTED + ): + return False + + # Make sure we don't apply "direct" if the room has more than two members. + if new_rule == ACCESS_RULE_DIRECT: + member_events_count = 0 + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events_count += 1 + + if member_events_count > 2: + return False + + prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) + + # Now that we know the new rule doesn't break the "direct" case, we can allow any + # new rule in rooms that had none before. + if prev_rules_event is None: + return True + + prev_rule = prev_rules_event.content.get("rule") + + # Currently, we can only go from "restricted" to "unrestricted". + if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: + return True + + return False + def _apply_restricted(self, event): # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. -- cgit 1.5.1 From 19a4298a51a99f4a920693824bdba8f4758de805 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:22:31 +0100 Subject: Fix function call --- synapse/tchap/event_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 18d6d3bd6f..a724b22790 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -120,7 +120,7 @@ class TchapEventRules(object): else: # We currently apply the default (restricted) if we don't know the rule, we # might want to change that in the future. - ret = self._apply_restricted(event, state_events) + ret = self._apply_restricted(event) return ret -- cgit 1.5.1 From 1cd0ecc1f22be02bfb11794cb472d333fd82d238 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:29:42 +0100 Subject: Rename into RoomAccessRules --- synapse/tchap/__init__.py | 14 -- synapse/tchap/event_rules.py | 248 ------------------------------ synapse/third_party_rules/__init__.py | 14 ++ synapse/third_party_rules/access_rules.py | 248 ++++++++++++++++++++++++++++++ 4 files changed, 262 insertions(+), 262 deletions(-) delete mode 100644 synapse/tchap/__init__.py delete mode 100644 synapse/tchap/event_rules.py create mode 100644 synapse/third_party_rules/__init__.py create mode 100644 synapse/third_party_rules/access_rules.py diff --git a/synapse/tchap/__init__.py b/synapse/tchap/__init__.py deleted file mode 100644 index 1453d04571..0000000000 --- a/synapse/tchap/__init__.py +++ /dev/null @@ -1,14 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2019 New Vector Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py deleted file mode 100644 index a724b22790..0000000000 --- a/synapse/tchap/event_rules.py +++ /dev/null @@ -1,248 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2019 New Vector Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import email.utils - -from twisted.internet import defer - -from synapse.api.constants import EventTypes -from synapse.config._base import ConfigError -from synapse.rulecheck.domain_rule_checker import DomainRuleChecker - -ACCESS_RULES_TYPE = "im.vector.room.access_rules" -ACCESS_RULE_RESTRICTED = "restricted" -ACCESS_RULE_UNRESTRICTED = "unrestricted" -ACCESS_RULE_DIRECT = "direct" - - -class TchapEventRules(object): - def __init__(self, config, http_client): - self.http_client = http_client - self.id_server = config["id_server"] - self.domains_forbidden_when_restricted = config.get( - "domains_forbidden_when_restricted", [], - ) - - @staticmethod - def parse_config(config): - if "id_server" in config: - return config - else: - raise ConfigError("No IS for event rules TchapEventRules") - - def on_create_room(self, requester, config, is_requester_admin): - for event in config.get("initial_state", []): - if event["type"] == ACCESS_RULES_TYPE: - # If there's already a rules event in the initial state, check if it - # breaks the rules for "direct", and if not don't do anything else. - if ( - not config.get("is_direct") - or event["content"]["rule"] != ACCESS_RULE_DIRECT - ): - return - - # Append an access rules event to be sent once every other event in initial_state - # has been sent. If "is_direct" exists and is set to True, the rule needs to be - # "direct", and "restricted" otherwise. - if config.get("is_direct"): - default_rule = ACCESS_RULE_DIRECT - else: - default_rule = ACCESS_RULE_RESTRICTED - - config["initial_state"].append({ - "type": ACCESS_RULES_TYPE, - "state_key": "", - "content": { - "rule": default_rule, - } - }) - - @defer.inlineCallbacks - def check_threepid_can_be_invited(self, medium, address, state_events): - rule = self._get_rule_from_state(state_events) - - if medium != "email": - defer.returnValue(False) - - if rule != ACCESS_RULE_RESTRICTED: - # Only "restricted" requires filtering 3PID invites. - defer.returnValue(True) - - parsed_address = email.utils.parseaddr(address)[1] - if parsed_address != address: - # Avoid reproducing the security issue described here: - # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 - # It's probably not worth it but let's just be overly safe here. - defer.returnValue(False) - - # Get the HS this address belongs to from the identity server. - res = yield self.http_client.get_json( - "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), - { - "medium": medium, - "address": address, - } - ) - - # Look for a domain that's not forbidden from being invited. - if not res.get("hs"): - defer.returnValue(False) - if res.get("hs") in self.domains_forbidden_when_restricted: - defer.returnValue(False) - - defer.returnValue(True) - - def check_event_allowed(self, event, state_events): - # Special-case the access rules event. - if event.type == ACCESS_RULES_TYPE: - return self._on_rules_change(event, state_events) - - rule = self._get_rule_from_state(state_events) - - if rule == ACCESS_RULE_RESTRICTED: - ret = self._apply_restricted(event) - elif rule == ACCESS_RULE_UNRESTRICTED: - ret = self._apply_unrestricted() - elif rule == ACCESS_RULE_DIRECT: - ret = self._apply_direct(event, state_events) - else: - # We currently apply the default (restricted) if we don't know the rule, we - # might want to change that in the future. - ret = self._apply_restricted(event) - - return ret - - def _on_rules_change(self, event, state_events): - new_rule = event.content.get("rule") - - # Check for invalid values. - if ( - new_rule != ACCESS_RULE_DIRECT - and new_rule != ACCESS_RULE_RESTRICTED - and new_rule != ACCESS_RULE_UNRESTRICTED - ): - return False - - # Make sure we don't apply "direct" if the room has more than two members. - if new_rule == ACCESS_RULE_DIRECT: - member_events_count = 0 - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events_count += 1 - - if member_events_count > 2: - return False - - prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) - - # Now that we know the new rule doesn't break the "direct" case, we can allow any - # new rule in rooms that had none before. - if prev_rules_event is None: - return True - - prev_rule = prev_rules_event.content.get("rule") - - # Currently, we can only go from "restricted" to "unrestricted". - if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: - return True - - return False - - def _apply_restricted(self, event): - # "restricted" currently means that users can only invite users if their server is - # included in a limited list of domains. - invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) - return invitee_domain not in self.domains_forbidden_when_restricted - - def _apply_unrestricted(self): - # "unrestricted" currently means that every event is allowed. - return True - - def _apply_direct(self, event, state_events): - # "direct" currently means that no member is allowed apart from the two initial - # members the room was created for (i.e. the room's creator and their first - # invitee). - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: - return True - - # Get the m.room.member and m.room.third_party_invite events from the room's - # state. - member_events = [] - threepid_invite_events = [] - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events.append(event) - if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_events.append(event) - - # There should never be more than one 3PID invite in the room state: if the second - # original user came and left, and we're inviting them using their email address, - # given we know they have a Matrix account binded to the address (so they could - # join the first time), Synapse will successfully look it up before attempting to - # store an invite on the IS. - if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: - # If we already have a 3PID invite in flight, don't accept another one. - return False - - if len(member_events) == 2: - # If the user was within the two initial user of the room, Synapse would have - # looked it up successfully and thus sent a m.room.member here instead of - # m.room.third_party_invite. - if event.type == EventTypes.ThirdPartyInvite: - return False - - # We can only have m.room.member events here. The rule in this case is to only - # allow the event if its target is one of the initial two members in the room, - # i.e. the state key of one of the two m.room.member states in the room. - target = event.state_key - for e in member_events: - if e.state_key == target: - return True - - return False - - # We're alone in the room (and always have been) and there's one 3PID invite in - # flight. - if len(member_events) == 1 and len(threepid_invite_events) == 1: - # We can only have m.room.member events here. In this case, we can only allow - # the event if it's either a m.room.member from the joined user (we can assume - # that the only m.room.member event is a join otherwise we wouldn't be able to - # send an event to the room) or an an invite event which target is the invited - # user. - target = event.state_key - is_from_threepid_invite = self._is_invite_from_threepid( - event, threepid_invite_events[0], - - ) - if is_from_threepid_invite or target == member_events[0].state_key: - return True - - return False - - return True - - @staticmethod - def _get_rule_from_state(state_events): - access_rules = state_events.get((ACCESS_RULES_TYPE, "")) - if access_rules is None: - rule = ACCESS_RULE_RESTRICTED - else: - rule = access_rules.content.get("rule") - return rule - - @staticmethod - def _is_invite_from_threepid(invite, threepid_invite): - token = invite.content.get("third_party_signed", {}).get("token", "") - return token == threepid_invite.state_key diff --git a/synapse/third_party_rules/__init__.py b/synapse/third_party_rules/__init__.py new file mode 100644 index 0000000000..1453d04571 --- /dev/null +++ b/synapse/third_party_rules/__init__.py @@ -0,0 +1,14 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py new file mode 100644 index 0000000000..1f03138752 --- /dev/null +++ b/synapse/third_party_rules/access_rules.py @@ -0,0 +1,248 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import email.utils + +from twisted.internet import defer + +from synapse.api.constants import EventTypes +from synapse.config._base import ConfigError +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker + +ACCESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULE_RESTRICTED = "restricted" +ACCESS_RULE_UNRESTRICTED = "unrestricted" +ACCESS_RULE_DIRECT = "direct" + + +class RoomAccessRules(object): + def __init__(self, config, http_client): + self.http_client = http_client + self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( + "domains_forbidden_when_restricted", [], + ) + + @staticmethod + def parse_config(config): + if "id_server" in config: + return config + else: + raise ConfigError("No IS for event rules TchapEventRules") + + def on_create_room(self, requester, config, is_requester_admin): + for event in config.get("initial_state", []): + if event["type"] == ACCESS_RULES_TYPE: + # If there's already a rules event in the initial state, check if it + # breaks the rules for "direct", and if not don't do anything else. + if ( + not config.get("is_direct") + or event["content"]["rule"] != ACCESS_RULE_DIRECT + ): + return + + # Append an access rules event to be sent once every other event in initial_state + # has been sent. If "is_direct" exists and is set to True, the rule needs to be + # "direct", and "restricted" otherwise. + if config.get("is_direct"): + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": default_rule, + } + }) + + @defer.inlineCallbacks + def check_threepid_can_be_invited(self, medium, address, state_events): + rule = self._get_rule_from_state(state_events) + + if medium != "email": + defer.returnValue(False) + + if rule != ACCESS_RULE_RESTRICTED: + # Only "restricted" requires filtering 3PID invites. + defer.returnValue(True) + + parsed_address = email.utils.parseaddr(address)[1] + if parsed_address != address: + # Avoid reproducing the security issue described here: + # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 + # It's probably not worth it but let's just be overly safe here. + defer.returnValue(False) + + # Get the HS this address belongs to from the identity server. + res = yield self.http_client.get_json( + "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), + { + "medium": medium, + "address": address, + } + ) + + # Look for a domain that's not forbidden from being invited. + if not res.get("hs"): + defer.returnValue(False) + if res.get("hs") in self.domains_forbidden_when_restricted: + defer.returnValue(False) + + defer.returnValue(True) + + def check_event_allowed(self, event, state_events): + # Special-case the access rules event. + if event.type == ACCESS_RULES_TYPE: + return self._on_rules_change(event, state_events) + + rule = self._get_rule_from_state(state_events) + + if rule == ACCESS_RULE_RESTRICTED: + ret = self._apply_restricted(event) + elif rule == ACCESS_RULE_UNRESTRICTED: + ret = self._apply_unrestricted() + elif rule == ACCESS_RULE_DIRECT: + ret = self._apply_direct(event, state_events) + else: + # We currently apply the default (restricted) if we don't know the rule, we + # might want to change that in the future. + ret = self._apply_restricted(event) + + return ret + + def _on_rules_change(self, event, state_events): + new_rule = event.content.get("rule") + + # Check for invalid values. + if ( + new_rule != ACCESS_RULE_DIRECT + and new_rule != ACCESS_RULE_RESTRICTED + and new_rule != ACCESS_RULE_UNRESTRICTED + ): + return False + + # Make sure we don't apply "direct" if the room has more than two members. + if new_rule == ACCESS_RULE_DIRECT: + member_events_count = 0 + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events_count += 1 + + if member_events_count > 2: + return False + + prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) + + # Now that we know the new rule doesn't break the "direct" case, we can allow any + # new rule in rooms that had none before. + if prev_rules_event is None: + return True + + prev_rule = prev_rules_event.content.get("rule") + + # Currently, we can only go from "restricted" to "unrestricted". + if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: + return True + + return False + + def _apply_restricted(self, event): + # "restricted" currently means that users can only invite users if their server is + # included in a limited list of domains. + invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + return invitee_domain not in self.domains_forbidden_when_restricted + + def _apply_unrestricted(self): + # "unrestricted" currently means that every event is allowed. + return True + + def _apply_direct(self, event, state_events): + # "direct" currently means that no member is allowed apart from the two initial + # members the room was created for (i.e. the room's creator and their first + # invitee). + if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + return True + + # Get the m.room.member and m.room.third_party_invite events from the room's + # state. + member_events = [] + threepid_invite_events = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events.append(event) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_events.append(event) + + # There should never be more than one 3PID invite in the room state: if the second + # original user came and left, and we're inviting them using their email address, + # given we know they have a Matrix account binded to the address (so they could + # join the first time), Synapse will successfully look it up before attempting to + # store an invite on the IS. + if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + # If we already have a 3PID invite in flight, don't accept another one. + return False + + if len(member_events) == 2: + # If the user was within the two initial user of the room, Synapse would have + # looked it up successfully and thus sent a m.room.member here instead of + # m.room.third_party_invite. + if event.type == EventTypes.ThirdPartyInvite: + return False + + # We can only have m.room.member events here. The rule in this case is to only + # allow the event if its target is one of the initial two members in the room, + # i.e. the state key of one of the two m.room.member states in the room. + target = event.state_key + for e in member_events: + if e.state_key == target: + return True + + return False + + # We're alone in the room (and always have been) and there's one 3PID invite in + # flight. + if len(member_events) == 1 and len(threepid_invite_events) == 1: + # We can only have m.room.member events here. In this case, we can only allow + # the event if it's either a m.room.member from the joined user (we can assume + # that the only m.room.member event is a join otherwise we wouldn't be able to + # send an event to the room) or an an invite event which target is the invited + # user. + target = event.state_key + is_from_threepid_invite = self._is_invite_from_threepid( + event, threepid_invite_events[0], + + ) + if is_from_threepid_invite or target == member_events[0].state_key: + return True + + return False + + return True + + @staticmethod + def _get_rule_from_state(state_events): + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + return rule + + @staticmethod + def _is_invite_from_threepid(invite, threepid_invite): + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite.state_key -- cgit 1.5.1 From 8aea2c3be12d8f96d96fc53cb4880d56ce4c8dc4 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 20:09:53 +0100 Subject: Docstrings --- synapse/third_party_rules/access_rules.py | 181 +++++++++++++++++++++++++----- 1 file changed, 154 insertions(+), 27 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 1f03138752..a8b3ed9458 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -28,9 +28,31 @@ ACCESS_RULE_DIRECT = "direct" class RoomAccessRules(object): + """Implementation of the ThirdPartyEventRules module API that allows federation admins + to define custom rules for specific events and actions. + Implements the custom behaviour for the "im.vector.room.access_rules" state event. + + Takes a config in the format: + + third_party_event_rules: + module: third_party_rules.RoomAccessRules + config: + # List of domains (server names) that can't be invited to rooms if the + # "restricted" rule is set. Defaults to an empty list. + domains_forbidden_when_restricted: [] + + # Identity server to use when checking the HS an email address belongs to + # using the /info endpoint. Required. + id_server: "vector.im" + + Don't forget to consider if you can invite users from your own domain. + """ + def __init__(self, config, http_client): self.http_client = http_client + self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( "domains_forbidden_when_restricted", [], ) @@ -43,34 +65,77 @@ class RoomAccessRules(object): raise ConfigError("No IS for event rules TchapEventRules") def on_create_room(self, requester, config, is_requester_admin): + """Implements synapse.events.ThirdPartyEventRules.on_create_room + + Checks if a im.vector.room.access_rules event is being set during room creation. + If yes, make sure the event is correct. Otherwise, append an event with the + default rule to the initial state. + """ + is_direct = config.get("is_direct") + rules_in_initial_state = False + + # If there's a rules event in the initial state, check if it complies with the + # spec for im.vector.room.access_rules and fix it if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - # If there's already a rules event in the initial state, check if it - # breaks the rules for "direct", and if not don't do anything else. - if ( - not config.get("is_direct") - or event["content"]["rule"] != ACCESS_RULE_DIRECT - ): - return - - # Append an access rules event to be sent once every other event in initial_state - # has been sent. If "is_direct" exists and is set to True, the rule needs to be - # "direct", and "restricted" otherwise. - if config.get("is_direct"): - default_rule = ACCESS_RULE_DIRECT - else: - default_rule = ACCESS_RULE_RESTRICTED + rules_in_initial_state = True + + rule = event["content"].get("rule") + + # Make sure the event has a valid content. + if rule is None: + event["content"] = { + "rule": self._on_create_room_default_rule(is_direct) + } + + # Make sure the rule name is valid. + if not self._is_rule_name_valid(rule): + event["content"]["rule"] = self._on_create_room_default_rule( + is_direct, + ) + + # Make sure the rule is "direct" if the room is a direct chat. + if is_direct and rule != ACCESS_RULE_DIRECT: + event["content"]["rule"] = ACCESS_RULE_DIRECT + + # Make sure the rule is not "direct" if the room isn't a direct chat. + if rule == ACCESS_RULE_DIRECT and not is_direct: + event["content"]["rule"] = ACCESS_RULE_RESTRICTED + + # If there's no rules event in the initial state, create one with the default + # setting. + if not rules_in_initial_state: + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": self._on_create_room_default_rule(is_direct), + } + }) - config["initial_state"].append({ - "type": ACCESS_RULES_TYPE, - "state_key": "", - "content": { - "rule": default_rule, - } - }) + @staticmethod + def _on_create_room_default_rule(is_direct): + """Returns the default rule to set. + + Args: + is_direct (bool): Is the room created with "is_direct" set to True. + + Returns: + str, the name of the rule tu use as the default. + """ + if is_direct: + return ACCESS_RULE_DIRECT + else: + return ACCESS_RULE_RESTRICTED @defer.inlineCallbacks def check_threepid_can_be_invited(self, medium, address, state_events): + """Implements synapse.events.ThirdPartyEventRules.check_threepid_can_be_invited + + Check if a threepid can be invited to the room via a 3PID invite given the current + rules and the threepid's address, by retrieving the HS it's mapped to from the + configured identity server, and checking if we can invite users from it. + """ rule = self._get_rule_from_state(state_events) if medium != "email": @@ -105,6 +170,11 @@ class RoomAccessRules(object): defer.returnValue(True) def check_event_allowed(self, event, state_events): + """Implements synapse.events.ThirdPartyEventRules.check_event_allowed + + Checks the event's type and the current rule and calls the right function to + determine whether the event can be allowed. + """ # Special-case the access rules event. if event.type == ACCESS_RULES_TYPE: return self._on_rules_change(event, state_events) @@ -125,14 +195,20 @@ class RoomAccessRules(object): return ret def _on_rules_change(self, event, state_events): + """Implement the checks and behaviour specified on allowing or forbidding a new + im.vector.room.access_rules event. + + Args: + event (synapse.events.EventBase): The event to check. + state_events (dict[tuple[event type, state key], EventBase]): The state of the + room before the event was sent. + Returns: + bool, True if the event can be allowed, False otherwise. + """ new_rule = event.content.get("rule") # Check for invalid values. - if ( - new_rule != ACCESS_RULE_DIRECT - and new_rule != ACCESS_RULE_RESTRICTED - and new_rule != ACCESS_RULE_UNRESTRICTED - ): + if not self._is_rule_name_valid(new_rule): return False # Make sure we don't apply "direct" if the room has more than two members. @@ -161,16 +237,37 @@ class RoomAccessRules(object): return False def _apply_restricted(self, event): + """Implements the checks and behaviour specified for the "restricted" rule. + + Args: + event (synapse.events.EventBase): The event to check. + Returns: + bool, True if the event can be allowed, False otherwise. + """ # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted def _apply_unrestricted(self): + """Implements the checks and behaviour specified for the "unrestricted" rule. + + Returns: + bool, True if the event can be allowed, False otherwise. + """ # "unrestricted" currently means that every event is allowed. return True def _apply_direct(self, event, state_events): + """Implements the checks and behaviour specified for the "direct" rule. + + Args: + event (synapse.events.EventBase): The event to check. + state_events (dict[tuple[event type, state key], EventBase]): The state of the + room before the event was sent. + Returns: + bool, True if the event can be allowed, False otherwise. + """ # "direct" currently means that no member is allowed apart from the two initial # members the room was created for (i.e. the room's creator and their first # invitee). @@ -235,6 +332,14 @@ class RoomAccessRules(object): @staticmethod def _get_rule_from_state(state_events): + """Extract the rule to be applied 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 rule (either "direct", "restricted" or "unrestricted") + """ access_rules = state_events.get((ACCESS_RULES_TYPE, "")) if access_rules is None: rule = ACCESS_RULE_RESTRICTED @@ -244,5 +349,27 @@ class RoomAccessRules(object): @staticmethod def _is_invite_from_threepid(invite, threepid_invite): + """Checks whether the given invite follows the given 3PID invite. + + Args: + invite (EventBase): The m.room.member event with "invite" membership. + threepid_invite (EventBase): The m.room.third_party_invite event. + """ token = invite.content.get("third_party_signed", {}).get("token", "") return token == threepid_invite.state_key + + @staticmethod + def _is_rule_name_valid(rule): + """Returns whether the given rule name is within the allowed values ("direct", + "restricted" or "unrestricted"). + + Args: + rule (str): The name of the rule. + Returns: + bool, True if the name is valid, False otherwise. + """ + return ( + rule == ACCESS_RULE_DIRECT + or rule == ACCESS_RULE_RESTRICTED + or rule == ACCESS_RULE_UNRESTRICTED + ) -- cgit 1.5.1 From f7339d42ee572f892c12501896a477c8f927b94e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 11:28:33 +0100 Subject: Fixes --- synapse/third_party_rules/access_rules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index a8b3ed9458..07a1649584 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -105,6 +105,9 @@ class RoomAccessRules(object): # If there's no rules event in the initial state, create one with the default # setting. if not rules_in_initial_state: + if not config.get("initial_state"): + config["initial_state"] = [] + config["initial_state"].append({ "type": ACCESS_RULES_TYPE, "state_key": "", @@ -246,6 +249,8 @@ class RoomAccessRules(object): """ # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. + if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + return True invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted -- cgit 1.5.1 From d36a876d2d4571321e8a989c5bfb73a3b2e1dc1f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:53:33 +0100 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 147 ++++++++++++++---------------- 1 file changed, 69 insertions(+), 78 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 07a1649584..096e23e149 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -18,14 +18,21 @@ import email.utils from twisted.internet import defer from synapse.api.constants import EventTypes +from synapse.api.errors import SynapseError from synapse.config._base import ConfigError -from synapse.rulecheck.domain_rule_checker import DomainRuleChecker +from synapse.types import get_domain_from_id ACCESS_RULES_TYPE = "im.vector.room.access_rules" ACCESS_RULE_RESTRICTED = "restricted" ACCESS_RULE_UNRESTRICTED = "unrestricted" ACCESS_RULE_DIRECT = "direct" +VALID_ACCESS_RULES = ( + ACCESS_RULE_DIRECT, + ACCESS_RULE_RESTRICTED, + ACCESS_RULE_UNRESTRICTED, +) + class RoomAccessRules(object): """Implementation of the ThirdPartyEventRules module API that allows federation admins @@ -75,7 +82,7 @@ class RoomAccessRules(object): rules_in_initial_state = False # If there's a rules event in the initial state, check if it complies with the - # spec for im.vector.room.access_rules and fix it if not. + # 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: rules_in_initial_state = True @@ -84,27 +91,27 @@ class RoomAccessRules(object): # Make sure the event has a valid content. if rule is None: - event["content"] = { - "rule": self._on_create_room_default_rule(is_direct) - } + raise SynapseError(400, "Invalid access rule",) # Make sure the rule name is valid. - if not self._is_rule_name_valid(rule): - event["content"]["rule"] = self._on_create_room_default_rule( - is_direct, - ) + if 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: - event["content"]["rule"] = ACCESS_RULE_DIRECT - - # Make sure the rule is not "direct" if the room isn't a direct chat. - if rule == ACCESS_RULE_DIRECT and not is_direct: - event["content"]["rule"] = ACCESS_RULE_RESTRICTED + if ( + (is_direct and rule != ACCESS_RULE_DIRECT) + or (rule == ACCESS_RULE_DIRECT and not is_direct) + ): + raise SynapseError(400, "Invalid access rule",) # If there's no rules event in the initial state, create one with the default # setting. if not rules_in_initial_state: + if is_direct: + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + if not config.get("initial_state"): config["initial_state"] = [] @@ -112,25 +119,10 @@ class RoomAccessRules(object): "type": ACCESS_RULES_TYPE, "state_key": "", "content": { - "rule": self._on_create_room_default_rule(is_direct), + "rule": default_rule, } }) - @staticmethod - def _on_create_room_default_rule(is_direct): - """Returns the default rule to set. - - Args: - is_direct (bool): Is the room created with "is_direct" set to True. - - Returns: - str, the name of the rule tu use as the default. - """ - if is_direct: - return ACCESS_RULE_DIRECT - else: - return ACCESS_RULE_RESTRICTED - @defer.inlineCallbacks def check_threepid_can_be_invited(self, medium, address, state_events): """Implements synapse.events.ThirdPartyEventRules.check_threepid_can_be_invited @@ -145,7 +137,9 @@ class RoomAccessRules(object): defer.returnValue(False) if rule != ACCESS_RULE_RESTRICTED: - # Only "restricted" requires filtering 3PID invites. + # Only "restricted" requires filtering 3PID invites. We don't need to do + # anything for "direct" here, because only "restricted" requires filtering + # based on the HS the address is mapped to. defer.returnValue(True) parsed_address = email.utils.parseaddr(address)[1] @@ -211,17 +205,16 @@ class RoomAccessRules(object): new_rule = event.content.get("rule") # Check for invalid values. - if not self._is_rule_name_valid(new_rule): + if new_rule not in VALID_ACCESS_RULES: return False # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: - member_events_count = 0 - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events_count += 1 + existing_members, threepid_tokens = self._get_members_and_tokens_from_state( + state_events, + ) - if member_events_count > 2: + if len(existing_members) > 2 or len(threepid_tokens) > 1: return False prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) @@ -251,7 +244,7 @@ class RoomAccessRules(object): # included in a limited list of domains. if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True - invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted def _apply_unrestricted(self): @@ -279,26 +272,21 @@ class RoomAccessRules(object): if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True - # Get the m.room.member and m.room.third_party_invite events from the room's - # state. - member_events = [] - threepid_invite_events = [] - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events.append(event) - if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_events.append(event) + # Get the room memberships and 3PID invite tokens from the room's state. + existing_members, threepid_tokens = self._get_members_and_tokens_from_state( + state_events, + ) # There should never be more than one 3PID invite in the room state: if the second # original user came and left, and we're inviting them using their email address, # given we know they have a Matrix account binded to the address (so they could # join the first time), Synapse will successfully look it up before attempting to # store an invite on the IS. - if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + if len(threepid_tokens) == 1 and event.type == EventTypes.ThirdPartyInvite: # If we already have a 3PID invite in flight, don't accept another one. return False - if len(member_events) == 2: + if len(existing_members) == 2: # If the user was within the two initial user of the room, Synapse would have # looked it up successfully and thus sent a m.room.member here instead of # m.room.third_party_invite. @@ -308,16 +296,11 @@ class RoomAccessRules(object): # We can only have m.room.member events here. The rule in this case is to only # allow the event if its target is one of the initial two members in the room, # i.e. the state key of one of the two m.room.member states in the room. - target = event.state_key - for e in member_events: - if e.state_key == target: - return True - - return False + return event.state_key in existing_members # We're alone in the room (and always have been) and there's one 3PID invite in # flight. - if len(member_events) == 1 and len(threepid_invite_events) == 1: + if len(existing_members) == 1 and len(threepid_tokens) == 1: # We can only have m.room.member events here. In this case, we can only allow # the event if it's either a m.room.member from the joined user (we can assume # that the only m.room.member event is a join otherwise we wouldn't be able to @@ -325,10 +308,9 @@ class RoomAccessRules(object): # user. target = event.state_key is_from_threepid_invite = self._is_invite_from_threepid( - event, threepid_invite_events[0], - + event, threepid_tokens[0], ) - if is_from_threepid_invite or target == member_events[0].state_key: + if is_from_threepid_invite or target == existing_members[0]: return True return False @@ -341,7 +323,7 @@ class RoomAccessRules(object): Args: state_events (dict[tuple[event type, state key], EventBase]): The set of state - events + events. Returns: str, the name of the rule (either "direct", "restricted" or "unrestricted") """ @@ -353,28 +335,37 @@ class RoomAccessRules(object): return rule @staticmethod - def _is_invite_from_threepid(invite, threepid_invite): - """Checks whether the given invite follows the given 3PID invite. + def _get_members_and_tokens_from_state(state_events): + """Retrieves from a list of state events the list of users that have a + m.room.member event in the room, and the tokens of 3PID invites in the room. Args: - invite (EventBase): The m.room.member event with "invite" membership. - threepid_invite (EventBase): The m.room.third_party_invite event. + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + existing_members (list[str]): List of targets of the m.room.member events in + the state. + threepid_invite_tokens (list[str]): List of tokens of the 3PID invites in the + state. """ - token = invite.content.get("third_party_signed", {}).get("token", "") - return token == threepid_invite.state_key + existing_members = [] + threepid_invite_tokens = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + existing_members.append(event.state_key) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_tokens.append(event.state_key) + + return existing_members, threepid_invite_tokens + @staticmethod - def _is_rule_name_valid(rule): - """Returns whether the given rule name is within the allowed values ("direct", - "restricted" or "unrestricted"). + def _is_invite_from_threepid(invite, threepid_invite_token): + """Checks whether the given invite follows the given 3PID invite. Args: - rule (str): The name of the rule. - Returns: - bool, True if the name is valid, False otherwise. + invite (EventBase): The m.room.member event with "invite" membership. + threepid_invite_token (str): The state key from the 3PID invite. """ - return ( - rule == ACCESS_RULE_DIRECT - or rule == ACCESS_RULE_RESTRICTED - or rule == ACCESS_RULE_UNRESTRICTED - ) + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite_token -- cgit 1.5.1 From bc0fd8f17023d235809fcdb8ed901a766873eced Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:54:19 +0100 Subject: Add tests for room creation hook --- tests/rest/client/test_room_access_rules.py | 136 ++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/rest/client/test_room_access_rules.py diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py new file mode 100644 index 0000000000..85347b3b0c --- /dev/null +++ b/tests/rest/client/test_room_access_rules.py @@ -0,0 +1,136 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +import json + +import synapse.rest.admin +from synapse.config._base import ConfigError +from synapse.rest import admin +from synapse.rest.client.v1 import login, room +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker +from synapse.third_party_rules.access_rules import ( + ACCESS_RULES_TYPE, + ACCESS_RULE_DIRECT, + ACCESS_RULE_RESTRICTED, + ACCESS_RULE_UNRESTRICTED, +) + +from tests import unittest + + +class RoomAccessEventTestCase(unittest.HomeserverTestCase): + + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def make_homeserver(self, reactor, clock): + config = self.default_config() + + config["third_party_event_rules"] = { + "module": "synapse.third_party_rules.access_rules.RoomAccessRules", + "config": { + "domains_forbidden_when_restricted": [ + "forbidden_domain" + ], + "id_server": "testis", + } + } + + self.hs = self.setup_test_homeserver(config=config) + + return self.hs + + def prepare(self, reactor, clock, homeserver): + self.user_id = self.register_user("kermit", "monkey") + self.tok = self.login("kermit", "monkey") + + self.restricted_room = self.create_room() + self.unrestricted_room = self.create_room(rule=ACCESS_RULE_UNRESTRICTED) + self.direct_room = self.create_room(direct=True) + + def test_create_room_no_rule(self): + """Tests that creating a room with no rule will set the default value.""" + room_id = self.create_room() + rule = self.current_rule_in_room(room_id) + + self.assertEqual(rule, ACCESS_RULE_RESTRICTED) + + def test_create_room_direct_no_rule(self): + """Tests that creating a direct room with no rule will set the default value.""" + room_id = self.create_room(direct=True) + rule = self.current_rule_in_room(room_id) + + self.assertEqual(rule, ACCESS_RULE_DIRECT) + + def test_create_room_valid_rule(self): + """Tests that creating a room with a valid rule will set the right value.""" + room_id = self.create_room(rule=ACCESS_RULE_UNRESTRICTED) + rule = self.current_rule_in_room(room_id) + + self.assertEqual(rule, ACCESS_RULE_UNRESTRICTED) + + def test_create_room_invalid_rule(self): + """Tests that creating a room with an invalid rule will set the default value.""" + self.create_room(rule=ACCESS_RULE_DIRECT, expected_code=400) + + def test_create_room_direct_invalid_rule(self): + """Tests that creating a direct room with an invalid rule will set the default + value. + """ + self.create_room(direct=True, rule=ACCESS_RULE_RESTRICTED, expected_code=400) + + # def test_limited + + def create_room(self, direct=False, rule=None, expected_code=200): + content = { + "is_direct": direct, + } + + if rule: + content["initial_state"] = [{ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": rule, + } + }] + + request, channel = self.make_request( + "POST", + "/_matrix/client/r0/createRoom", + json.dumps(content), + access_token=self.tok, + ) + self.render(request) + + self.assertEqual(channel.code, expected_code, channel.result) + + if expected_code == 200: + return channel.json_body["room_id"] + + def current_rule_in_room(self, room_id): + request, channel = self.make_request( + "GET", + "/_matrix/client/r0/rooms/%s/state/%s" % (room_id, ACCESS_RULES_TYPE), + access_token=self.tok, + ) + self.render(request) + + self.assertEqual(channel.code, 200, channel.result) + return channel.json_body["rule"] -- cgit 1.5.1 From a09767d57d701b624854cc1e6868ae3840fb3845 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:55:24 +0100 Subject: Lint --- tests/rest/client/test_room_access_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 85347b3b0c..8052fae94e 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -22,10 +22,10 @@ from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.rulecheck.domain_rule_checker import DomainRuleChecker from synapse.third_party_rules.access_rules import ( - ACCESS_RULES_TYPE, ACCESS_RULE_DIRECT, ACCESS_RULE_RESTRICTED, ACCESS_RULE_UNRESTRICTED, + ACCESS_RULES_TYPE, ) from tests import unittest -- cgit 1.5.1 From 2a1f35193b478a9c097061dbec92e22fb1cbdb8f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:57:04 +0100 Subject: Remove unused imports --- tests/rest/client/test_room_access_rules.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 8052fae94e..1f43902421 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -16,11 +16,8 @@ import json -import synapse.rest.admin -from synapse.config._base import ConfigError from synapse.rest import admin from synapse.rest.client.v1 import login, room -from synapse.rulecheck.domain_rule_checker import DomainRuleChecker from synapse.third_party_rules.access_rules import ( ACCESS_RULE_DIRECT, ACCESS_RULE_RESTRICTED, -- cgit 1.5.1 From cefc5542fb6ad0cd3f4b03f0d720d0cd91eb1a52 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:59:05 +0100 Subject: Lint --- synapse/third_party_rules/access_rules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 096e23e149..fbd829cdd2 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -358,7 +358,6 @@ class RoomAccessRules(object): return existing_members, threepid_invite_tokens - @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): """Checks whether the given invite follows the given 3PID invite. -- cgit 1.5.1 From 145291108da2c550eac45037122ec723d694ee97 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 16:32:54 +0100 Subject: Add tests for inviting with access rules --- tests/rest/client/test_room_access_rules.py | 104 +++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 1f43902421..de655fa580 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -16,6 +16,11 @@ import json +from mock import Mock + +from twisted.internet import defer + +from synapse.federation.federation_base import event_from_pdu_json from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.third_party_rules.access_rules import ( @@ -49,7 +54,18 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): } } - self.hs = self.setup_test_homeserver(config=config) + def send_invite(destination, room_id, event_id, pdu): + return defer.succeed(pdu) + + federation_client = Mock(spec=[ + "send_invite", + ]) + federation_client.send_invite.side_effect = send_invite + + self.hs = self.setup_test_homeserver( + config=config, + federation_client=federation_client, + ) return self.hs @@ -61,6 +77,16 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): self.unrestricted_room = self.create_room(rule=ACCESS_RULE_UNRESTRICTED) self.direct_room = self.create_room(direct=True) + self.invitee_id = self.register_user("invitee", "test") + self.invitee_tok = self.login("invitee", "test") + + self.helper.invite( + room=self.direct_room, + src=self.user_id, + targ=self.invitee_id, + tok=self.tok, + ) + def test_create_room_no_rule(self): """Tests that creating a room with no rule will set the default value.""" room_id = self.create_room() @@ -92,7 +118,81 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): """ self.create_room(direct=True, rule=ACCESS_RULE_RESTRICTED, expected_code=400) - # def test_limited + def test_restricted(self): + """Tests that in restricted mode we're unable to invite users from blacklisted + servers but can invite other users. + """ + self.helper.invite( + room=self.restricted_room, + src=self.user_id, + targ="@test:forbidden_domain", + tok=self.tok, + expect_code=403, + ) + + self.helper.invite( + room=self.restricted_room, + src=self.user_id, + targ="@test:not_forbidden_domain", + tok=self.tok, + expect_code=200, + ) + + def test_direct(self): + """Tests that, in direct mode, other users than the initial two can't be invited, + but the following scenario works: + * invited user joins the room + * invited user leaves the room + * room creator re-invites invited user + """ + self.helper.invite( + room=self.direct_room, + src=self.user_id, + targ="@not_invited:test", + tok=self.tok, + expect_code=403, + ) + + self.helper.join( + room=self.direct_room, + user=self.invitee_id, + tok=self.invitee_tok, + expect_code=200, + ) + + self.helper.leave( + room=self.direct_room, + user=self.invitee_id, + tok=self.invitee_tok, + expect_code=200, + ) + + self.helper.invite( + room=self.direct_room, + src=self.user_id, + targ=self.invitee_id, + tok=self.tok, + expect_code=200, + ) + + def test_unrestricted(self): + """Tests that, in unrestricted mode, we can invite whoever we want. + """ + self.helper.invite( + room=self.unrestricted_room, + src=self.user_id, + targ="@test:forbidden_domain", + tok=self.tok, + expect_code=200, + ) + + self.helper.invite( + room=self.unrestricted_room, + src=self.user_id, + targ="@test:not_forbidden_domain", + tok=self.tok, + expect_code=200, + ) def create_room(self, direct=False, rule=None, expected_code=200): content = { -- cgit 1.5.1 From 1532369dcd54b6d9ecfb2e5c7b66f9e288e67c4c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 16:46:57 +0100 Subject: Remove unused import --- tests/rest/client/test_room_access_rules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index de655fa580..01cbb3f4d8 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -20,7 +20,6 @@ from mock import Mock from twisted.internet import defer -from synapse.federation.federation_base import event_from_pdu_json from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.third_party_rules.access_rules import ( -- cgit 1.5.1 From 71572761240b18645b16adb0bd607d2092865ba0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 17:43:34 +0100 Subject: Don't process 3PIDs in _apply_restricted --- synapse/third_party_rules/access_rules.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index fbd829cdd2..d13f8de888 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -242,7 +242,9 @@ class RoomAccessRules(object): """ # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + # We're not filtering on m.room.third_party_member events here because the + # filtering on threepids is done in check_threepid_can_be_invited. + if event.type != EventTypes.Member: return True invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted -- cgit 1.5.1 From 9b3c69f66129819bac87b78c91b072df8e57846c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 17:59:07 +0100 Subject: Add tests for 3PID invites --- tests/rest/client/test_room_access_rules.py | 158 +++++++++++++++++++++++++--- 1 file changed, 146 insertions(+), 12 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 01cbb3f4d8..4b8359762a 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -15,6 +15,8 @@ import json +import random +import string from mock import Mock @@ -32,7 +34,7 @@ from synapse.third_party_rules.access_rules import ( from tests import unittest -class RoomAccessEventTestCase(unittest.HomeserverTestCase): +class RoomAccessTestCase(unittest.HomeserverTestCase): servlets = [ admin.register_servlets, @@ -52,18 +54,51 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): "id_server": "testis", } } + config["trusted_third_party_id_servers"] = [ + "testis", + ] def send_invite(destination, room_id, event_id, pdu): return defer.succeed(pdu) - federation_client = Mock(spec=[ + def get_json(uri, args={}, headers=None): + address_domain = args["address"].split("@")[1] + return defer.succeed({"hs": address_domain}) + + def post_urlencoded_get_json(uri, args={}, headers=None): + token = ''.join(random.choice(string.ascii_letters) for _ in range(10)) + return defer.succeed({ + "token": token, + "public_keys": [ + { + "public_key": "serverpublickey", + "key_validity_url": "https://testis/pubkey/isvalid", + }, + { + "public_key": "phemeralpublickey", + "key_validity_url": "https://testis/pubkey/ephemeral/isvalid", + }, + ], + "display_name": "f...@b...", + }) + + mock_federation_client = Mock(spec=[ "send_invite", ]) - federation_client.send_invite.side_effect = send_invite + mock_federation_client.send_invite.side_effect = send_invite + mock_http_client = Mock(spec=[ + "get_json", + "post_urlencoded_get_json" + ]) + # Mocking the response for /info on the IS API. + mock_http_client.get_json.side_effect = get_json + # Mocking the response for /store-invite on the IS API. + mock_http_client.post_urlencoded_get_json.side_effect = post_urlencoded_get_json self.hs = self.setup_test_homeserver( config=config, - federation_client=federation_client, + federation_client=mock_federation_client, + simple_http_client=mock_http_client, ) return self.hs @@ -74,13 +109,17 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): self.restricted_room = self.create_room() self.unrestricted_room = self.create_room(rule=ACCESS_RULE_UNRESTRICTED) - self.direct_room = self.create_room(direct=True) + self.direct_rooms = [ + self.create_room(direct=True), + self.create_room(direct=True), + self.create_room(direct=True), + ] self.invitee_id = self.register_user("invitee", "test") self.invitee_tok = self.login("invitee", "test") self.helper.invite( - room=self.direct_room, + room=self.direct_rooms[0], src=self.user_id, targ=self.invitee_id, tok=self.tok, @@ -121,6 +160,7 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): """Tests that in restricted mode we're unable to invite users from blacklisted servers but can invite other users. """ + # We can't invite a user from a forbidden HS. self.helper.invite( room=self.restricted_room, src=self.user_id, @@ -129,54 +169,117 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): expect_code=403, ) + # We can invite a user which HS isn't forbidden. self.helper.invite( room=self.restricted_room, src=self.user_id, - targ="@test:not_forbidden_domain", + targ="@test:allowed_domain", tok=self.tok, expect_code=200, ) + # We can't send a 3PID invite to an address that is mapped to a forbidden HS. + self.send_threepid_invite( + address="test@forbidden_domain", + room_id=self.restricted_room, + expected_code=403, + ) + + # We can send a 3PID invite to an address that is mapped to an HS that's not + # forbidden. + self.send_threepid_invite( + address="test@allowed_domain", + room_id=self.restricted_room, + expected_code=200, + ) + def test_direct(self): """Tests that, in direct mode, other users than the initial two can't be invited, but the following scenario works: * invited user joins the room * invited user leaves the room * room creator re-invites invited user + Also tests that a user from a HS that's in the list of forbidden domains (to use + in restricted mode) can be invited. """ + not_invited_user = "@not_invited:forbidden_domain" + + # We can't invite a new user to the room. self.helper.invite( - room=self.direct_room, + room=self.direct_rooms[0], src=self.user_id, - targ="@not_invited:test", + targ=not_invited_user, tok=self.tok, expect_code=403, ) + # The invited user can join the room. self.helper.join( - room=self.direct_room, + room=self.direct_rooms[0], user=self.invitee_id, tok=self.invitee_tok, expect_code=200, ) + # The invited user can leave the room. self.helper.leave( - room=self.direct_room, + room=self.direct_rooms[0], user=self.invitee_id, tok=self.invitee_tok, expect_code=200, ) + # The invited user can be re-invited to the room. self.helper.invite( - room=self.direct_room, + room=self.direct_rooms[0], src=self.user_id, targ=self.invitee_id, tok=self.tok, expect_code=200, ) + # If we're alone in the room and have always been the only member, we can invite + # someone. + self.helper.invite( + room=self.direct_rooms[1], + src=self.user_id, + targ=not_invited_user, + tok=self.tok, + expect_code=200, + ) + + # We can't send a 3PID invite to a room that already has two members. + self.send_threepid_invite( + address="test@allowed_domain", + room_id=self.direct_rooms[0], + expected_code=403, + ) + + # We can't send a 3PID invite to a room that already has a pending invite. + self.send_threepid_invite( + address="test@allowed_domain", + room_id=self.direct_rooms[1], + expected_code=403, + ) + + # We can send a 3PID invite to a room in which we've always been the only member. + self.send_threepid_invite( + address="test@forbidden_domain", + room_id=self.direct_rooms[2], + expected_code=200, + ) + + # We can send a 3PID invite to a room in which there's a 3PID invite. + self.send_threepid_invite( + address="test@forbidden_domain", + room_id=self.direct_rooms[2], + expected_code=403, + ) + def test_unrestricted(self): """Tests that, in unrestricted mode, we can invite whoever we want. """ + # We can invite self.helper.invite( room=self.unrestricted_room, src=self.user_id, @@ -193,6 +296,21 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): expect_code=200, ) + # We can send a 3PID invite to an address that is mapped to a forbidden HS. + self.send_threepid_invite( + address="test@forbidden_domain", + room_id=self.unrestricted_room, + expected_code=200, + ) + + # We can send a 3PID invite to an address that is mapped to an HS that's not + # forbidden. + self.send_threepid_invite( + address="test@allowed_domain", + room_id=self.unrestricted_room, + expected_code=200, + ) + def create_room(self, direct=False, rule=None, expected_code=200): content = { "is_direct": direct, @@ -230,3 +348,19 @@ class RoomAccessEventTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200, channel.result) return channel.json_body["rule"] + + def send_threepid_invite(self, address, room_id, expected_code=200): + params = { + "id_server": "testis", + "medium": "email", + "address": address, + } + + request, channel = self.make_request( + "POST", + "/_matrix/client/r0/rooms/%s/invite" % room_id, + json.dumps(params), + access_token=self.tok, + ) + self.render(request) + self.assertEqual(channel.code, expected_code, channel.result) -- cgit 1.5.1 From c1bc48f9d4dcb127d31723e03fe9d9df1cd484c6 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 18:07:05 +0100 Subject: Add tests for constraints on changing the rule for a room --- tests/rest/client/test_room_access_rules.py | 60 +++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 4b8359762a..5c46dabf32 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -311,6 +311,52 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expected_code=200, ) + def test_change_rules(self): + """Tests that we can only change the current rule from restricted to + unrestricted. + """ + # We can change the rule from restricted to unrestricted. + self.change_rule_in_room( + room_id=self.restricted_room, + new_rule=ACCESS_RULE_UNRESTRICTED, + expected_code=200, + ) + + # We can't change the rule from restricted to direct. + self.change_rule_in_room( + room_id=self.restricted_room, + new_rule=ACCESS_RULE_DIRECT, + expected_code=403, + ) + + # We can't change the rule from unrestricted to restricted. + self.change_rule_in_room( + room_id=self.unrestricted_room, + new_rule=ACCESS_RULE_RESTRICTED, + expected_code=403, + ) + + # We can't change the rule from unrestricted to direct. + self.change_rule_in_room( + room_id=self.unrestricted_room, + new_rule=ACCESS_RULE_DIRECT, + expected_code=403, + ) + + # We can't change the rule from direct to restricted. + self.change_rule_in_room( + room_id=self.direct_rooms[0], + new_rule=ACCESS_RULE_RESTRICTED, + expected_code=403, + ) + + # We can't change the rule from direct to unrestricted. + self.change_rule_in_room( + room_id=self.direct_rooms[0], + new_rule=ACCESS_RULE_UNRESTRICTED, + expected_code=403, + ) + def create_room(self, direct=False, rule=None, expected_code=200): content = { "is_direct": direct, @@ -349,6 +395,20 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 200, channel.result) return channel.json_body["rule"] + def change_rule_in_room(self, room_id, new_rule, expected_code=200): + data = { + "rule": new_rule, + } + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/rooms/%s/state/%s" % (room_id, ACCESS_RULES_TYPE), + json.dumps(data), + access_token=self.tok, + ) + self.render(request) + + self.assertEqual(channel.code, expected_code, channel.result) + def send_threepid_invite(self, address, room_id, expected_code=200): params = { "id_server": "testis", -- cgit 1.5.1