From 488ed3e444cf37cb02c67a2569ec9f39c85583c1 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 9 May 2018 14:40:18 +0100 Subject: [PATCH 1/4] Generic "are users in domain X allowed to invite users in domain Y" logic --- synapse/rulecheck/__init__.py | 0 synapse/rulecheck/domain_rule_checker.py | 78 +++++++++++++++++++ tests/rulecheck/__init__.py | 14 ++++ tests/rulecheck/test_domainrulecheck.py | 96 ++++++++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 synapse/rulecheck/__init__.py create mode 100644 synapse/rulecheck/domain_rule_checker.py create mode 100644 tests/rulecheck/__init__.py create mode 100644 tests/rulecheck/test_domainrulecheck.py diff --git a/synapse/rulecheck/__init__.py b/synapse/rulecheck/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py new file mode 100644 index 000000000..256cca2ed --- /dev/null +++ b/synapse/rulecheck/domain_rule_checker.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 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 logging + +from synapse.config._base import ConfigError + +logger = logging.getLogger(__name__) + +""" +DomainRuleChecker + +Takes a config in the format: + +spam_checker: + module: "rulecheck.DomainRuleChecker" + config: + domain_mapping: + "inviter_domain": [ "invitee_domain_permitted", "other_invitee_domain_permitted" ] + "other_inviter_domain": [ "invitee_domain_permitted" ] + default: False + } + +Don't forget to consider if you can invite users from your own domain. +""" +class DomainRuleChecker(object): + + def __init__(self, config): + self.domain_mapping = config["domain_mapping"] or {} + self.default = config["default"] + + def check_event_for_spam(self, event): + return False + + def user_may_invite(self, inviter_userid, invitee_userid, room_id): + inviter_domain = self._get_domain_from_id(inviter_userid) + invitee_domain = self._get_domain_from_id(invitee_userid) + + valid_targets = self.domain_mapping.get(inviter_domain) + if not valid_targets: + return self.default + + return invitee_domain in valid_targets + + def user_may_create_room(self, userid): + return True + + def user_may_create_room_alias(self, userid, room_alias): + return True + + def user_may_publish_room(self, userid, room_id): + return True + + @staticmethod + def parse_config(config): + if "default" in config: + return config + else: + raise ConfigError("No default set for spam_config DomainRuleChecker") + + @staticmethod + def _get_domain_from_id(string): + idx = string.find(":") + if idx == -1: + raise Exception("Invalid ID: %r" % (string,)) + return string[idx + 1:] diff --git a/tests/rulecheck/__init__.py b/tests/rulecheck/__init__.py new file mode 100644 index 000000000..a354d38ca --- /dev/null +++ b/tests/rulecheck/__init__.py @@ -0,0 +1,14 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 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/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py new file mode 100644 index 000000000..141c8e444 --- /dev/null +++ b/tests/rulecheck/test_domainrulecheck.py @@ -0,0 +1,96 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 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 tests import unittest + +from synapse.config._base import ConfigError +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker + + +class DomainRuleCheckerTestCase(unittest.TestCase): + + def test_allowed(self): + config = { + "default": False, + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + check = DomainRuleChecker(config) + self.assertTrue(check.user_may_invite("test:source_one","test:target_one", "room")) + self.assertTrue(check.user_may_invite("test:source_one","test:target_two", "room")) + self.assertTrue(check.user_may_invite("test:source_two","test:target_two", "room")) + + + def test_disallowed(self): + config = { + "default": True, + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + check = DomainRuleChecker(config) + self.assertFalse(check.user_may_invite("test:source_one","test:target_three", "room")) + self.assertFalse(check.user_may_invite("test:source_two","test:target_three", "room")) + self.assertFalse(check.user_may_invite("test:source_two","test:target_one", "room")) + + + def test_default_allow(self): + config = { + "default": True, + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + check = DomainRuleChecker(config) + self.assertTrue(check.user_may_invite("test:source_three","test:target_one", "room")) + + def test_default_deny(self): + config = { + "default": False, + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + check = DomainRuleChecker(config) + self.assertFalse(check.user_may_invite("test:source_three","test:target_one", "room")) + + + def test_config_parse(self): + config = { + "default": False, + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + self.assertEquals(config, DomainRuleChecker.parse_config(config)) + + + def test_config_parse_failure(self): + config = { + "domain_mapping": { + "source_one": [ "target_one", "target_two" ], + "source_two": [ "target_two" ] + } + } + self.assertRaises(ConfigError, DomainRuleChecker.parse_config, config) + + From 1e8cfc9e77a671c0f5b6818e285c3a10bdff567b Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 9 May 2018 15:11:19 +0100 Subject: [PATCH 2/4] pep8 style fixes --- synapse/rulecheck/domain_rule_checker.py | 2 + tests/rulecheck/test_domainrulecheck.py | 82 ++++++++++++------------ 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py index 256cca2ed..c9771e0b6 100644 --- a/synapse/rulecheck/domain_rule_checker.py +++ b/synapse/rulecheck/domain_rule_checker.py @@ -35,6 +35,8 @@ spam_checker: Don't forget to consider if you can invite users from your own domain. """ + + class DomainRuleChecker(object): def __init__(self, config): diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py index 141c8e444..1c068a397 100644 --- a/tests/rulecheck/test_domainrulecheck.py +++ b/tests/rulecheck/test_domainrulecheck.py @@ -24,73 +24,75 @@ class DomainRuleCheckerTestCase(unittest.TestCase): def test_allowed(self): config = { - "default": False, - "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] - } + "default": False, + "domain_mapping": { + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] + } } check = DomainRuleChecker(config) - self.assertTrue(check.user_may_invite("test:source_one","test:target_one", "room")) - self.assertTrue(check.user_may_invite("test:source_one","test:target_two", "room")) - self.assertTrue(check.user_may_invite("test:source_two","test:target_two", "room")) - + self.assertTrue(check.user_may_invite("test:source_one", + "test:target_one", "room")) + self.assertTrue(check.user_may_invite("test:source_one", + "test:target_two", "room")) + self.assertTrue(check.user_may_invite("test:source_two", + "test:target_two", "room")) def test_disallowed(self): config = { "default": True, "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] } } check = DomainRuleChecker(config) - self.assertFalse(check.user_may_invite("test:source_one","test:target_three", "room")) - self.assertFalse(check.user_may_invite("test:source_two","test:target_three", "room")) - self.assertFalse(check.user_may_invite("test:source_two","test:target_one", "room")) - + self.assertFalse(check.user_may_invite("test:source_one", + "test:target_three", "room")) + self.assertFalse(check.user_may_invite("test:source_two", + "test:target_three", "room")) + self.assertFalse(check.user_may_invite("test:source_two", + "test:target_one", "room")) def test_default_allow(self): config = { - "default": True, - "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] - } + "default": True, + "domain_mapping": { + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] + } } check = DomainRuleChecker(config) - self.assertTrue(check.user_may_invite("test:source_three","test:target_one", "room")) + self.assertTrue(check.user_may_invite("test:source_three", + "test:target_one", "room")) def test_default_deny(self): config = { - "default": False, - "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] - } + "default": False, + "domain_mapping": { + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] + } } check = DomainRuleChecker(config) - self.assertFalse(check.user_may_invite("test:source_three","test:target_one", "room")) - + self.assertFalse(check.user_may_invite("test:source_three", + "test:target_one", "room")) def test_config_parse(self): config = { - "default": False, - "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] - } + "default": False, + "domain_mapping": { + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] + } } self.assertEquals(config, DomainRuleChecker.parse_config(config)) - def test_config_parse_failure(self): config = { - "domain_mapping": { - "source_one": [ "target_one", "target_two" ], - "source_two": [ "target_two" ] - } + "domain_mapping": { + "source_one": ["target_one", "target_two"], + "source_two": ["target_two"] + } } self.assertRaises(ConfigError, DomainRuleChecker.parse_config, config) - - From 11f1bace3c464da9f67f17d462cf7f9c199c48ac Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 11 May 2018 12:47:58 +0100 Subject: [PATCH 3/4] Address PR feedback - add code and test to handle configuration of an empty array - move docstrings around and update class level documentation --- synapse/rulecheck/domain_rule_checker.py | 39 ++++++++++++------------ tests/rulecheck/test_domainrulecheck.py | 5 ++- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py index c9771e0b6..a750fe753 100644 --- a/synapse/rulecheck/domain_rule_checker.py +++ b/synapse/rulecheck/domain_rule_checker.py @@ -19,25 +19,25 @@ from synapse.config._base import ConfigError logger = logging.getLogger(__name__) -""" -DomainRuleChecker - -Takes a config in the format: - -spam_checker: - module: "rulecheck.DomainRuleChecker" - config: - domain_mapping: - "inviter_domain": [ "invitee_domain_permitted", "other_invitee_domain_permitted" ] - "other_inviter_domain": [ "invitee_domain_permitted" ] - default: False - } - -Don't forget to consider if you can invite users from your own domain. -""" - class DomainRuleChecker(object): + """ + A re-implementation of the SpamChecker that prevents users in one domain from + inviting users in other domains to rooms, based on a configuration. + + Takes a config in the format: + + spam_checker: + module: "rulecheck.DomainRuleChecker" + config: + domain_mapping: + "inviter_domain": [ "invitee_domain_permitted", "other_domain_permitted" ] + "other_inviter_domain": [ "invitee_domain_permitted" ] + default: False + } + + Don't forget to consider if you can invite users from your own domain. + """ def __init__(self, config): self.domain_mapping = config["domain_mapping"] or {} @@ -50,11 +50,10 @@ class DomainRuleChecker(object): inviter_domain = self._get_domain_from_id(inviter_userid) invitee_domain = self._get_domain_from_id(invitee_userid) - valid_targets = self.domain_mapping.get(inviter_domain) - if not valid_targets: + if inviter_domain not in self.domain_mapping: return self.default - return invitee_domain in valid_targets + return invitee_domain in self.domain_mapping.get(inviter_domain) def user_may_create_room(self, userid): return True diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py index 1c068a397..a24fb5376 100644 --- a/tests/rulecheck/test_domainrulecheck.py +++ b/tests/rulecheck/test_domainrulecheck.py @@ -43,7 +43,8 @@ class DomainRuleCheckerTestCase(unittest.TestCase): "default": True, "domain_mapping": { "source_one": ["target_one", "target_two"], - "source_two": ["target_two"] + "source_two": ["target_two"], + "source_four": [] } } check = DomainRuleChecker(config) @@ -53,6 +54,8 @@ class DomainRuleCheckerTestCase(unittest.TestCase): "test:target_three", "room")) self.assertFalse(check.user_may_invite("test:source_two", "test:target_one", "room")) + self.assertFalse(check.user_may_invite("test:source_four", + "test:target_one", "room")) def test_default_allow(self): config = { From 81beae30b803fdd4fec87b1b1150bcc2ff19ae70 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 18 May 2018 16:12:22 +0100 Subject: [PATCH 4/4] Update with documentation suggestions --- synapse/rulecheck/domain_rule_checker.py | 31 ++++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py index a750fe753..3caa6b34c 100644 --- a/synapse/rulecheck/domain_rule_checker.py +++ b/synapse/rulecheck/domain_rule_checker.py @@ -44,36 +44,57 @@ class DomainRuleChecker(object): self.default = config["default"] def check_event_for_spam(self, event): + """Implements synapse.events.SpamChecker.check_event_for_spam + """ return False def user_may_invite(self, inviter_userid, invitee_userid, room_id): + """Implements synapse.events.SpamChecker.user_may_invite + """ inviter_domain = self._get_domain_from_id(inviter_userid) invitee_domain = self._get_domain_from_id(invitee_userid) if inviter_domain not in self.domain_mapping: return self.default - return invitee_domain in self.domain_mapping.get(inviter_domain) + return invitee_domain in self.domain_mapping[inviter_domain] def user_may_create_room(self, userid): + """Implements synapse.events.SpamChecker.user_may_create_room + """ return True def user_may_create_room_alias(self, userid, room_alias): + """Implements synapse.events.SpamChecker.user_may_create_room_alias + """ return True def user_may_publish_room(self, userid, room_id): + """Implements synapse.events.SpamChecker.user_may_publish_room + """ return True @staticmethod def parse_config(config): + """Implements synapse.events.SpamChecker.parse_config + """ if "default" in config: return config else: raise ConfigError("No default set for spam_config DomainRuleChecker") @staticmethod - def _get_domain_from_id(string): - idx = string.find(":") + def _get_domain_from_id(mxid): + """Parses a string and returns the domain part of the mxid. + + Args: + mxid (str): a valid mxid + + Returns: + str: the domain part of the mxid + + """ + idx = mxid.find(":") if idx == -1: - raise Exception("Invalid ID: %r" % (string,)) - return string[idx + 1:] + raise Exception("Invalid ID: %r" % (mxid,)) + return mxid[idx + 1:]