Merge pull request #2 from matrix-org/babolivier/dinsic-3pid-invite

Don't treat 3PID revocation as a new 3PID invite

changelog.d/2.bugfix

@@ -0,0 +1 @@
+Don't treat 3PID revocation as a new 3PID invite.

synapse/third_party_rules/access_rules.py

@@ -274,7 +274,7 @@ def _on_rules_change(self, event, state_events):
         # 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
             )
 
             if len(existing_members) > 2 or len(threepid_tokens) > 1:
@@ -365,7 +365,7 @@ def _on_membership_or_invite_direct(self, event, state_events):
         """
         # 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
         )
 
         # There should never be more than one 3PID invite in the room state: if the second
@@ -374,8 +374,12 @@ def _on_membership_or_invite_direct(self, event, state_events):
         # 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
@@ -557,11 +561,12 @@ def _get_members_and_tokens_from_state(state_events):
         """
         existing_members = []
         threepid_invite_tokens = []
-        for key, event in state_events.items():
-            if key[0] == EventTypes.Member:
-                existing_members.append(event.state_key)
-            if key[0] == EventTypes.ThirdPartyInvite:
-                threepid_invite_tokens.append(event.state_key)
+        for key, state_event in state_events.items():
+            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)
 
         return existing_members, threepid_invite_tokens
 

tests/rest/client/test_room_access_rules.py

@@ -588,6 +588,54 @@ def test_change_room_topic(self):
             expect_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,
+        )
+
+        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,
@@ -679,3 +727,19 @@ def send_threepid_invite(self, address, room_id, expected_code=200):
         )
         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