From dcdfdb3f17bd3b6b3d4f7ea933c7dc0450976b39 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 16:38:49 +0200 Subject: [PATCH 01/26] Add test case --- tests/rest/client/test_room_access_rules.py | 108 +++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index df48a89e9305..83da1238bab6 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -22,7 +22,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.rest import admin from synapse.rest.client.v1 import login, room from synapse.third_party_rules.access_rules import ( @@ -156,6 +156,84 @@ def test_create_room_direct_invalid_rule(self): """ self.create_room(direct=True, rule=ACCESS_RULE_RESTRICTED, expected_code=400) + def test_public_room(self): + """Tests that it's not possible to have a room with the public join rule and an + access rule that's not restricted. + """ + # Creating a room with the public_chat preset should succeed and set the access + # rule to restricted. + preset_room_id = self.create_room(preset=RoomCreationPreset.PUBLIC_CHAT) + self.assertEqual( + self.current_rule_in_room(preset_room_id), ACCESS_RULE_RESTRICTED, + ) + + # Creating a room with the public join rule in its initial state should succeed + # and set the access rule to restricted. + init_state_room_id = self.create_room(initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }]) + self.assertEqual( + self.current_rule_in_room(init_state_room_id), ACCESS_RULE_RESTRICTED, + ) + + # Changing access rule to unrestricted should fail. + self.change_rule_in_room( + preset_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=400, + ) + self.change_rule_in_room( + init_state_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=400, + ) + + # Changing access rule to direct should fail. + self.change_rule_in_room( + preset_room_id, ACCESS_RULE_DIRECT, expected_code=400, + ) + self.change_rule_in_room( + init_state_room_id, ACCESS_RULE_DIRECT, expected_code=400, + ) + + # Changing join rule to public in an unrestricted room should fail. + self.change_join_rule_in_room( + self.unrestricted_room, JoinRules.PUBLIC, expected_code=400, + ) + # Changing join rule to public in an direct room should fail. + self.change_join_rule_in_room( + self.direct_rooms[0], JoinRules.PUBLIC, expected_code=400, + ) + + # Creating a new room with the public_chat preset and an access rule that isn't + # restricted should fail. + self.create_room( + preset=RoomCreationPreset.PUBLIC_CHAT, rule=ACCESS_RULE_UNRESTRICTED, + expected_code=400, + ) + self.create_room( + preset=RoomCreationPreset.PUBLIC_CHAT, rule=ACCESS_RULE_DIRECT, + expected_code=400, + ) + + # Creating a room with the public join rule in its initial state and an access + # rule that isn't restricted should fail. + self.create_room( + initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }], rule=ACCESS_RULE_UNRESTRICTED, expected_code=400, + ) + self.create_room( + initial_state=[{ + "type": "m.room.join_rules", + "content": { + "join_rule": JoinRules.PUBLIC, + }, + }], rule=ACCESS_RULE_DIRECT, expected_code=400, + ) + def test_restricted(self): """Tests that in restricted mode we're unable to invite users from blacklisted servers but can invite other users. @@ -405,11 +483,17 @@ def test_change_rules(self): expected_code=403, ) - def create_room(self, direct=False, rule=None, expected_code=200): + def create_room( + self, direct=False, rule=None, preset=None, initial_state=None, + expected_code=200, + ): content = { "is_direct": direct, } + if preset: + content["preset"] = preset + if rule: content["initial_state"] = [{ "type": ACCESS_RULES_TYPE, @@ -419,6 +503,12 @@ def create_room(self, direct=False, rule=None, expected_code=200): } }] + if initial_state: + if "initial_state" not in content: + content["initial_state"] = [] + + content["initial_state"] += initial_state + request, channel = self.make_request( "POST", "/_matrix/client/r0/createRoom", @@ -457,6 +547,20 @@ def change_rule_in_room(self, room_id, new_rule, expected_code=200): self.assertEqual(channel.code, expected_code, channel.result) + def change_join_rule_in_room(self, room_id, new_join_rule, expected_code=200): + data = { + "join_rule": new_join_rule, + } + request, channel = self.make_request( + "PUT", + "/_matrix/client/r0/rooms/%s/state/%s" % (room_id, EventTypes.JoinRules), + 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", From d1a78ba2a3272fc6e2b7fb9512431b31a6d3ed58 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 16:48:36 +0200 Subject: [PATCH 02/26] Implement restriction on public room creation --- synapse/third_party_rules/access_rules.py | 41 +++++++++++++++++------ 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index e3f97bdf3aa8..2cb77e811552 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -17,7 +17,7 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.types import get_domain_from_id @@ -94,35 +94,52 @@ def on_create_room(self, requester, config, is_requester_admin): default rule to the initial state. """ is_direct = config.get("is_direct") - rule = None + preset = config.get("preset") + access_rule = None + join_rule = None # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - rule = event["content"].get("rule") + access_rule = event["content"].get("rule") # Make sure the event has a valid content. - if rule is None: + if access_rule is None: raise SynapseError(400, "Invalid access rule") # Make sure the rule name is valid. - if rule not in VALID_ACCESS_RULES: + if access_rule not in VALID_ACCESS_RULES: raise SynapseError(400, "Invalid access rule") # Make sure the rule is "direct" if the room is a direct chat. if ( - (is_direct and rule != ACCESS_RULE_DIRECT) - or (rule == ACCESS_RULE_DIRECT and not is_direct) + (is_direct and access_rule != ACCESS_RULE_DIRECT) + or (access_rule == ACCESS_RULE_DIRECT and not is_direct) ): raise SynapseError(400, "Invalid access rule") + if event["type"] == EventTypes.JoinRules: + join_rule = event["content"].get("join_rule") + + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # If there's no rules event in the initial state, create one with the default # setting. - if not rule: + if not access_rule: if is_direct: default_rule = ACCESS_RULE_DIRECT else: + # If the default value for non-direct chat changes, we should make another + # case here for rooms created with either a "public" join_rule or the + # "public_chat" preset to make sure those keep defaulting to "restricted" default_rule = ACCESS_RULE_RESTRICTED if not config.get("initial_state"): @@ -136,11 +153,11 @@ def on_create_room(self, requester, config, is_requester_admin): } }) - rule = default_rule + access_rule = default_rule # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( - config.get("power_level_content_override", {}), rule, + config.get("power_level_content_override", {}), access_rule, ) if not allowed: raise SynapseError(400, "Invalid power levels content override") @@ -148,7 +165,9 @@ def on_create_room(self, requester, config, is_requester_admin): # Second loop for events we need to know the current rule to process. for event in config.get("initial_state", []): if event["type"] == EventTypes.PowerLevels: - allowed = self._is_power_level_content_allowed(event["content"], rule) + allowed = self._is_power_level_content_allowed( + event["content"], access_rule + ) if not allowed: raise SynapseError(400, "Invalid power levels content") From ea5f86304e6be03ce32df16cf1996eb41da93b01 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 17:27:07 +0200 Subject: [PATCH 03/26] Implement restrictions on new events --- synapse/third_party_rules/access_rules.py | 41 ++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 2cb77e811552..a7fd9eab2e3a 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -232,6 +232,9 @@ def check_event_allowed(self, event, state_events): if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: return self._on_membership_or_invite(event, rule, state_events) + if event.type == EventTypes.JoinRules: + return self._on_join_rule_change(event, rule) + return True def _on_rules_change(self, event, state_events): @@ -251,6 +254,12 @@ def _on_rules_change(self, event, state_events): if new_rule not in VALID_ACCESS_RULES: return False + # We must not allow rooms with the "public" join rule to be given any other access + # rule than "restricted". + join_rule = self._get_join_rule_from_state(state_events) + if join_rule == JoinRules.PUBLIC and new_rule != ACCESS_RULE_RESTRICTED: + return False + # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( @@ -400,7 +409,6 @@ def _is_power_level_content_allowed(self, content, access_rule): access_rule (str): The access rule in place in this room. Returns: bool, True if the event can be allowed, False otherwise. - """ # Check if we need to apply the restrictions with the current rule. if access_rule not in RULES_WITH_RESTRICTED_POWER_LEVELS: @@ -424,6 +432,22 @@ def _is_power_level_content_allowed(self, content, access_rule): return True + def _on_join_rule_change(self, event, rule): + """Check whether a join rule change is allowed. A join rule change is always + allowed unless the new join rule is "public" and the current access rule isn't + "restricted". + + Args: + event (synapse.events.EventBase): The event to check. + rule (str): The name of the rule to apply. + Returns: + bool, True if the event can be allowed, False otherwise. + """ + if event.content.get('join_rule') == JoinRules.PUBLIC: + return rule == ACCESS_RULE_RESTRICTED + + return True + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. @@ -441,6 +465,21 @@ def _get_rule_from_state(state_events): rule = access_rules.content.get("rule") return rule + @staticmethod + def _get_join_rule_from_state(state_events): + """Extract the room's join rule from the given set of state events. + + Args: + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + str, the name of the join rule (either "public", or "invite") + """ + join_rule_event = state_events.get((EventTypes.JoinRules, "")) + if join_rule_event is None: + return "" + return join_rule_event.content.get("join_rule") + @staticmethod def _get_members_and_tokens_from_state(state_events): """Retrieves from a list of state events the list of users that have a From 0bb375c124e8120f6c3b63382aa86c50f3757df2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 17:29:17 +0200 Subject: [PATCH 04/26] Turns out the default preset is public_chat --- tests/rest/client/test_room_access_rules.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 83da1238bab6..96f7ac94d3f5 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -484,16 +484,14 @@ def test_change_rules(self): ) def create_room( - self, direct=False, rule=None, preset=None, initial_state=None, - expected_code=200, + self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, + initial_state=None, expected_code=200, ): content = { "is_direct": direct, + "preset": preset, } - if preset: - content["preset"] = preset - if rule: content["initial_state"] = [{ "type": ACCESS_RULES_TYPE, From dd92685179655ac555d40390dcab8970261d19da Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:03:36 +0200 Subject: [PATCH 05/26] Only check the join rule on room creation if an access rule is also provided --- synapse/third_party_rules/access_rules.py | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index a7fd9eab2e3a..786f3d9ad3c2 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,18 +122,18 @@ def on_create_room(self, requester, config, is_requester_admin): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - - # If there's no rules event in the initial state, create one with the default - # setting. - if not access_rule: + if access_rule: + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + else: + # If there's no rules event in the initial state, create one with the default + # setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: From ddf256c77f9e999181b2d77d6a05e5102855d11b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:03:59 +0200 Subject: [PATCH 06/26] Fix status code for forbidden events --- tests/rest/client/test_room_access_rules.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 96f7ac94d3f5..7e23add6b79b 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -181,27 +181,27 @@ def test_public_room(self): # Changing access rule to unrestricted should fail. self.change_rule_in_room( - preset_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=400, + preset_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=403, ) self.change_rule_in_room( - init_state_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=400, + init_state_room_id, ACCESS_RULE_UNRESTRICTED, expected_code=403, ) # Changing access rule to direct should fail. self.change_rule_in_room( - preset_room_id, ACCESS_RULE_DIRECT, expected_code=400, + preset_room_id, ACCESS_RULE_DIRECT, expected_code=403, ) self.change_rule_in_room( - init_state_room_id, ACCESS_RULE_DIRECT, expected_code=400, + init_state_room_id, ACCESS_RULE_DIRECT, expected_code=403, ) # Changing join rule to public in an unrestricted room should fail. self.change_join_rule_in_room( - self.unrestricted_room, JoinRules.PUBLIC, expected_code=400, + self.unrestricted_room, JoinRules.PUBLIC, expected_code=403, ) # Changing join rule to public in an direct room should fail. self.change_join_rule_in_room( - self.direct_rooms[0], JoinRules.PUBLIC, expected_code=400, + self.direct_rooms[0], JoinRules.PUBLIC, expected_code=403, ) # Creating a new room with the public_chat preset and an access rule that isn't From aea03c9d734d3dd5f0650b9d127bc9026266505c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:14:41 +0200 Subject: [PATCH 07/26] Doc --- synapse/third_party_rules/access_rules.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 786f3d9ad3c2..07b449ab3269 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -123,6 +123,11 @@ def on_create_room(self, requester, config, is_requester_admin): join_rule = event["content"].get("join_rule") if access_rule: + # If there's an access rules event in the initial state, check if the prefix + # or the join rule in use is compatible (i.e. if it involves a "public" join + # rule, the access rule must be "restricted"). We don't need to check that if + # there's no access rule provided, as in this case the access rule will + # default to "restricted", with which any join rule is allowed. if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: raise SynapseError(400, "Invalid access rule") @@ -132,8 +137,8 @@ def on_create_room(self, requester, config, is_requester_admin): ): raise SynapseError(400, "Invalid access rule") else: - # If there's no rules event in the initial state, create one with the default - # setting. + # If there's no access rules event in the initial state, create one with the + # default setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: @@ -437,6 +442,13 @@ def _on_join_rule_change(self, event, rule): allowed unless the new join rule is "public" and the current access rule isn't "restricted". + Note that we currently rely on the default access rule being "restricted": during + room creation, the m.room.join_rules event will be sent *before* the + im.vector.room.access_rules one, so the access rule that will be considered here + in this case will be the default "restricted" one. This is fine since the + "restricted" access rule allows any value for the join rule, but we should keep + that in mind if we need to change the default access rule in the future. + Args: event (synapse.events.EventBase): The event to check. rule (str): The name of the rule to apply. From 2526b79ce6923bcf681ecd846b718269833e5a7e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:15:44 +0200 Subject: [PATCH 08/26] Merge ifs --- synapse/third_party_rules/access_rules.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 07b449ab3269..c37737ea6057 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -128,12 +128,11 @@ def on_create_room(self, requester, config, is_requester_admin): # rule, the access rule must be "restricted"). We don't need to check that if # there's no access rule provided, as in this case the access rule will # default to "restricted", with which any join rule is allowed. - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED ): raise SynapseError(400, "Invalid access rule") else: From 00b47fdfc7827083405bee4372537bd9a0fadafb Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:17:07 +0200 Subject: [PATCH 09/26] Changelog --- changelog.d/5760.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5760.feature diff --git a/changelog.d/5760.feature b/changelog.d/5760.feature new file mode 100644 index 000000000000..98ec230a3b2e --- /dev/null +++ b/changelog.d/5760.feature @@ -0,0 +1 @@ +Force the access rule to be "restricted" if the join rule is "public" From ffa30b507fb164efa470118255f8ee4a29ce24f7 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:19:13 +0200 Subject: [PATCH 10/26] Fix changelog --- changelog.d/5760.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5760.feature b/changelog.d/5760.feature index 98ec230a3b2e..90302d793eb6 100644 --- a/changelog.d/5760.feature +++ b/changelog.d/5760.feature @@ -1 +1 @@ -Force the access rule to be "restricted" if the join rule is "public" +Force the access rule to be "restricted" if the join rule is "public". From 8a471557636a188ca7cc4dd92171597b40f5fb92 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 26 Jul 2019 18:45:31 +0200 Subject: [PATCH 11/26] Add ability to pass arguments to looping calls --- synapse/util/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 0ae7e2ef3b3e..5277f30d30c3 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -58,7 +58,7 @@ def time_msec(self): """Returns the current system time in miliseconds since epoch.""" return int(self.time() * 1000) - def looping_call(self, f, msec): + def looping_call(self, f, msec, *args): """Call a function repeatedly. Waits `msec` initially before calling `f` for the first time. @@ -67,7 +67,7 @@ def looping_call(self, f, msec): f(function): The function to call repeatedly. msec(float): How long to wait between calls in milliseconds. """ - call = task.LoopingCall(f) + call = task.LoopingCall(f, *args) call.clock = self._reactor d = call.start(msec / 1000.0, now=False) d.addErrback( From bec6d9e09015a5775147ca31416f9c27ed46a23b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 10:03:14 +0200 Subject: [PATCH 12/26] Add kwargs and doc --- synapse/util/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 5277f30d30c3..8f5a526800fa 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -58,7 +58,7 @@ def time_msec(self): """Returns the current system time in miliseconds since epoch.""" return int(self.time() * 1000) - def looping_call(self, f, msec, *args): + def looping_call(self, f, msec, *args, **kwargs): """Call a function repeatedly. Waits `msec` initially before calling `f` for the first time. @@ -66,8 +66,10 @@ def looping_call(self, f, msec, *args): Args: f(function): The function to call repeatedly. msec(float): How long to wait between calls in milliseconds. + *args: Postional arguments to pass to function. + **kwargs: Key arguments to pass to function. """ - call = task.LoopingCall(f, *args) + call = task.LoopingCall(f, *args, **kwargs) call.clock = self._reactor d = call.start(msec / 1000.0, now=False) d.addErrback( From 132887db8c3e7b6cede9cf9f5e69afbe75c5647a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 10:04:09 +0200 Subject: [PATCH 13/26] Changelog --- changelog.d/5780.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5780.misc diff --git a/changelog.d/5780.misc b/changelog.d/5780.misc new file mode 100644 index 000000000000..b7eb56e625b2 --- /dev/null +++ b/changelog.d/5780.misc @@ -0,0 +1 @@ +Allow looping calls to be given arguments. From 36c61df6595cf8a73b7ea0b8351c07378aac0b31 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 16:07:12 +0200 Subject: [PATCH 14/26] Check room ID and type of redacted event --- synapse/storage/events_worker.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index cc7df5cf14df..f18129f7a398 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -277,27 +277,33 @@ def get_events_as_list( # 2. have _get_event_from_row just call the first half of # that - orig_sender = yield self._simple_select_one_onecol( + orig_event_info = yield self._simple_select_one( table="events", keyvalues={"event_id": entry.event.redacts}, - retcol="sender", + retcols=["sender", "room_id", "type"], allow_none=True, ) + if not orig_event_info: + # We don't have the event that is being redacted, so we + # assume that the event isn't authorized for now. (If we + # later receive the event, then we will always redact + # it anyway, since we have this redaction) + continue + + if orig_event_info["room_id"] != entry.event.room_id: + continue + + if orig_event_info["type"] == EventTypes.Redaction: + continue + expected_domain = get_domain_from_id(entry.event.sender) if ( - orig_sender - and get_domain_from_id(orig_sender) == expected_domain + get_domain_from_id(orig_event_info["sender"]) == expected_domain ): # This redaction event is allowed. Mark as not needing a # recheck. entry.event.internal_metadata.recheck_redaction = False - else: - # We don't have the event that is being redacted, so we - # assume that the event isn't authorized for now. (If we - # later receive the event, then we will always redact - # it anyway, since we have this redaction) - continue if allow_rejected or not entry.event.rejected_reason: if check_redacted and entry.redacted_event: @@ -567,6 +573,12 @@ def _get_event_from_row( # Senders don't match, so the event isn't actually redacted redacted_event = None + if because.room_id != original_ev.room_id: + redacted_event = None + + if original_ev.type == EventTypes.Redaction: + redacted_event = None + cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event ) From bbd6208b3ea256da0d8920292f55cb5b554b8cfc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 17:22:42 +0200 Subject: [PATCH 15/26] Do checks sooner --- synapse/storage/events_worker.py | 40 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index f18129f7a398..cbb2529d5ed4 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -538,7 +538,7 @@ def _get_event_from_row( ) redacted_event = None - if redacted: + if redacted and original_ev.type != EventTypes.Redaction: redacted_event = prune_event(original_ev) redaction_id = yield self._simple_select_one_onecol( @@ -556,28 +556,26 @@ def _get_event_from_row( ) if because: - # It's fine to do add the event directly, since get_pdu_json - # will serialise this field correctly - redacted_event.unsigned["redacted_because"] = because - - # Starting in room version v3, some redactions need to be - # rechecked if we didn't have the redacted event at the - # time, so we recheck on read instead. - if because.internal_metadata.need_to_check_redaction(): - expected_domain = get_domain_from_id(original_ev.sender) - if get_domain_from_id(because.sender) == expected_domain: - # This redaction event is allowed. Mark as not needing a - # recheck. - because.internal_metadata.recheck_redaction = False - else: - # Senders don't match, so the event isn't actually redacted - redacted_event = None - if because.room_id != original_ev.room_id: redacted_event = None - - if original_ev.type == EventTypes.Redaction: - redacted_event = None + else: + # It's fine to do add the event directly, since get_pdu_json + # will serialise this field correctly + redacted_event.unsigned["redacted_because"] = because + + # Starting in room version v3, some redactions need to be + # rechecked if we didn't have the redacted event at the + # time, so we recheck on read instead. + if because.internal_metadata.need_to_check_redaction(): + expected_domain = get_domain_from_id(original_ev.sender) + if get_domain_from_id(because.sender) == expected_domain: + # This redaction event is allowed. Mark as not needing a + # recheck. + because.internal_metadata.recheck_redaction = False + else: + # Senders don't match, so the event isn't actually + # redacted + redacted_event = None cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event From 8ced9a2f58783ef9dd93e9313c24114f7924e771 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 15:55:18 +0200 Subject: [PATCH 16/26] Don't make the checks depend on recheck_redaction --- synapse/storage/events_worker.py | 43 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index cbb2529d5ed4..b5604cbdef59 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -255,6 +255,29 @@ def get_events_as_list( # didn't have the redacted event at the time, so we recheck on read # instead. if not allow_rejected and entry.event.type == EventTypes.Redaction: + orig_event_info = yield self._simple_select_one( + table="events", + keyvalues={"event_id": entry.event.redacts}, + retcols=["sender", "room_id", "type"], + allow_none=True, + ) + + if not orig_event_info: + # We don't have the event that is being redacted, so we + # assume that the event isn't authorized for now. (If we + # later receive the event, then we will always redact + # it anyway, since we have this redaction) + continue + + if orig_event_info["room_id"] != entry.event.room_id: + # Don't process redactions if the redacted event doesn't belong to the + # redaction's room. + continue + + if orig_event_info["type"] == EventTypes.Redaction: + # Don't process redactions of redactions. + continue + if entry.event.internal_metadata.need_to_check_redaction(): # XXX: we need to avoid calling get_event here. # @@ -277,26 +300,6 @@ def get_events_as_list( # 2. have _get_event_from_row just call the first half of # that - orig_event_info = yield self._simple_select_one( - table="events", - keyvalues={"event_id": entry.event.redacts}, - retcols=["sender", "room_id", "type"], - allow_none=True, - ) - - if not orig_event_info: - # We don't have the event that is being redacted, so we - # assume that the event isn't authorized for now. (If we - # later receive the event, then we will always redact - # it anyway, since we have this redaction) - continue - - if orig_event_info["room_id"] != entry.event.room_id: - continue - - if orig_event_info["type"] == EventTypes.Redaction: - continue - expected_domain = get_domain_from_id(entry.event.sender) if ( get_domain_from_id(orig_event_info["sender"]) == expected_domain From 0fda4e2e50b7ee33458cba58085147bc459453fe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 15:56:02 +0200 Subject: [PATCH 17/26] Should now work, unless we can't find the redaction event which happens for some reason (need to investigate) --- synapse/storage/events_worker.py | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index b5604cbdef59..2983aec55bf9 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -559,26 +559,26 @@ def _get_event_from_row( ) if because: + # It's fine to do add the event directly, since get_pdu_json + # will serialise this field correctly + redacted_event.unsigned["redacted_because"] = because + + # Starting in room version v3, some redactions need to be + # rechecked if we didn't have the redacted event at the + # time, so we recheck on read instead. + if because.internal_metadata.need_to_check_redaction(): + expected_domain = get_domain_from_id(original_ev.sender) + if get_domain_from_id(because.sender) == expected_domain: + # This redaction event is allowed. Mark as not needing a + # recheck. + because.internal_metadata.recheck_redaction = False + else: + # Senders don't match, so the event isn't actually + # redacted + redacted_event = None + if because.room_id != original_ev.room_id: redacted_event = None - else: - # It's fine to do add the event directly, since get_pdu_json - # will serialise this field correctly - redacted_event.unsigned["redacted_because"] = because - - # Starting in room version v3, some redactions need to be - # rechecked if we didn't have the redacted event at the - # time, so we recheck on read instead. - if because.internal_metadata.need_to_check_redaction(): - expected_domain = get_domain_from_id(original_ev.sender) - if get_domain_from_id(because.sender) == expected_domain: - # This redaction event is allowed. Mark as not needing a - # recheck. - because.internal_metadata.recheck_redaction = False - else: - # Senders don't match, so the event isn't actually - # redacted - redacted_event = None cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event From d2bb51080e344b179d5d5e3789e00480b8f5286a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 16:15:01 +0200 Subject: [PATCH 18/26] Incorporate review --- synapse/third_party_rules/access_rules.py | 28 +++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index c37737ea6057..56527d636524 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,20 +122,7 @@ def on_create_room(self, requester, config, is_requester_admin): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if access_rule: - # If there's an access rules event in the initial state, check if the prefix - # or the join rule in use is compatible (i.e. if it involves a "public" join - # rule, the access rule must be "restricted"). We don't need to check that if - # there's no access rule provided, as in this case the access rule will - # default to "restricted", with which any join rule is allowed. - if ( - ( - join_rule == JoinRules.PUBLIC - or preset == RoomCreationPreset.PUBLIC_CHAT - ) and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - else: + if access_rule is None: # If there's no access rules event in the initial state, create one with the # default setting. if is_direct: @@ -159,6 +146,17 @@ def on_create_room(self, requester, config, is_requester_admin): access_rule = default_rule + # Check that the preset or the join rule in use is compatible with the access + # rule, whether it's a user-defined one or the default one (i.e. if it involves + # a "public" join rule, the access rule must be "restricted"). + if ( + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( config.get("power_level_content_override", {}), access_rule, @@ -488,7 +486,7 @@ def _get_join_rule_from_state(state_events): """ join_rule_event = state_events.get((EventTypes.JoinRules, "")) if join_rule_event is None: - return "" + return None return join_rule_event.content.get("join_rule") @staticmethod From c4e56a8ee96ad721a8be6e2448660bf0d93f91f0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 15:11:27 +0200 Subject: [PATCH 19/26] Ignore invalid redactions in _get_event_from_row --- synapse/storage/events_worker.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 2983aec55bf9..b771cf98ce3f 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -579,6 +579,11 @@ def _get_event_from_row( if because.room_id != original_ev.room_id: redacted_event = None + else: + # The lack of a redaction likely means that the redaction is invalid + # and therefore not returned by get_event, so it should be safe to + # just ignore it here. + redacted_event = None cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event From 35ec13baab12c1800d8e6d5503643bdc76ef165f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 15:48:57 +0200 Subject: [PATCH 20/26] Ignore redactions of redactions in get_events_as_list --- synapse/storage/events_worker.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index b771cf98ce3f..5dc49822b59d 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -272,10 +272,7 @@ def get_events_as_list( if orig_event_info["room_id"] != entry.event.room_id: # Don't process redactions if the redacted event doesn't belong to the # redaction's room. - continue - - if orig_event_info["type"] == EventTypes.Redaction: - # Don't process redactions of redactions. + logger.info("Ignoring redation in another room.") continue if entry.event.internal_metadata.need_to_check_redaction(): From 0c6500a08bbaac34b7630d66339c03dc076b2dbe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 10:19:04 +0200 Subject: [PATCH 21/26] Explain rationale --- synapse/third_party_rules/access_rules.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 56527d636524..1a295ea7cea4 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -438,6 +438,10 @@ def _on_join_rule_change(self, event, rule): """Check whether a join rule change is allowed. A join rule change is always allowed unless the new join rule is "public" and the current access rule isn't "restricted". + The rationale is that external users (those whose server would be denied access + to rooms enforcing the "restricted" access rule) should always rely on non- + external users for access to rooms, therefore they shouldn't be able to access + rooms that don't require an invite to be joined. Note that we currently rely on the default access rule being "restricted": during room creation, the m.room.join_rules event will be sent *before* the From 235271be4f716919765a7b1545fd680a00b9bda8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 18:12:04 +0200 Subject: [PATCH 22/26] Allow defining HTML templates to serve the user on account renewal --- synapse/config/registration.py | 50 ++++++++++++++++++- synapse/handlers/account_validity.py | 10 +++- synapse/res/templates/account_renewed.html | 1 + synapse/res/templates/invalid_token.html | 1 + .../rest/client/v2_alpha/account_validity.py | 21 +++++--- 5 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 synapse/res/templates/account_renewed.html create mode 100644 synapse/res/templates/invalid_token.html diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 5041cfa4ee7b..316b3719aaa3 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -13,6 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os +import pkg_resources + from distutils.util import strtobool from synapse.config._base import Config, ConfigError @@ -41,8 +44,41 @@ def __init__(self, config, synapse_config): self.startup_job_max_delta = self.period * 10. / 100. - if self.renew_by_email_enabled and "public_baseurl" not in synapse_config: - raise ConfigError("Can't send renewal emails without 'public_baseurl'") + if self.renew_by_email_enabled: + if "public_baseurl" not in synapse_config: + raise ConfigError("Can't send renewal emails without 'public_baseurl'") + + template_dir = config.get("template_dir") + + if not template_dir: + template_dir = pkg_resources.resource_filename("synapse", "res/templates") + + if "account_renewed_html_path" in config: + file_path = os.path.join( + template_dir, config["account_renewed_html_path"], + ) + + self.account_renewed_html_content = self.read_file( + file_path, + "account_validity.account_renewed_html_path", + ) + else: + self.account_renewed_html_content = ( + "Your account has been successfully renewed." + ) + + if "invalid_token_html_path" in config: + file_path = os.path.join( + template_dir, config["invalid_token_html_path"], + ) + + self.invalid_token_html_content = self.read_file( + file_path, "account_validity.invalid_token_html_path", + ) + else: + self.invalid_token_html_content = ( + "Invalid renewal token." + ) class RegistrationConfig(Config): @@ -161,6 +197,16 @@ def default_config(self, generate_secrets=False, **kwargs): # period: 6w # renew_at: 1w # renew_email_subject: "Renew your %%(app)s account" + # # Directory in which Synapse will try to find the HTML files to serve to the + # # user when trying to renew an account. Optional, defaults to + # # synapse/res/templates. + # template_dir: "res/templates" + # # HTML to be displayed to the user after they successfully renewed their + # # account. Optional. + # account_renewed_html_path: "account_renewed.html" + # # HTML to be displayed when the user tries to renew an account with an invalid + # # renewal token. Optional. + # invalid_token_html_path: "invalid_token.html" # The user must provide all of the below types of 3PID when registering. # diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 5e0b92eb1cc2..396f0059f711 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -223,11 +223,19 @@ def renew_account(self, renewal_token): Args: renewal_token (str): Token sent with the renewal request. + Returns: + bool: Whether the provided token is valid. """ - user_id = yield self.store.get_user_from_renewal_token(renewal_token) + try: + user_id = yield self.store.get_user_from_renewal_token(renewal_token) + except StoreError: + defer.returnValue(False) + logger.debug("Renewing an account for user %s", user_id) yield self.renew_account_for_user(user_id) + defer.returnValue(True) + @defer.inlineCallbacks def renew_account_for_user(self, user_id, expiration_ts=None, email_sent=False): """Renews the account attached to a given user by pushing back the diff --git a/synapse/res/templates/account_renewed.html b/synapse/res/templates/account_renewed.html new file mode 100644 index 000000000000..894da030afb7 --- /dev/null +++ b/synapse/res/templates/account_renewed.html @@ -0,0 +1 @@ +Your account has been successfully renewed. diff --git a/synapse/res/templates/invalid_token.html b/synapse/res/templates/invalid_token.html new file mode 100644 index 000000000000..6bd2b9836437 --- /dev/null +++ b/synapse/res/templates/invalid_token.html @@ -0,0 +1 @@ +Invalid renewal token. diff --git a/synapse/rest/client/v2_alpha/account_validity.py b/synapse/rest/client/v2_alpha/account_validity.py index 63bdc3356434..9384cd0d53b4 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -40,6 +40,8 @@ def __init__(self, hs): self.hs = hs self.account_activity_handler = hs.get_account_validity_handler() self.auth = hs.get_auth() + self.success_html = hs.config.account_validity.account_renewed_html_content + self.failure_html = hs.config.account_validity.invalid_token_html_content @defer.inlineCallbacks def on_GET(self, request): @@ -47,14 +49,21 @@ def on_GET(self, request): raise SynapseError(400, "Missing renewal token") renewal_token = request.args[b"token"][0] - yield self.account_activity_handler.renew_account(renewal_token.decode('utf8')) + token_valid = yield self.account_activity_handler.renew_account( + renewal_token.decode("utf8"), + ) - request.setResponseCode(200) + if token_valid: + status_code = 200 + response = self.success_html + else: + status_code = 404 + response = self.failure_html + + request.setResponseCode(status_code) request.setHeader(b"Content-Type", b"text/html; charset=utf-8") - request.setHeader(b"Content-Length", b"%d" % ( - len(AccountValidityRenewServlet.SUCCESS_HTML), - )) - request.write(AccountValidityRenewServlet.SUCCESS_HTML) + request.setHeader(b"Content-Length", b"%d" % (len(response),)) + request.write(response.encode("utf8")) finish_request(request) defer.returnValue(None) From 9502bd8d7888596df102a55ada623f36b4552f69 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 12:00:08 +0200 Subject: [PATCH 23/26] Add tests --- tests/rest/client/v2_alpha/test_register.py | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index f56a34a41d33..af1e60059121 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -375,6 +375,8 @@ def make_homeserver(self, reactor, clock): "renew_at": 172800000, # Time in ms for 2 days "renew_by_email_enabled": True, "renew_email_subject": "Renew your account", + "account_renewed_html_path": "account_renewed.html", + "invalid_token_html_path": "invalid_token.html", } # Email config. @@ -425,6 +427,19 @@ def test_renewal_email(self): self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) + # Check that we're getting HTML back. + content_type = None + for header in channel.result.get("headers", []): + if header[0] == b"Content-Type": + content_type = header[1] + self.assertEqual(content_type, b"text/html; charset=utf-8", channel.result) + + # Check that the HTML we're getting is the one we expect on a successful renewal. + expected_html = self.hs.config.account_validity.account_renewed_html_content + self.assertEqual( + channel.result["body"], expected_html.encode("utf8"), channel.result + ) + # Move 3 days forward. If the renewal failed, every authed request with # our access token should be denied from now, otherwise they should # succeed. @@ -433,6 +448,28 @@ def test_renewal_email(self): self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) + def test_renewal_invalid_token(self): + # Hit the renewal endpoint with an invalid token and check that it behaves as + # expected, i.e. that it responds with 404 Not Found and the correct HTML. + url = "/_matrix/client/unstable/account_validity/renew?token=123" + request, channel = self.make_request(b"GET", url) + self.render(request) + self.assertEquals(channel.result["code"], b"404", channel.result) + + # Check that we're getting HTML back. + content_type = None + for header in channel.result.get("headers", []): + if header[0] == b"Content-Type": + content_type = header[1] + self.assertEqual(content_type, b"text/html; charset=utf-8", channel.result) + + # Check that the HTML we're getting is the one we expect when using an + # invalid/unknown token. + expected_html = self.hs.config.account_validity.invalid_token_html_content + self.assertEqual( + channel.result["body"], expected_html.encode("utf8"), channel.result + ) + def test_manual_email_send(self): self.email_attempts = [] From a9567ee1a61fa1194d2d93309c16a07937f061a4 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 12:08:06 +0200 Subject: [PATCH 24/26] Changelog --- changelog.d/5807.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5807.feature diff --git a/changelog.d/5807.feature b/changelog.d/5807.feature new file mode 100644 index 000000000000..8b7d29a23cb4 --- /dev/null +++ b/changelog.d/5807.feature @@ -0,0 +1 @@ +Allow defining HTML templates to serve the user on account renewal attempt when using the account validity feature. From cd47482257fd31b51d279c5b7e935a8eddcb2457 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 12:08:25 +0200 Subject: [PATCH 25/26] Sample config --- docs/sample_config.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index daf0687dbbc9..cb58b82d2286 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -797,6 +797,16 @@ uploads_path: "DATADIR/uploads" # period: 6w # renew_at: 1w # renew_email_subject: "Renew your %(app)s account" +# # Directory in which Synapse will try to find the HTML files to serve to the +# # user when trying to renew an account. Optional, defaults to +# # synapse/res/templates. +# template_dir: "res/templates" +# # HTML to be displayed to the user after they successfully renewed their +# # account. Optional. +# account_renewed_html_path: "account_renewed.html" +# # HTML to be displayed when the user tries to renew an account with an invalid +# # renewal token. Optional. +# invalid_token_html_path: "invalid_token.html" # The user must provide all of the below types of 3PID when registering. # From 359aed416872a86aa09dbe1bdff6434d707d75e8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 12:11:27 +0200 Subject: [PATCH 26/26] Lint --- docs/sample_config.yaml | 2 +- synapse/config/registration.py | 19 +++++++------------ .../rest/client/v2_alpha/account_validity.py | 2 +- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index cb58b82d2286..8413861623a7 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -800,7 +800,7 @@ uploads_path: "DATADIR/uploads" # # Directory in which Synapse will try to find the HTML files to serve to the # # user when trying to renew an account. Optional, defaults to # # synapse/res/templates. -# template_dir: "res/templates" +# template_dir: "res/templates" # # HTML to be displayed to the user after they successfully renewed their # # account. Optional. # account_renewed_html_path: "account_renewed.html" diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 316b3719aaa3..14752298e9e1 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -14,10 +14,10 @@ # limitations under the License. import os -import pkg_resources - from distutils.util import strtobool +import pkg_resources + from synapse.config._base import Config, ConfigError from synapse.types import RoomAlias from synapse.util.stringutils import random_string_with_symbols @@ -54,13 +54,10 @@ def __init__(self, config, synapse_config): template_dir = pkg_resources.resource_filename("synapse", "res/templates") if "account_renewed_html_path" in config: - file_path = os.path.join( - template_dir, config["account_renewed_html_path"], - ) + file_path = os.path.join(template_dir, config["account_renewed_html_path"]) self.account_renewed_html_content = self.read_file( - file_path, - "account_validity.account_renewed_html_path", + file_path, "account_validity.account_renewed_html_path" ) else: self.account_renewed_html_content = ( @@ -68,12 +65,10 @@ def __init__(self, config, synapse_config): ) if "invalid_token_html_path" in config: - file_path = os.path.join( - template_dir, config["invalid_token_html_path"], - ) + file_path = os.path.join(template_dir, config["invalid_token_html_path"]) self.invalid_token_html_content = self.read_file( - file_path, "account_validity.invalid_token_html_path", + file_path, "account_validity.invalid_token_html_path" ) else: self.invalid_token_html_content = ( @@ -200,7 +195,7 @@ def default_config(self, generate_secrets=False, **kwargs): # # Directory in which Synapse will try to find the HTML files to serve to the # # user when trying to renew an account. Optional, defaults to # # synapse/res/templates. - # template_dir: "res/templates" + # template_dir: "res/templates" # # HTML to be displayed to the user after they successfully renewed their # # account. Optional. # account_renewed_html_path: "account_renewed.html" diff --git a/synapse/rest/client/v2_alpha/account_validity.py b/synapse/rest/client/v2_alpha/account_validity.py index 9384cd0d53b4..8091b7828591 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -50,7 +50,7 @@ def on_GET(self, request): renewal_token = request.args[b"token"][0] token_valid = yield self.account_activity_handler.renew_account( - renewal_token.decode("utf8"), + renewal_token.decode("utf8") ) if token_valid: