From eddc6d8855b0aa50fe85f85d40bd3ffc24678238 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 5 Sep 2019 16:25:22 +0100 Subject: Forbid changing the name, avatar or topic of a direct room --- synapse/third_party_rules/access_rules.py | 50 +++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 1a295ea7ce..41862f6d0b 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -237,6 +237,15 @@ class RoomAccessRules(object): if event.type == EventTypes.JoinRules: return self._on_join_rule_change(event, rule) + if event.type == EventTypes.RoomAvatar: + return self._on_room_avatar_change(event, rule) + + if event.type == EventTypes.Name: + return self._on_room_name_change(event, rule) + + if event.type == EventTypes.Topic: + return self._on_room_topic_change(event, rule) + return True def _on_rules_change(self, event, state_events): @@ -461,6 +470,47 @@ class RoomAccessRules(object): return True + def _on_room_avatar_change(self, event, rule): + """Check whether a change of room avatar is allowed. + The current rule is to forbid such a change in direct chats but allow it + everywhere else. + + 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. + """ + return rule != ACCESS_RULE_DIRECT + + + def _on_room_name_change(self, event, rule): + """Check whether a change of room name is allowed. + The current rule is to forbid such a change in direct chats but allow it + everywhere else. + + 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. + """ + return rule != ACCESS_RULE_DIRECT + + + def _on_room_topic_change(self, event, rule): + """Check whether a change of room topic is allowed. + The current rule is to forbid such a change in direct chats but allow it + everywhere else. + + 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. + """ + return rule != ACCESS_RULE_DIRECT + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. -- cgit 1.5.1 From d1d464388acff70b75b61bd1ec6b9d33999c5924 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 5 Sep 2019 16:35:13 +0100 Subject: Lint --- synapse/third_party_rules/access_rules.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 41862f6d0b..f564c0484c 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -483,7 +483,6 @@ class RoomAccessRules(object): """ return rule != ACCESS_RULE_DIRECT - def _on_room_name_change(self, event, rule): """Check whether a change of room name is allowed. The current rule is to forbid such a change in direct chats but allow it @@ -497,7 +496,6 @@ class RoomAccessRules(object): """ return rule != ACCESS_RULE_DIRECT - def _on_room_topic_change(self, event, rule): """Check whether a change of room topic is allowed. The current rule is to forbid such a change in direct chats but allow it -- cgit 1.5.1 From b15557cd466c30281c92436c2d5a05b9680c1786 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:32:11 +0100 Subject: Don't treat 3PID revokation as a new 3PID invite --- synapse/third_party_rules/access_rules.py | 26 ++++++++++---- tests/rest/client/test_room_access_rules.py | 55 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 1a295ea7ce..5698e3e062 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -265,7 +265,7 @@ class RoomAccessRules(object): # 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( - state_events, + state_events, event ) if len(existing_members) > 2 or len(threepid_tokens) > 1: @@ -356,7 +356,7 @@ class RoomAccessRules(object): """ # Get the room memberships and 3PID invite tokens from the room's state. existing_members, threepid_tokens = self._get_members_and_tokens_from_state( - state_events, + state_events, event ) # There should never be more than one 3PID invite in the room state: if the second @@ -494,13 +494,14 @@ class RoomAccessRules(object): return join_rule_event.content.get("join_rule") @staticmethod - def _get_members_and_tokens_from_state(state_events): + def _get_members_and_tokens_from_state(state_events, event): """Retrieves from a list of state events the list of users that have a m.room.member event in the room, and the tokens of 3PID invites in the room. Args: state_events (dict[tuple[event type, state key], EventBase]): The set of state events. + event (EventBase): The event being checked. Returns: existing_members (list[str]): List of targets of the m.room.member events in the state. @@ -509,13 +510,24 @@ class RoomAccessRules(object): """ existing_members = [] threepid_invite_tokens = [] - for key, event in state_events.items(): + for key, state_event in state_events.items(): if key[0] == EventTypes.Member: - existing_members.append(event.state_key) + existing_members.append(state_event.state_key) if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_tokens.append(event.state_key) + threepid_invite_tokens.append(state_event.state_key) - return existing_members, threepid_invite_tokens + # If the event is a state event, there already is an event with the same state key + # in the room's state, then the event is updating an existing event from the + # room's state, in which case we need to remove the entry from the list in order + # to avoid conflicts. + if event.is_state(): + def filter_out_event(state_key): + return event.state_key != state_key + + existing_members = filter(filter_out_event, existing_members) + threepid_invite_tokens = filter(filter_out_event, threepid_invite_tokens) + + return list(existing_members), list(threepid_invite_tokens) @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index 7e23add6b7..bb164c1e5e 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -483,6 +483,44 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): expected_code=403, ) + def test_revoke_3pid_invite_direct(self): + """Tests that revoking a 3PID invite doesn't cause the room access rules module to + confuse the revokation as a new 3PID invite. + """ + invite_token = "sometoken" + + invite_body = { + "display_name": "ker...@exa...", + "public_keys": [ + { + "key_validity_url": "https://validity_url", + "public_key": "ta8IQ0u1sp44HVpxYi7dFOdS/bfwDjcy4xLFlfY5KOA" + }, + { + "key_validity_url": "https://validity_url", + "public_key": "4_9nzEeDwR5N9s51jPodBiLnqH43A2_g2InVT137t9I" + } + ], + "key_validity_url": "https://validity_url", + "public_key": "ta8IQ0u1sp44HVpxYi7dFOdS/bfwDjcy4xLFlfY5KOA" + } + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body=invite_body, + tok=self.tok, + ) + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body={}, + tok=self.tok, + ) + def create_room( self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, initial_state=None, expected_code=200, @@ -574,3 +612,20 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): ) self.render(request) self.assertEqual(channel.code, expected_code, channel.result) + + def send_state_with_state_key( + self, room_id, event_type, state_key, body, tok, expect_code=200 + ): + path = "/_matrix/client/r0/rooms/%s/state/%s/%s" % ( + room_id, event_type, state_key + ) + + request, channel = self.make_request( + "PUT", path, json.dumps(body), access_token=tok + ) + self.render(request) + + self.assertEqual(channel.code, expect_code, channel.result) + + return channel.json_body + -- cgit 1.5.1 From b2ec4467c942975dea8d69a0bec76d88e344f54d Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:36:43 +0100 Subject: Don't process revoked/redacted events as part of the room's membership info --- synapse/third_party_rules/access_rules.py | 4 ++-- tests/rest/client/test_room_access_rules.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 5698e3e062..ac09807738 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -511,9 +511,9 @@ class RoomAccessRules(object): existing_members = [] threepid_invite_tokens = [] for key, state_event in state_events.items(): - if key[0] == EventTypes.Member: + if key[0] == EventTypes.Member and state_event.content: existing_members.append(state_event.state_key) - if key[0] == EventTypes.ThirdPartyInvite: + if key[0] == EventTypes.ThirdPartyInvite and state_event.content: threepid_invite_tokens.append(state_event.state_key) # If the event is a state event, there already is an event with the same state key diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py index bb164c1e5e..8fa828fa15 100644 --- a/tests/rest/client/test_room_access_rules.py +++ b/tests/rest/client/test_room_access_rules.py @@ -521,6 +521,16 @@ class RoomAccessTestCase(unittest.HomeserverTestCase): tok=self.tok, ) + invite_token = "someothertoken" + + self.send_state_with_state_key( + room_id=self.direct_rooms[1], + event_type=EventTypes.ThirdPartyInvite, + state_key=invite_token, + body=invite_body, + tok=self.tok, + ) + def create_room( self, direct=False, rule=None, preset=RoomCreationPreset.TRUSTED_PRIVATE_CHAT, initial_state=None, expected_code=200, -- cgit 1.5.1 From e1c4d2c8bacaa85712eb3a26d6b7dd2fb34585d4 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 15:48:02 +0100 Subject: Only filter on 3PID invite tokens --- synapse/third_party_rules/access_rules.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 0fa36ba200..2635200989 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -569,13 +569,13 @@ class RoomAccessRules(object): # room's state, in which case we need to remove the entry from the list in order # to avoid conflicts. if event.is_state(): - def filter_out_event(state_key): - return event.state_key != state_key - - existing_members = filter(filter_out_event, existing_members) - threepid_invite_tokens = filter(filter_out_event, threepid_invite_tokens) + existing_members = existing_members + threepid_invite_tokens = filter( + lambda sk: event.state_key != sk, + threepid_invite_tokens, + ) - return list(existing_members), list(threepid_invite_tokens) + return existing_members, len(threepid_invite_tokens) @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): -- cgit 1.5.1 From 5076c2ebf6c967590c0fc8c51ef13154ac356ee6 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 6 Sep 2019 17:48:42 +0100 Subject: Typo --- synapse/third_party_rules/access_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 2635200989..33496519c3 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -575,7 +575,7 @@ class RoomAccessRules(object): threepid_invite_tokens, ) - return existing_members, len(threepid_invite_tokens) + return existing_members, list(threepid_invite_tokens) @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): -- cgit 1.5.1 From 66be293c79cb240f37f585af2451dd814d8894e2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 9 Sep 2019 17:52:41 +0100 Subject: Process revocations in _on_membership_or_invite_direct --- synapse/third_party_rules/access_rules.py | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 33496519c3..55b59fba8d 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -274,7 +274,7 @@ class RoomAccessRules(object): # 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( - state_events, event + state_events ) if len(existing_members) > 2 or len(threepid_tokens) > 1: @@ -365,7 +365,7 @@ class RoomAccessRules(object): """ # Get the room memberships and 3PID invite tokens from the room's state. existing_members, threepid_tokens = self._get_members_and_tokens_from_state( - state_events, event + state_events ) # There should never be more than one 3PID invite in the room state: if the second @@ -374,8 +374,12 @@ class RoomAccessRules(object): # join the first time), Synapse will successfully look it up before attempting to # store an invite on the IS. if len(threepid_tokens) == 1 and event.type == EventTypes.ThirdPartyInvite: - # If we already have a 3PID invite in flight, don't accept another one. - return False + # If we already have a 3PID invite in flight, don't accept another one, unless + # the new one has the same invite token as its state key. This is because 3PID + # invite revocations must be allowed, and a revocation is basically a new 3PID + # invite event with an empty content and the same token as the invite it + # revokes. + return event.state_key in threepid_tokens if len(existing_members) == 2: # If the user was within the two initial user of the room, Synapse would have @@ -542,14 +546,13 @@ class RoomAccessRules(object): return join_rule_event.content.get("join_rule") @staticmethod - def _get_members_and_tokens_from_state(state_events, event): + def _get_members_and_tokens_from_state(state_events): """Retrieves from a list of state events the list of users that have a m.room.member event in the room, and the tokens of 3PID invites in the room. Args: state_events (dict[tuple[event type, state key], EventBase]): The set of state events. - event (EventBase): The event being checked. Returns: existing_members (list[str]): List of targets of the m.room.member events in the state. @@ -562,20 +565,10 @@ class RoomAccessRules(object): if key[0] == EventTypes.Member and state_event.content: existing_members.append(state_event.state_key) if key[0] == EventTypes.ThirdPartyInvite and state_event.content: + # Don't include revoked invites. threepid_invite_tokens.append(state_event.state_key) - # If the event is a state event, there already is an event with the same state key - # in the room's state, then the event is updating an existing event from the - # room's state, in which case we need to remove the entry from the list in order - # to avoid conflicts. - if event.is_state(): - existing_members = existing_members - threepid_invite_tokens = filter( - lambda sk: event.state_key != sk, - threepid_invite_tokens, - ) - - return existing_members, list(threepid_invite_tokens) + return existing_members, threepid_invite_tokens @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): -- cgit 1.5.1 From e6a7f8964f6f6f87493a6f81002d2149e177d639 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Sep 2019 11:12:21 +0100 Subject: Allow membership events which membership isn't join or invite in restricted rooms --- synapse/third_party_rules/access_rules.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'synapse/third_party_rules') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 55b59fba8d..bd79de845f 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, JoinRules, RoomCreationPreset +from synapse.api.constants import EventTypes, JoinRules, Membership, RoomCreationPreset from synapse.api.errors import SynapseError from synapse.config._base import ConfigError from synapse.types import get_domain_from_id @@ -336,6 +336,14 @@ class RoomAccessRules(object): # called before check_event_allowed. if event.type == EventTypes.ThirdPartyInvite: return True + + # We only need to process "join" and "invite" memberships, in order to be backward + # compatible, e.g. if a user from a blacklisted server joined a restricted room + # before the rules started being enforced on the server, that user must be able to + # leave it. + if event.membership not in [Membership.JOIN, Membership.INVITE]: + return True + invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted -- cgit 1.5.1