From 4b0e761ae09f7e3d1bbd2e8362d23816502cfa2a Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Fri, 14 Aug 2020 13:53:53 -0400
Subject: [PATCH 1/5] Stop shadow-banned users from sending invites.

---
 changelog.d/8095.feature        |  1 +
 synapse/api/errors.py           |  8 ++++
 synapse/handlers/room.py        | 16 +++++++-
 synapse/handlers/room_member.py | 62 +++++++++++++++++++++++++++++-
 synapse/rest/admin/rooms.py     |  3 ++
 synapse/rest/client/v1/room.py  | 67 ++++++++++++++++++++-------------
 6 files changed, 126 insertions(+), 31 deletions(-)
 create mode 100644 changelog.d/8095.feature

diff --git a/changelog.d/8095.feature b/changelog.d/8095.feature
new file mode 100644
index 000000000000..813e6d0903d9
--- /dev/null
+++ b/changelog.d/8095.feature
@@ -0,0 +1 @@
+Add support for shadow-banning users (ignoring any message send requests).
diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 6e40630ab6d4..6e8b38ddc4a3 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -604,3 +604,11 @@ def to_synapse_error(self):
         errmsg = j.pop("error", self.msg)
 
         return ProxiedRequestError(self.code, errmsg, errcode, j)
+
+
+class ShadowBanError(Exception):
+    """
+    Raised when a shadow-banned user attempts to perform an action.
+
+    This should be caught and a proper "fake" success response send to the user.
+    """
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 442cca28e6b5..efc54908adb4 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -20,6 +20,7 @@
 import itertools
 import logging
 import math
+import random
 import string
 from collections import OrderedDict
 from typing import TYPE_CHECKING, Any, Awaitable, Dict, List, Optional, Tuple
@@ -626,6 +627,7 @@ async def create_room(
             if mapping:
                 raise SynapseError(400, "Room alias already taken", Codes.ROOM_IN_USE)
 
+        invite_3pid_list = config.get("invite_3pid", [])
         invite_list = config.get("invite", [])
         for i in invite_list:
             try:
@@ -634,6 +636,14 @@ async def create_room(
             except Exception:
                 raise SynapseError(400, "Invalid user_id: %s" % (i,))
 
+        if (invite_list or invite_3pid_list) and requester.shadow_banned:
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+
+            # Allow the request to go through, but remove any associated invites.
+            invite_3pid_list = []
+            invite_list = []
+
         await self.event_creation_handler.assert_accepted_privacy_policy(requester)
 
         power_level_content_override = config.get("power_level_content_override")
@@ -648,8 +658,6 @@ async def create_room(
                 % (user_id,),
             )
 
-        invite_3pid_list = config.get("invite_3pid", [])
-
         visibility = config.get("visibility", None)
         is_public = visibility == "public"
 
@@ -744,6 +752,8 @@ async def create_room(
             if is_direct:
                 content["is_direct"] = is_direct
 
+            # Note that update_membership with an action of "invite" can raise a
+            # ShadowBanError, but this was handled above by emptying invite_list.
             _, last_stream_id = await self.room_member_handler.update_membership(
                 requester,
                 UserID.from_string(invitee),
@@ -758,6 +768,8 @@ async def create_room(
             id_access_token = invite_3pid.get("id_access_token")  # optional
             address = invite_3pid["address"]
             medium = invite_3pid["medium"]
+            # Note that do_3pid_invite can raise a  ShadowBanError, but this was
+            # handled above by emptying invite_3pid_list.
             last_stream_id = await self.hs.get_room_member_handler().do_3pid_invite(
                 room_id,
                 requester.user,
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 31705cdbdb7d..b16555e1a786 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -15,6 +15,7 @@
 
 import abc
 import logging
+import random
 from http import HTTPStatus
 from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, Union
 
@@ -22,7 +23,13 @@
 
 from synapse import types
 from synapse.api.constants import MAX_DEPTH, EventTypes, Membership
-from synapse.api.errors import AuthError, Codes, LimitExceededError, SynapseError
+from synapse.api.errors import (
+    AuthError,
+    Codes,
+    LimitExceededError,
+    ShadowBanError,
+    SynapseError,
+)
 from synapse.api.ratelimiting import Ratelimiter
 from synapse.api.room_versions import EventFormatVersions
 from synapse.crypto.event_signing import compute_event_reference_hash
@@ -285,6 +292,31 @@ async def update_membership(
         content: Optional[dict] = None,
         require_consent: bool = True,
     ) -> Tuple[str, int]:
+        """Update a user's membership in a room.
+
+        Params:
+            requester: The user who is performing the update.
+            target: The user whose membership is being updated.
+            room_id: The room ID whose membership is being updated.
+            action: The membership change, see synapse.api.constants.Membership.
+            txn_id: The transaction ID, if given.
+            remote_room_hosts: Remote servers to send the update to.
+            third_party_signed: Information from a 3PID invite.
+            ratelimit: Whether to rate limit the request.
+            content: The content of the created event.
+            require_consent: Whether consent is required.
+
+        Returns:
+            A tuple of the new event ID and stream ID.
+
+        Raises:
+            ShadowBanError if a shadow-banned requester attempts to send an invite.
+        """
+        if action == Membership.INVITE and requester.shadow_banned:
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+            raise ShadowBanError()
+
         key = (room_id,)
 
         with (await self.member_linearizer.queue(key)):
@@ -773,6 +805,25 @@ async def do_3pid_invite(
         txn_id: Optional[str],
         id_access_token: Optional[str] = None,
     ) -> int:
+        """Invite a 3PID to a room.
+
+        Args:
+            room_id: The room to invite the 3PID to.
+            inviter: The user sending the invite.
+            medium: The 3PID's medium.
+            address: The 3PID's address.
+            id_server: The identity server to use.
+            requester: The user making the request.
+            txn_id: The transaction ID this is part of, or None if this is not
+                part of a transaction.
+            id_access_token: The optional identity server access token.
+
+        Returns:
+             The new stream ID.
+
+        Raises:
+            ShadowBanError if the requester has been shadow-banned.
+        """
         if self.config.block_non_admin_invites:
             is_requester_admin = await self.auth.is_server_admin(requester.user)
             if not is_requester_admin:
@@ -780,6 +831,11 @@ async def do_3pid_invite(
                     403, "Invites have been disabled on this server", Codes.FORBIDDEN
                 )
 
+        if requester.shadow_banned:
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+            raise ShadowBanError()
+
         # We need to rate limit *before* we send out any 3PID invites, so we
         # can't just rely on the standard ratelimiting of events.
         await self.base_handler.ratelimit(requester)
@@ -804,6 +860,8 @@ async def do_3pid_invite(
         )
 
         if invitee:
+            # Note that update_membership with an action of "invite" can raise
+            # a ShadowBanError, but this was done above already.
             _, stream_id = await self.update_membership(
                 requester, UserID.from_string(invitee), room_id, "invite", txn_id=txn_id
             )
@@ -1042,7 +1100,7 @@ async def _remote_join(
                 return event_id, stream_id
 
             # The room is too large. Leave.
-            requester = types.create_requester(user, None, False, None)
+            requester = types.create_requester(user, None, False, False, None)
             await self.update_membership(
                 requester=requester, target=user, room_id=room_id, action="leave"
             )
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 7c292ef3f939..436be0d867a7 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -316,6 +316,9 @@ async def on_POST(self, request, room_identifier):
         join_rules_event = room_state.get((EventTypes.JoinRules, ""))
         if join_rules_event:
             if not (join_rules_event.content.get("join_rule") == JoinRules.PUBLIC):
+                # update_membership with an action of "invite" can raise a
+                # ShadowBanError. This is not handled since it is assumed that
+                # an admin isn't going to call this API with shadow-banned user.
                 await self.room_member_handler.update_membership(
                     requester=requester,
                     target=fake_requester.user,
diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 2ab30ce8977d..52160b3a3cd5 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -29,6 +29,7 @@
     Codes,
     HttpResponseException,
     InvalidClientCredentialsError,
+    ShadowBanError,
     SynapseError,
 )
 from synapse.api.filtering import Filter
@@ -46,6 +47,7 @@
 from synapse.storage.state import StateFilter
 from synapse.streams.config import PaginationConfig
 from synapse.types import RoomAlias, RoomID, StreamToken, ThirdPartyInstanceID, UserID
+from synapse.util.stringutils import random_string
 
 MYPY = False
 if MYPY:
@@ -201,14 +203,17 @@ async def on_PUT(self, request, room_id, event_type, state_key, txn_id=None):
             event_dict["state_key"] = state_key
 
         if event_type == EventTypes.Member:
-            membership = content.get("membership", None)
-            event_id, _ = await self.room_member_handler.update_membership(
-                requester,
-                target=UserID.from_string(state_key),
-                room_id=room_id,
-                action=membership,
-                content=content,
-            )
+            try:
+                membership = content.get("membership", None)
+                event_id, _ = await self.room_member_handler.update_membership(
+                    requester,
+                    target=UserID.from_string(state_key),
+                    room_id=room_id,
+                    action=membership,
+                    content=content,
+                )
+            except ShadowBanError:
+                event_id = "$" + random_string(43)
         else:
             (
                 event,
@@ -716,16 +721,20 @@ async def on_POST(self, request, room_id, membership_action, txn_id=None):
             content = {}
 
         if membership_action == "invite" and self._has_3pid_invite_keys(content):
-            await self.room_member_handler.do_3pid_invite(
-                room_id,
-                requester.user,
-                content["medium"],
-                content["address"],
-                content["id_server"],
-                requester,
-                txn_id,
-                content.get("id_access_token"),
-            )
+            try:
+                await self.room_member_handler.do_3pid_invite(
+                    room_id,
+                    requester.user,
+                    content["medium"],
+                    content["address"],
+                    content["id_server"],
+                    requester,
+                    txn_id,
+                    content.get("id_access_token"),
+                )
+            except ShadowBanError:
+                # Pretend the request succeed.
+                pass
             return 200, {}
 
         target = requester.user
@@ -737,15 +746,19 @@ async def on_POST(self, request, room_id, membership_action, txn_id=None):
         if "reason" in content:
             event_content = {"reason": content["reason"]}
 
-        await self.room_member_handler.update_membership(
-            requester=requester,
-            target=target,
-            room_id=room_id,
-            action=membership_action,
-            txn_id=txn_id,
-            third_party_signed=content.get("third_party_signed", None),
-            content=event_content,
-        )
+        try:
+            await self.room_member_handler.update_membership(
+                requester=requester,
+                target=target,
+                room_id=room_id,
+                action=membership_action,
+                txn_id=txn_id,
+                third_party_signed=content.get("third_party_signed", None),
+                content=event_content,
+            )
+        except ShadowBanError:
+            # Pretend the request succeed.
+            pass
 
         return_value = {}
 

From 25961b3b091ca5e50375e52b20e29852941da4f4 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Fri, 14 Aug 2020 14:02:22 -0400
Subject: [PATCH 2/5] Add tests.

---
 tests/rest/client/v1/test_rooms.py | 63 ++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index ef6b775ed2b8..86849ef60c96 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -1974,3 +1974,66 @@ def test_bad_alias(self):
         """An alias which does not point to the room raises a SynapseError."""
         self._set_canonical_alias({"alias": "@unknown:test"}, expected_code=400)
         self._set_canonical_alias({"alt_aliases": ["@unknown:test"]}, expected_code=400)
+
+
+class ShadowBannedTestCase(unittest.HomeserverTestCase):
+    servlets = [
+        synapse.rest.admin.register_servlets_for_client_rest_resource,
+        directory.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(self, reactor, clock, homeserver):
+        self.banned_user_id = self.register_user("banned", "test")
+        self.banned_access_token = self.login("banned", "test")
+
+        self.store = self.hs.get_datastore()
+
+        self.get_success(
+            self.store.db_pool.simple_update(
+                table="users",
+                keyvalues={"name": self.banned_user_id},
+                updatevalues={"shadow_banned": True},
+                desc="shadow_ban",
+            )
+        )
+
+        self.other_user_id = self.register_user("otheruser", "pass")
+        self.other_access_token = self.login("otheruser", "pass")
+
+    def test_invite(self):
+        """Invites from shadow-banned users don't actually get sent."""
+
+        # The create works fine.
+        room_id = self.helper.create_room_as(
+            self.banned_user_id, tok=self.banned_access_token
+        )
+
+        # Inviting the user completes successfully.
+        self.helper.invite(
+            room=room_id,
+            src=self.banned_user_id,
+            tok=self.banned_access_token,
+            targ=self.other_user_id,
+        )
+
+        # But the user wasn't actually invited.
+        invited_rooms = self.get_success(
+            self.store.get_invited_rooms_for_local_user(self.other_user_id)
+        )
+        self.assertEqual(invited_rooms, [])
+
+    def test_create_room(self):
+        """A shadow-banned users should be able to create a room."""
+
+        room_id = self.helper.create_room_as(
+            self.banned_user_id, tok=self.banned_access_token
+        )
+
+        # This creates a real room, so the other user should be able to join it.
+        self.helper.join(room_id, self.other_user_id, tok=self.other_access_token)
+
+        # Both users should be in the room.
+        users = self.get_success(self.store.get_users_in_room(room_id))
+        self.assertCountEqual(users, ["@banned:test", "@otheruser:test"])

From 25f4b3082c4e61227011025ff770fbd104f57915 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Wed, 19 Aug 2020 13:28:56 -0400
Subject: [PATCH 3/5] Fix lots of typos.

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
---
 synapse/api/errors.py              | 2 +-
 synapse/handlers/room.py           | 2 +-
 synapse/handlers/room_member.py    | 4 ++--
 synapse/rest/admin/rooms.py        | 2 +-
 synapse/rest/client/v1/room.py     | 4 ++--
 tests/rest/client/v1/test_rooms.py | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 6e8b38ddc4a3..7d972e6adeda 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -610,5 +610,5 @@ class ShadowBanError(Exception):
     """
     Raised when a shadow-banned user attempts to perform an action.
 
-    This should be caught and a proper "fake" success response send to the user.
+    This should be caught and a proper "fake" success response sent to the user.
     """
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index efc54908adb4..0fc71475c36d 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -637,7 +637,7 @@ async def create_room(
                 raise SynapseError(400, "Invalid user_id: %s" % (i,))
 
         if (invite_list or invite_3pid_list) and requester.shadow_banned:
-            # We randomly sleep a bit just to annoy the requester a bit.
+            # We randomly sleep a bit just to annoy the requester.
             await self.clock.sleep(random.randint(1, 10))
 
             # Allow the request to go through, but remove any associated invites.
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index b16555e1a786..0369decad009 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -313,7 +313,7 @@ async def update_membership(
             ShadowBanError if a shadow-banned requester attempts to send an invite.
         """
         if action == Membership.INVITE and requester.shadow_banned:
-            # We randomly sleep a bit just to annoy the requester a bit.
+            # We randomly sleep a bit just to annoy the requester.
             await self.clock.sleep(random.randint(1, 10))
             raise ShadowBanError()
 
@@ -832,7 +832,7 @@ async def do_3pid_invite(
                 )
 
         if requester.shadow_banned:
-            # We randomly sleep a bit just to annoy the requester a bit.
+            # We randomly sleep a bit just to annoy the requester.
             await self.clock.sleep(random.randint(1, 10))
             raise ShadowBanError()
 
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 436be0d867a7..09726d52d67e 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -318,7 +318,7 @@ async def on_POST(self, request, room_identifier):
             if not (join_rules_event.content.get("join_rule") == JoinRules.PUBLIC):
                 # update_membership with an action of "invite" can raise a
                 # ShadowBanError. This is not handled since it is assumed that
-                # an admin isn't going to call this API with shadow-banned user.
+                # an admin isn't going to call this API with a shadow-banned user.
                 await self.room_member_handler.update_membership(
                     requester=requester,
                     target=fake_requester.user,
diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 52160b3a3cd5..594adf8af881 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -733,7 +733,7 @@ async def on_POST(self, request, room_id, membership_action, txn_id=None):
                     content.get("id_access_token"),
                 )
             except ShadowBanError:
-                # Pretend the request succeed.
+                # Pretend the request succeeded.
                 pass
             return 200, {}
 
@@ -757,7 +757,7 @@ async def on_POST(self, request, room_id, membership_action, txn_id=None):
                 content=event_content,
             )
         except ShadowBanError:
-            # Pretend the request succeed.
+            # Pretend the request succeeded.
             pass
 
         return_value = {}
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 86849ef60c96..b3992e92aabd 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -2025,7 +2025,7 @@ def test_invite(self):
         self.assertEqual(invited_rooms, [])
 
     def test_create_room(self):
-        """A shadow-banned users should be able to create a room."""
+        """A shadow-banned user should be able to create a room."""
 
         room_id = self.helper.create_room_as(
             self.banned_user_id, tok=self.banned_access_token

From 86193e69fac5c42f3c4dd560b48ec6b608057479 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 19 Aug 2020 14:15:12 -0400
Subject: [PATCH 4/5] Add a test for inviting a 3PID.

---
 tests/rest/client/v1/test_rooms.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index b3992e92aabd..dee495e4a23d 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -2024,6 +2024,31 @@ def test_invite(self):
         )
         self.assertEqual(invited_rooms, [])
 
+    def test_invite_3pid(self):
+        """Ensure that a 3PID invite does not attempt to contact the identity server."""
+        identity_handler = self.hs.get_handlers().identity_handler
+        identity_handler.lookup_3pid = Mock(
+            side_effect=AssertionError("This should not get called")
+        )
+
+        # The create works fine.
+        room_id = self.helper.create_room_as(
+            self.banned_user_id, tok=self.banned_access_token
+        )
+
+        # Inviting the user completes successfully.
+        request, channel = self.make_request(
+            "POST",
+            "/rooms/%s/invite" % (room_id,),
+            {"id_server": "test", "medium": "email", "address": "test@test.test"},
+            access_token=self.banned_access_token,
+        )
+        self.render(request)
+        self.assertEquals(200, channel.code, channel.result)
+
+        # This should have raised an error earlier, but double check this wasn't called.
+        identity_handler.lookup_3pid.assert_not_called()
+
     def test_create_room(self):
         """A shadow-banned user should be able to create a room."""
 

From 6279ee63a45656d2838f2d1ed1ecbe2043eb66c6 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 19 Aug 2020 14:22:31 -0400
Subject: [PATCH 5/5] Expand creation test to include invites.

---
 tests/rest/client/v1/test_rooms.py | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index dee495e4a23d..e674eb90d7f6 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -2050,13 +2050,25 @@ def test_invite_3pid(self):
         identity_handler.lookup_3pid.assert_not_called()
 
     def test_create_room(self):
-        """A shadow-banned user should be able to create a room."""
+        """Invitations during a room creation should be discarded, but the room still gets created."""
+        # The room creation is successful.
+        request, channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/createRoom",
+            {"visibility": "public", "invite": [self.other_user_id]},
+            access_token=self.banned_access_token,
+        )
+        self.render(request)
+        self.assertEquals(200, channel.code, channel.result)
+        room_id = channel.json_body["room_id"]
 
-        room_id = self.helper.create_room_as(
-            self.banned_user_id, tok=self.banned_access_token
+        # But the user wasn't actually invited.
+        invited_rooms = self.get_success(
+            self.store.get_invited_rooms_for_local_user(self.other_user_id)
         )
+        self.assertEqual(invited_rooms, [])
 
-        # This creates a real room, so the other user should be able to join it.
+        # Since a real room was created, the other user should be able to join it.
         self.helper.join(room_id, self.other_user_id, tok=self.other_access_token)
 
         # Both users should be in the room.