From d1a78ba2a3272fc6e2b7fb9512431b31a6d3ed58 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 16:48:36 +0200 Subject: Implement restriction on public room creation --- synapse/third_party_rules/access_rules.py | 41 ++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index e3f97bdf3a..2cb77e8115 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -17,7 +17,7 @@ import email.utils from twisted.internet import defer -from synapse.api.constants import EventTypes +from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.types import get_domain_from_id @@ -94,35 +94,52 @@ class RoomAccessRules(object): default rule to the initial state. """ is_direct = config.get("is_direct") - rule = None + preset = config.get("preset") + access_rule = None + join_rule = None # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - rule = event["content"].get("rule") + access_rule = event["content"].get("rule") # Make sure the event has a valid content. - if rule is None: + if access_rule is None: raise SynapseError(400, "Invalid access rule") # Make sure the rule name is valid. - if rule not in VALID_ACCESS_RULES: + if access_rule not in VALID_ACCESS_RULES: raise SynapseError(400, "Invalid access rule") # Make sure the rule is "direct" if the room is a direct chat. if ( - (is_direct and rule != ACCESS_RULE_DIRECT) - or (rule == ACCESS_RULE_DIRECT and not is_direct) + (is_direct and access_rule != ACCESS_RULE_DIRECT) + or (access_rule == ACCESS_RULE_DIRECT and not is_direct) ): raise SynapseError(400, "Invalid access rule") + if event["type"] == EventTypes.JoinRules: + join_rule = event["content"].get("join_rule") + + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # If there's no rules event in the initial state, create one with the default # setting. - if not rule: + if not access_rule: if is_direct: default_rule = ACCESS_RULE_DIRECT else: + # If the default value for non-direct chat changes, we should make another + # case here for rooms created with either a "public" join_rule or the + # "public_chat" preset to make sure those keep defaulting to "restricted" default_rule = ACCESS_RULE_RESTRICTED if not config.get("initial_state"): @@ -136,11 +153,11 @@ class RoomAccessRules(object): } }) - rule = default_rule + access_rule = default_rule # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( - config.get("power_level_content_override", {}), rule, + config.get("power_level_content_override", {}), access_rule, ) if not allowed: raise SynapseError(400, "Invalid power levels content override") @@ -148,7 +165,9 @@ class RoomAccessRules(object): # Second loop for events we need to know the current rule to process. for event in config.get("initial_state", []): if event["type"] == EventTypes.PowerLevels: - allowed = self._is_power_level_content_allowed(event["content"], rule) + allowed = self._is_power_level_content_allowed( + event["content"], access_rule + ) if not allowed: raise SynapseError(400, "Invalid power levels content") -- cgit 1.5.1 From ea5f86304e6be03ce32df16cf1996eb41da93b01 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 24 Jul 2019 17:27:07 +0200 Subject: Implement restrictions on new events --- synapse/third_party_rules/access_rules.py | 41 ++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 2cb77e8115..a7fd9eab2e 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -232,6 +232,9 @@ class RoomAccessRules(object): if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: return self._on_membership_or_invite(event, rule, state_events) + if event.type == EventTypes.JoinRules: + return self._on_join_rule_change(event, rule) + return True def _on_rules_change(self, event, state_events): @@ -251,6 +254,12 @@ class RoomAccessRules(object): if new_rule not in VALID_ACCESS_RULES: return False + # We must not allow rooms with the "public" join rule to be given any other access + # rule than "restricted". + join_rule = self._get_join_rule_from_state(state_events) + if join_rule == JoinRules.PUBLIC and new_rule != ACCESS_RULE_RESTRICTED: + return False + # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: existing_members, threepid_tokens = self._get_members_and_tokens_from_state( @@ -400,7 +409,6 @@ class RoomAccessRules(object): access_rule (str): The access rule in place in this room. Returns: bool, True if the event can be allowed, False otherwise. - """ # Check if we need to apply the restrictions with the current rule. if access_rule not in RULES_WITH_RESTRICTED_POWER_LEVELS: @@ -424,6 +432,22 @@ class RoomAccessRules(object): return True + def _on_join_rule_change(self, event, rule): + """Check whether a join rule change is allowed. A join rule change is always + allowed unless the new join rule is "public" and the current access rule isn't + "restricted". + + Args: + event (synapse.events.EventBase): The event to check. + rule (str): The name of the rule to apply. + Returns: + bool, True if the event can be allowed, False otherwise. + """ + if event.content.get('join_rule') == JoinRules.PUBLIC: + return rule == ACCESS_RULE_RESTRICTED + + return True + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. @@ -441,6 +465,21 @@ class RoomAccessRules(object): rule = access_rules.content.get("rule") return rule + @staticmethod + def _get_join_rule_from_state(state_events): + """Extract the room's join rule from the given set of state events. + + Args: + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + str, the name of the join rule (either "public", or "invite") + """ + join_rule_event = state_events.get((EventTypes.JoinRules, "")) + if join_rule_event is None: + return "" + return join_rule_event.content.get("join_rule") + @staticmethod def _get_members_and_tokens_from_state(state_events): """Retrieves from a list of state events the list of users that have a -- cgit 1.5.1 From dd92685179655ac555d40390dcab8970261d19da Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:03:36 +0200 Subject: Only check the join rule on room creation if an access rule is also provided --- synapse/third_party_rules/access_rules.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index a7fd9eab2e..786f3d9ad3 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,18 +122,18 @@ class RoomAccessRules(object): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - - # If there's no rules event in the initial state, create one with the default - # setting. - if not access_rule: + if access_rule: + if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: + raise SynapseError(400, "Invalid access rule") + + if ( + preset == RoomCreationPreset.PUBLIC_CHAT + and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + else: + # If there's no rules event in the initial state, create one with the default + # setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: -- cgit 1.5.1 From aea03c9d734d3dd5f0650b9d127bc9026266505c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:14:41 +0200 Subject: Doc --- synapse/third_party_rules/access_rules.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 786f3d9ad3..07b449ab32 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -123,6 +123,11 @@ class RoomAccessRules(object): join_rule = event["content"].get("join_rule") if access_rule: + # If there's an access rules event in the initial state, check if the prefix + # or the join rule in use is compatible (i.e. if it involves a "public" join + # rule, the access rule must be "restricted"). We don't need to check that if + # there's no access rule provided, as in this case the access rule will + # default to "restricted", with which any join rule is allowed. if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: raise SynapseError(400, "Invalid access rule") @@ -132,8 +137,8 @@ class RoomAccessRules(object): ): raise SynapseError(400, "Invalid access rule") else: - # If there's no rules event in the initial state, create one with the default - # setting. + # If there's no access rules event in the initial state, create one with the + # default setting. if is_direct: default_rule = ACCESS_RULE_DIRECT else: @@ -437,6 +442,13 @@ class RoomAccessRules(object): allowed unless the new join rule is "public" and the current access rule isn't "restricted". + Note that we currently rely on the default access rule being "restricted": during + room creation, the m.room.join_rules event will be sent *before* the + im.vector.room.access_rules one, so the access rule that will be considered here + in this case will be the default "restricted" one. This is fine since the + "restricted" access rule allows any value for the join rule, but we should keep + that in mind if we need to change the default access rule in the future. + Args: event (synapse.events.EventBase): The event to check. rule (str): The name of the rule to apply. -- cgit 1.5.1 From 2526b79ce6923bcf681ecd846b718269833e5a7e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 25 Jul 2019 10:15:44 +0200 Subject: Merge ifs --- synapse/third_party_rules/access_rules.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 07b449ab32..c37737ea60 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -128,12 +128,11 @@ class RoomAccessRules(object): # rule, the access rule must be "restricted"). We don't need to check that if # there's no access rule provided, as in this case the access rule will # default to "restricted", with which any join rule is allowed. - if join_rule == JoinRules.PUBLIC and access_rule != ACCESS_RULE_RESTRICTED: - raise SynapseError(400, "Invalid access rule") - if ( - preset == RoomCreationPreset.PUBLIC_CHAT - and access_rule != ACCESS_RULE_RESTRICTED + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED ): raise SynapseError(400, "Invalid access rule") else: -- cgit 1.5.1 From 8a471557636a188ca7cc4dd92171597b40f5fb92 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 26 Jul 2019 18:45:31 +0200 Subject: Add ability to pass arguments to looping calls --- synapse/util/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse') diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 0ae7e2ef3b..5277f30d30 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -58,7 +58,7 @@ class Clock(object): """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 @@ class Clock(object): 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( -- cgit 1.5.1 From bec6d9e09015a5775147ca31416f9c27ed46a23b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 10:03:14 +0200 Subject: Add kwargs and doc --- synapse/util/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'synapse') diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index 5277f30d30..8f5a526800 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -58,7 +58,7 @@ class Clock(object): """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 @@ class Clock(object): 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( -- cgit 1.5.1 From 36c61df6595cf8a73b7ea0b8351c07378aac0b31 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 16:07:12 +0200 Subject: Check room ID and type of redacted event --- synapse/storage/events_worker.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index cc7df5cf14..f18129f7a3 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -277,27 +277,33 @@ class EventsWorkerStore(SQLBaseStore): # 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 @@ class EventsWorkerStore(SQLBaseStore): # 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 ) -- cgit 1.5.1 From bbd6208b3ea256da0d8920292f55cb5b554b8cfc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 29 Jul 2019 17:22:42 +0200 Subject: Do checks sooner --- synapse/storage/events_worker.py | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index f18129f7a3..cbb2529d5e 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -538,7 +538,7 @@ class EventsWorkerStore(SQLBaseStore): ) 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 @@ class EventsWorkerStore(SQLBaseStore): ) 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 -- cgit 1.5.1 From 8ced9a2f58783ef9dd93e9313c24114f7924e771 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 15:55:18 +0200 Subject: Don't make the checks depend on recheck_redaction --- synapse/storage/events_worker.py | 43 +++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index cbb2529d5e..b5604cbdef 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -255,6 +255,29 @@ class EventsWorkerStore(SQLBaseStore): # 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 @@ class EventsWorkerStore(SQLBaseStore): # 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 -- cgit 1.5.1 From 0fda4e2e50b7ee33458cba58085147bc459453fe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 15:56:02 +0200 Subject: 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(-) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index b5604cbdef..2983aec55b 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -559,26 +559,26 @@ class EventsWorkerStore(SQLBaseStore): ) 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 -- cgit 1.5.1 From d2bb51080e344b179d5d5e3789e00480b8f5286a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 30 Jul 2019 16:15:01 +0200 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index c37737ea60..56527d6365 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -122,20 +122,7 @@ class RoomAccessRules(object): if event["type"] == EventTypes.JoinRules: join_rule = event["content"].get("join_rule") - if access_rule: - # If there's an access rules event in the initial state, check if the prefix - # or the join rule in use is compatible (i.e. if it involves a "public" join - # rule, the access rule must be "restricted"). We don't need to check that if - # there's no access rule provided, as in this case the access rule will - # default to "restricted", with which any join rule is allowed. - if ( - ( - join_rule == JoinRules.PUBLIC - or preset == RoomCreationPreset.PUBLIC_CHAT - ) and access_rule != ACCESS_RULE_RESTRICTED - ): - raise SynapseError(400, "Invalid access rule") - else: + if access_rule is None: # If there's no access rules event in the initial state, create one with the # default setting. if is_direct: @@ -159,6 +146,17 @@ class RoomAccessRules(object): access_rule = default_rule + # Check that the preset or the join rule in use is compatible with the access + # rule, whether it's a user-defined one or the default one (i.e. if it involves + # a "public" join rule, the access rule must be "restricted"). + if ( + ( + join_rule == JoinRules.PUBLIC + or preset == RoomCreationPreset.PUBLIC_CHAT + ) and access_rule != ACCESS_RULE_RESTRICTED + ): + raise SynapseError(400, "Invalid access rule") + # Check if the creator can override values for the power levels. allowed = self._is_power_level_content_allowed( config.get("power_level_content_override", {}), access_rule, @@ -488,7 +486,7 @@ class RoomAccessRules(object): """ join_rule_event = state_events.get((EventTypes.JoinRules, "")) if join_rule_event is None: - return "" + return None return join_rule_event.content.get("join_rule") @staticmethod -- cgit 1.5.1 From c4e56a8ee96ad721a8be6e2448660bf0d93f91f0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 15:11:27 +0200 Subject: Ignore invalid redactions in _get_event_from_row --- synapse/storage/events_worker.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 2983aec55b..b771cf98ce 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -579,6 +579,11 @@ class EventsWorkerStore(SQLBaseStore): 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 -- cgit 1.5.1 From 35ec13baab12c1800d8e6d5503643bdc76ef165f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 15:48:57 +0200 Subject: Ignore redactions of redactions in get_events_as_list --- synapse/storage/events_worker.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'synapse') diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index b771cf98ce..5dc49822b5 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -272,10 +272,7 @@ class EventsWorkerStore(SQLBaseStore): 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(): -- cgit 1.5.1 From 0c6500a08bbaac34b7630d66339c03dc076b2dbe Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 10:19:04 +0200 Subject: Explain rationale --- synapse/third_party_rules/access_rules.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 56527d6365..1a295ea7ce 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -438,6 +438,10 @@ class RoomAccessRules(object): """Check whether a join rule change is allowed. A join rule change is always allowed unless the new join rule is "public" and the current access rule isn't "restricted". + The rationale is that external users (those whose server would be denied access + to rooms enforcing the "restricted" access rule) should always rely on non- + external users for access to rooms, therefore they shouldn't be able to access + rooms that don't require an invite to be joined. Note that we currently rely on the default access rule being "restricted": during room creation, the m.room.join_rules event will be sent *before* the -- cgit 1.5.1 From 235271be4f716919765a7b1545fd680a00b9bda8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 31 Jul 2019 18:12:04 +0200 Subject: 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 + synapse/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 (limited to 'synapse') diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 5041cfa4ee..316b3719aa 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 @@ class AccountValidityConfig(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 @@ class RegistrationConfig(Config): # 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 5e0b92eb1c..396f0059f7 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -223,11 +223,19 @@ class AccountValidityHandler(object): 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 0000000000..894da030af --- /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 0000000000..6bd2b98364 --- /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 63bdc33564..9384cd0d53 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -40,6 +40,8 @@ class AccountValidityRenewServlet(RestServlet): 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 @@ class AccountValidityRenewServlet(RestServlet): 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) -- cgit 1.5.1 From 359aed416872a86aa09dbe1bdff6434d707d75e8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 1 Aug 2019 12:11:27 +0200 Subject: Lint --- docs/sample_config.yaml | 2 +- synapse/config/registration.py | 19 +++++++------------ synapse/rest/client/v2_alpha/account_validity.py | 2 +- 3 files changed, 9 insertions(+), 14 deletions(-) (limited to 'synapse') diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index cb58b82d22..8413861623 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 316b3719aa..14752298e9 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 @@ class AccountValidityConfig(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 @@ class AccountValidityConfig(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 @@ class RegistrationConfig(Config): # # 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 9384cd0d53..8091b78285 100644 --- a/synapse/rest/client/v2_alpha/account_validity.py +++ b/synapse/rest/client/v2_alpha/account_validity.py @@ -50,7 +50,7 @@ class AccountValidityRenewServlet(RestServlet): 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: -- cgit 1.5.1