From 17549a25aadb7c17140499adf9e8d40531de5769 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 3 Aug 2020 13:24:22 -0400
Subject: [PATCH 01/12] Support shadow banning users.

---
 synapse/api/errors.py                         | 16 +++++++++++++
 synapse/handlers/message.py                   | 13 ++++++++++
 synapse/handlers/register.py                  |  8 +++++++
 synapse/handlers/room.py                      | 17 ++++++++++---
 synapse/handlers/room_member.py               | 24 ++++++++++++++++++-
 synapse/replication/http/register.py          |  4 ++++
 .../storage/data_stores/main/registration.py  | 16 +++++++++++++
 .../main/schema/delta/58/09shadow_ban.sql     | 16 +++++++++++++
 8 files changed, 110 insertions(+), 4 deletions(-)
 create mode 100644 synapse/storage/data_stores/main/schema/delta/58/09shadow_ban.sql

diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index b3bab1aa526c..0a8393a9dfe1 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -145,6 +145,22 @@ def error_dict(self):
         return cs_error(self.msg, self.errcode)
 
 
+class ShadowBanError(SynapseError):
+    """An exception that returns a 200 OK with a fake body.
+    """
+
+    def __init__(self, body=None):
+        if body is None:
+            body = {}
+
+        self.body = body
+
+        super().__init__(200, "")
+
+    def error_dict(self):
+        return self.body
+
+
 class ProxiedRequestError(SynapseError):
     """An error from a general matrix endpoint, eg. from a proxied Matrix API call.
 
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index e451d6dc86cb..4a63c72513d2 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -15,6 +15,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
+import random
 from typing import TYPE_CHECKING, List, Optional, Tuple
 
 from canonicaljson import encode_canonical_json, json
@@ -34,6 +35,7 @@
     Codes,
     ConsentNotGivenError,
     NotFoundError,
+    ShadowBanError,
     SynapseError,
 )
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
@@ -58,6 +60,7 @@
 from synapse.util.async_helpers import Linearizer
 from synapse.util.frozenutils import frozendict_json_encoder
 from synapse.util.metrics import measure_func
+from synapse.util.stringutils import random_string
 from synapse.visibility import filter_events_for_client
 
 from ._base import BaseHandler
@@ -710,6 +713,7 @@ async def create_and_send_nonmember_event(
         event_dict: EventBase,
         ratelimit: bool = True,
         txn_id: Optional[str] = None,
+        ignore_shadow_ban: bool = False,
     ) -> Tuple[EventBase, int]:
         """
         Creates an event, then sends it.
@@ -717,6 +721,15 @@ async def create_and_send_nonmember_event(
         See self.create_event and self.send_nonmember_event.
         """
 
+        if not ignore_shadow_ban and await self.store.is_shadow_banned(
+            requester.user.to_string()
+        ):
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+
+            # We return a response that looks roughly legit.
+            raise ShadowBanError({"event_id": "$" + random_string(43)})
+
         # We limit the number of concurrent event sends in a room so that we
         # don't fork the DAG too much. If we don't limit then we can end up in
         # a situation where event persistence can't keep up, causing
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index c94209ab3d4e..2b8cb48be4ff 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -142,6 +142,7 @@ async def register_user(
         address=None,
         bind_emails=[],
         by_admin=False,
+        shadow_banned=False,
     ):
         """Registers a new client on the server.
 
@@ -159,6 +160,7 @@ async def register_user(
             bind_emails (List[str]): list of emails to bind to this account.
             by_admin (bool): True if this registration is being made via the
               admin api, otherwise False.
+            shadow_banned (bool): Shadow ban the created user.
         Returns:
             str: user_id
         Raises:
@@ -194,6 +196,7 @@ async def register_user(
                 admin=admin,
                 user_type=user_type,
                 address=address,
+                shadow_banned=shadow_banned,
             )
 
             if self.hs.config.user_directory_search_all_users:
@@ -224,6 +227,7 @@ async def register_user(
                         make_guest=make_guest,
                         create_profile_with_displayname=default_display_name,
                         address=address,
+                        shadow_banned=shadow_banned,
                     )
 
                     # Successfully registered
@@ -529,6 +533,7 @@ def register_with_store(
         admin=False,
         user_type=None,
         address=None,
+        shadow_banned=False,
     ):
         """Register user in the datastore.
 
@@ -546,6 +551,7 @@ def register_with_store(
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
             address (str|None): the IP address used to perform the registration.
+            shadow_banned (bool): Whether to shadow ban the user
 
         Returns:
             Awaitable
@@ -561,6 +567,7 @@ def register_with_store(
                 admin=admin,
                 user_type=user_type,
                 address=address,
+                shadow_banned=shadow_banned,
             )
         else:
             return self.store.register_user(
@@ -572,6 +579,7 @@ def register_with_store(
                 create_profile_with_displayname=create_profile_with_displayname,
                 admin=admin,
                 user_type=user_type,
+                shadow_banned=shadow_banned,
             )
 
     async def register_device(
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 0c5b99234d27..c8bafa24cd2c 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 Optional, Tuple
@@ -614,6 +615,7 @@ async def create_room(
         else:
             room_alias = None
 
+        invite_3pid_list = config.get("invite_3pid", [])
         invite_list = config.get("invite", [])
         for i in invite_list:
             try:
@@ -622,6 +624,17 @@ async def create_room(
             except Exception:
                 raise SynapseError(400, "Invalid user_id: %s" % (i,))
 
+        if (invite_list or invite_3pid_list) and await self.store.is_shadow_banned(
+            user_id
+        ):
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+
+            # We actually allow this request to go through, but remove any
+            # associated invites
+            invite_list = []
+            invite_3pid_list = []
+
         await self.event_creation_handler.assert_accepted_privacy_policy(requester)
 
         power_level_content_override = config.get("power_level_content_override")
@@ -636,8 +649,6 @@ async def create_room(
                 % (user_id,),
             )
 
-        invite_3pid_list = config.get("invite_3pid", [])
-
         visibility = config.get("visibility", None)
         is_public = visibility == "public"
 
@@ -802,7 +813,7 @@ async def send(etype, content, **kwargs) -> int:
                 _,
                 last_stream_id,
             ) = await self.event_creation_handler.create_and_send_nonmember_event(
-                creator, event, ratelimit=False
+                creator, event, ratelimit=False, ignore_shadow_ban=True,
             )
             return last_stream_id
 
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 8e409f24e89e..9bc642466bbf 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 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
@@ -281,6 +288,14 @@ async def update_membership(
         content: Optional[dict] = None,
         require_consent: bool = True,
     ) -> Tuple[str, int]:
+        if action == Membership.INVITE and await self.store.is_shadow_banned(
+            requester.user.to_string()
+        ):
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+            # We return a response that looks roughly legit.
+            raise ShadowBanError({})
+
         key = (room_id,)
 
         with (await self.member_linearizer.queue(key)):
@@ -776,6 +791,13 @@ async def do_3pid_invite(
                     403, "Invites have been disabled on this server", Codes.FORBIDDEN
                 )
 
+        if await self.store.is_shadow_banned(requester.user.to_string()):
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+
+            # We return a response that looks roughly legit.
+            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)
diff --git a/synapse/replication/http/register.py b/synapse/replication/http/register.py
index ce9420aa695e..ef67bbdb9ccb 100644
--- a/synapse/replication/http/register.py
+++ b/synapse/replication/http/register.py
@@ -44,6 +44,7 @@ async def _serialize_payload(
         admin,
         user_type,
         address,
+        shadow_banned,
     ):
         """
         Args:
@@ -60,6 +61,7 @@ async def _serialize_payload(
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
             address (str|None): the IP address used to perform the regitration.
+            shadow_banned (bool): Whether to sahdow ban the user
         """
         return {
             "password_hash": password_hash,
@@ -70,6 +72,7 @@ async def _serialize_payload(
             "admin": admin,
             "user_type": user_type,
             "address": address,
+            "shadow_banned": shadow_banned,
         }
 
     async def _handle_request(self, request, user_id):
@@ -87,6 +90,7 @@ async def _handle_request(self, request, user_id):
             admin=content["admin"],
             user_type=content["user_type"],
             address=content["address"],
+            shadow_banned=content["shadow_banned"],
         )
 
         return 200, {}
diff --git a/synapse/storage/data_stores/main/registration.py b/synapse/storage/data_stores/main/registration.py
index 27d2c5028c42..356a3cbe4d1c 100644
--- a/synapse/storage/data_stores/main/registration.py
+++ b/synapse/storage/data_stores/main/registration.py
@@ -69,6 +69,15 @@ def get_user_by_id(self, user_id):
             desc="get_user_by_id",
         )
 
+    async def is_shadow_banned(self, user_id):
+        return await self.db.simple_select_one_onecol(
+            table="users",
+            keyvalues={"name": user_id},
+            retcol="shadow_banned",
+            allow_none=True,
+            desc="is_shadow_banned",
+        )
+
     @defer.inlineCallbacks
     def is_trial_user(self, user_id):
         """Checks if user is in the "trial" period, i.e. within the first
@@ -969,6 +978,7 @@ def register_user(
         create_profile_with_displayname=None,
         admin=False,
         user_type=None,
+        shadow_banned=False,
     ):
         """Attempts to register an account.
 
@@ -985,6 +995,8 @@ def register_user(
             admin (boolean): is an admin user?
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
+            shadow_banned (bool): Whether the user is shadow banned,
+                i.e. they may be told their requests succeeded but we ignored them.
 
         Raises:
             StoreError if the user_id could not be registered.
@@ -1003,6 +1015,7 @@ def register_user(
             create_profile_with_displayname,
             admin,
             user_type,
+            shadow_banned,
         )
 
     def _register_user(
@@ -1016,6 +1029,7 @@ def _register_user(
         create_profile_with_displayname,
         admin,
         user_type,
+        shadow_banned,
     ):
         user_id_obj = UserID.from_string(user_id)
 
@@ -1045,6 +1059,7 @@ def _register_user(
                         "appservice_id": appservice_id,
                         "admin": 1 if admin else 0,
                         "user_type": user_type,
+                        "shadow_banned": shadow_banned,
                     },
                 )
             else:
@@ -1059,6 +1074,7 @@ def _register_user(
                         "appservice_id": appservice_id,
                         "admin": 1 if admin else 0,
                         "user_type": user_type,
+                        "shadow_banned": shadow_banned,
                     },
                 )
 
diff --git a/synapse/storage/data_stores/main/schema/delta/58/09shadow_ban.sql b/synapse/storage/data_stores/main/schema/delta/58/09shadow_ban.sql
new file mode 100644
index 000000000000..9592a62b38e9
--- /dev/null
+++ b/synapse/storage/data_stores/main/schema/delta/58/09shadow_ban.sql
@@ -0,0 +1,16 @@
+/* Copyright 2020 The Matrix.org Foundation C.I.C
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ALTER TABLE users ADD COLUMN shadow_banned BOOLEAN;

From b9402e3a709e23aaf3840f6e6ddc03f8a42df7fa Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 3 Aug 2020 14:39:45 -0400
Subject: [PATCH 02/12] Add tests.

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

diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 5ccda8b2bd63..74a967ca4cdd 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -1976,3 +1976,77 @@ 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 ShadowBanTestCase(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.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_message(self):
+        """Messages from shadow banned users don't actually get sent."""
+
+        room_id = self.helper.create_room_as(
+            self.other_user_id, tok=self.other_access_token
+        )
+
+        # The user should be in the room.
+        self.helper.join(room_id, self.banned_user_id, tok=self.banned_access_token)
+
+        # Sending a message should complete successfully.
+        result = self.helper.send_event(
+            room_id=room_id,
+            type=EventTypes.Message,
+            content={"msgtype": "m.text", "body": "with right label"},
+            tok=self.banned_access_token,
+        )
+        self.assertIn("event_id", result)
+        event_id = result["event_id"]
+
+        latest_events = self.get_success(
+            self.store.get_latest_event_ids_in_room(room_id)
+        )
+        self.assertNotIn(event_id, latest_events)

From 190eca15b35d696668debf063c013e12e82d0d01 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 4 Aug 2020 07:45:27 -0400
Subject: [PATCH 03/12] Fix a typo.

---
 synapse/replication/http/register.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/replication/http/register.py b/synapse/replication/http/register.py
index ef67bbdb9ccb..4a8270eb1e00 100644
--- a/synapse/replication/http/register.py
+++ b/synapse/replication/http/register.py
@@ -61,7 +61,7 @@ async def _serialize_payload(
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
             address (str|None): the IP address used to perform the regitration.
-            shadow_banned (bool): Whether to sahdow ban the user
+            shadow_banned (bool): Whether to shadow ban the user
         """
         return {
             "password_hash": password_hash,

From fc7bbd2bf6a6c648cf0e995da56a175e8d12ff17 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 4 Aug 2020 07:48:06 -0400
Subject: [PATCH 04/12] Add a changelog entry.

---
 changelog.d/8028.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/8028.feature

diff --git a/changelog.d/8028.feature b/changelog.d/8028.feature
new file mode 100644
index 000000000000..7294224aed66
--- /dev/null
+++ b/changelog.d/8028.feature
@@ -0,0 +1 @@
+Users can now be shadow banned -- they may be told their requests succeeded while Synapse ignored them.

From 58aad73a5e4c9c711108f853bbd197704d0ce9e3 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 6 Aug 2020 10:03:26 -0400
Subject: [PATCH 05/12] Fix name of database variables.

---
 synapse/storage/databases/main/registration.py | 2 +-
 tests/rest/client/v1/test_rooms.py             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index ac62f709dda5..91472657daec 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -70,7 +70,7 @@ def get_user_by_id(self, user_id):
         )
 
     async def is_shadow_banned(self, user_id):
-        return await self.db.simple_select_one_onecol(
+        return await self.db_pool.simple_select_one_onecol(
             table="users",
             keyvalues={"name": user_id},
             retcol="shadow_banned",
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 74a967ca4cdd..7073c0084f81 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -1993,7 +1993,7 @@ def prepare(self, reactor, clock, homeserver):
         self.store = self.hs.get_datastore()
 
         self.get_success(
-            self.store.db.simple_update(
+            self.store.db_pool.simple_update(
                 table="users",
                 keyvalues={"name": self.banned_user_id},
                 updatevalues={"shadow_banned": True},

From c858898411a70a5e54751f30ae26757559957508 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Fri, 7 Aug 2020 10:45:56 -0400
Subject: [PATCH 06/12] Clarify comments and changelog.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 changelog.d/8028.feature                       | 2 +-
 synapse/handlers/register.py                   | 4 ++--
 synapse/replication/http/register.py           | 2 +-
 synapse/storage/databases/main/registration.py | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/changelog.d/8028.feature b/changelog.d/8028.feature
index 7294224aed66..813e6d0903d9 100644
--- a/changelog.d/8028.feature
+++ b/changelog.d/8028.feature
@@ -1 +1 @@
-Users can now be shadow banned -- they may be told their requests succeeded while Synapse ignored them.
+Add support for shadow-banning users (ignoring any message send requests).
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 2b8cb48be4ff..999bc6efb56a 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -160,7 +160,7 @@ async def register_user(
             bind_emails (List[str]): list of emails to bind to this account.
             by_admin (bool): True if this registration is being made via the
               admin api, otherwise False.
-            shadow_banned (bool): Shadow ban the created user.
+            shadow_banned (bool): Shadow-ban the created user.
         Returns:
             str: user_id
         Raises:
@@ -551,7 +551,7 @@ def register_with_store(
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
             address (str|None): the IP address used to perform the registration.
-            shadow_banned (bool): Whether to shadow ban the user
+            shadow_banned (bool): Whether to shadow-ban the user
 
         Returns:
             Awaitable
diff --git a/synapse/replication/http/register.py b/synapse/replication/http/register.py
index 4a8270eb1e00..a02b27474d9a 100644
--- a/synapse/replication/http/register.py
+++ b/synapse/replication/http/register.py
@@ -61,7 +61,7 @@ async def _serialize_payload(
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
             address (str|None): the IP address used to perform the regitration.
-            shadow_banned (bool): Whether to shadow ban the user
+            shadow_banned (bool): Whether to shadow-ban the user
         """
         return {
             "password_hash": password_hash,
diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index 91472657daec..d69038f77f13 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -1001,8 +1001,8 @@ def register_user(
             admin (boolean): is an admin user?
             user_type (str|None): type of user. One of the values from
                 api.constants.UserTypes, or None for a normal user.
-            shadow_banned (bool): Whether the user is shadow banned,
-                i.e. they may be told their requests succeeded but we ignored them.
+            shadow_banned (bool): Whether the user is shadow-banned,
+                i.e. they may be told their requests succeeded but we ignore them.
 
         Raises:
             StoreError if the user_id could not be registered.

From 1d2ebf84a27ce93cda2f94799037ada7d5a0a367 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 10 Aug 2020 08:07:17 -0400
Subject: [PATCH 07/12] Move super call to the top of the function and fix
 docstrings / types.

---
 synapse/api/errors.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 0a8393a9dfe1..787082c9681a 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -149,14 +149,18 @@ class ShadowBanError(SynapseError):
     """An exception that returns a 200 OK with a fake body.
     """
 
-    def __init__(self, body=None):
+    def __init__(self, body: Optional[Dict] = None):
+        """
+        Args:
+            body: The fake body to return as a successful response.
+        """
+        super().__init__(200, "")
+
         if body is None:
             body = {}
 
         self.body = body
 
-        super().__init__(200, "")
-
     def error_dict(self):
         return self.body
 

From c00207098ad005f2d8c8a59549f8b461b75e31c5 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 10 Aug 2020 09:41:47 -0400
Subject: [PATCH 08/12] Add additional comments.

---
 synapse/storage/databases/main/registration.py        | 11 ++++++++++-
 .../databases/main/schema/delta/58/09shadow_ban.sql   |  2 ++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index d69038f77f13..b9f95a39a618 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -69,7 +69,16 @@ def get_user_by_id(self, user_id):
             desc="get_user_by_id",
         )
 
-    async def is_shadow_banned(self, user_id):
+    async def is_shadow_banned(self, user_id: str) -> bool:
+        """Checks if a user is "shadow-banned", meaning that the user might
+        be told their request succeeded when it was actually ignored.
+
+        Args:
+            user_id: The user to check.
+
+        Returns:
+            True if the user is shadow-banned.
+        """
         return await self.db_pool.simple_select_one_onecol(
             table="users",
             keyvalues={"name": user_id},
diff --git a/synapse/storage/databases/main/schema/delta/58/09shadow_ban.sql b/synapse/storage/databases/main/schema/delta/58/09shadow_ban.sql
index 9592a62b38e9..260b009b4842 100644
--- a/synapse/storage/databases/main/schema/delta/58/09shadow_ban.sql
+++ b/synapse/storage/databases/main/schema/delta/58/09shadow_ban.sql
@@ -13,4 +13,6 @@
  * limitations under the License.
  */
 
+-- A shadow-banned user may be told that their requests succeeded when they were
+-- actually ignored.
 ALTER TABLE users ADD COLUMN shadow_banned BOOLEAN;

From acd63f8eebf0a98dbd23c4a05182af9bbe7323e3 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 11 Aug 2020 08:23:53 -0400
Subject: [PATCH 09/12] Move whether the user was shadow banned to the
 requester.

---
 synapse/api/auth.py                           | 12 ++++++++-
 synapse/handlers/message.py                   |  4 +--
 synapse/handlers/room.py                      |  4 +--
 synapse/handlers/room_member.py               |  8 +++---
 .../storage/databases/main/registration.py    | 20 +--------------
 synapse/types.py                              | 25 ++++++++++++++++---
 tests/api/test_auth.py                        |  1 +
 tests/storage/test_cleanup_extrems.py         |  4 +--
 tests/storage/test_event_metrics.py           |  2 +-
 tests/storage/test_roommember.py              |  2 +-
 tests/test_federation.py                      |  2 +-
 tests/unittest.py                             |  8 ++++--
 12 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 2178e623da8e..ec2446b31889 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -217,6 +217,7 @@ def get_user_by_req(
             user = user_info["user"]
             token_id = user_info["token_id"]
             is_guest = user_info["is_guest"]
+            shadow_banned = user_info["shadow_banned"]
 
             # Deny the request if the user account has expired.
             if self._account_validity.enabled and not allow_expired:
@@ -256,7 +257,12 @@ def get_user_by_req(
                 opentracing.set_tag("device_id", device_id)
 
             return synapse.types.create_requester(
-                user, token_id, is_guest, device_id, app_service=app_service
+                user,
+                token_id,
+                is_guest,
+                shadow_banned,
+                device_id,
+                app_service=app_service,
             )
         except KeyError:
             raise MissingClientTokenError()
@@ -303,6 +309,7 @@ def get_user_by_access_token(
             Deferred[dict]: dict that includes:
                `user` (UserID)
                `is_guest` (bool)
+               `shadow_banned` (bool)
                `token_id` (int|None): access token id. May be None if guest
                `device_id` (str|None): device corresponding to access token
         Raises:
@@ -362,6 +369,7 @@ def get_user_by_access_token(
                 ret = {
                     "user": user,
                     "is_guest": True,
+                    "shadow_banned": False,
                     "token_id": None,
                     # all guests get the same device id
                     "device_id": GUEST_DEVICE_ID,
@@ -371,6 +379,7 @@ def get_user_by_access_token(
                 ret = {
                     "user": user,
                     "is_guest": False,
+                    "shadow_banned": False,
                     "token_id": None,
                     "device_id": None,
                 }
@@ -495,6 +504,7 @@ def _look_up_user_by_access_token(self, token):
             "user": UserID.from_string(ret.get("name")),
             "token_id": ret.get("token_id", None),
             "is_guest": False,
+            "shadow_banned": ret.get("shadow_banned"),
             "device_id": ret.get("device_id"),
             "valid_until_ms": ret.get("valid_until_ms"),
         }
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index a3769466793a..62898ecaceea 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -721,9 +721,7 @@ async def create_and_send_nonmember_event(
         See self.create_event and self.send_nonmember_event.
         """
 
-        if not ignore_shadow_ban and await self.store.is_shadow_banned(
-            requester.user.to_string()
-        ):
+        if not ignore_shadow_ban and requester.shadow_banned:
             # We randomly sleep a bit just to annoy the requester a bit.
             await self.clock.sleep(random.randint(1, 10))
 
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 7e1a13ed4637..2d597dcd4b57 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -624,9 +624,7 @@ async def create_room(
             except Exception:
                 raise SynapseError(400, "Invalid user_id: %s" % (i,))
 
-        if (invite_list or invite_3pid_list) and await self.store.is_shadow_banned(
-            user_id
-        ):
+        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))
 
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 9bc642466bbf..68ef9e917284 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -288,9 +288,7 @@ async def update_membership(
         content: Optional[dict] = None,
         require_consent: bool = True,
     ) -> Tuple[str, int]:
-        if action == Membership.INVITE and await self.store.is_shadow_banned(
-            requester.user.to_string()
-        ):
+        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))
             # We return a response that looks roughly legit.
@@ -791,7 +789,7 @@ async def do_3pid_invite(
                     403, "Invites have been disabled on this server", Codes.FORBIDDEN
                 )
 
-        if await self.store.is_shadow_banned(requester.user.to_string()):
+        if requester.shadow_banned:
             # We randomly sleep a bit just to annoy the requester a bit.
             await self.clock.sleep(random.randint(1, 10))
 
@@ -1060,7 +1058,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/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index b9f95a39a618..201046ec6301 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -69,24 +69,6 @@ def get_user_by_id(self, user_id):
             desc="get_user_by_id",
         )
 
-    async def is_shadow_banned(self, user_id: str) -> bool:
-        """Checks if a user is "shadow-banned", meaning that the user might
-        be told their request succeeded when it was actually ignored.
-
-        Args:
-            user_id: The user to check.
-
-        Returns:
-            True if the user is shadow-banned.
-        """
-        return await self.db_pool.simple_select_one_onecol(
-            table="users",
-            keyvalues={"name": user_id},
-            retcol="shadow_banned",
-            allow_none=True,
-            desc="is_shadow_banned",
-        )
-
     @defer.inlineCallbacks
     def is_trial_user(self, user_id):
         """Checks if user is in the "trial" period, i.e. within the first
@@ -336,7 +318,7 @@ def set_server_admin_txn(txn):
 
     def _query_for_auth(self, txn, token):
         sql = (
-            "SELECT users.name, users.is_guest, access_tokens.id as token_id,"
+            "SELECT users.name, users.is_guest, users.shadow_banned, access_tokens.id as token_id,"
             " access_tokens.device_id, access_tokens.valid_until_ms"
             " FROM users"
             " INNER JOIN access_tokens on users.name = access_tokens.user_id"
diff --git a/synapse/types.py b/synapse/types.py
index 238b93806448..bd24d939d4e3 100644
--- a/synapse/types.py
+++ b/synapse/types.py
@@ -50,7 +50,15 @@ class Collection(Iterable[T_co], Container[T_co], Sized):
 
 class Requester(
     namedtuple(
-        "Requester", ["user", "access_token_id", "is_guest", "device_id", "app_service"]
+        "Requester",
+        [
+            "user",
+            "access_token_id",
+            "is_guest",
+            "shadow_banned",
+            "device_id",
+            "app_service",
+        ],
     )
 ):
     """
@@ -61,6 +69,7 @@ class Requester(
         access_token_id (int|None):  *ID* of the access token used for this
             request, or None if it came via the appservice API or similar
         is_guest (bool):  True if the user making this request is a guest user
+        shadow_banned (bool):  True if the user making this request has been shadow-banned.
         device_id (str|None):  device_id which was set at authentication time
         app_service (ApplicationService|None):  the AS requesting on behalf of the user
     """
@@ -76,6 +85,7 @@ def serialize(self):
             "user_id": self.user.to_string(),
             "access_token_id": self.access_token_id,
             "is_guest": self.is_guest,
+            "shadow_banned": self.shadow_banned,
             "device_id": self.device_id,
             "app_server_id": self.app_service.id if self.app_service else None,
         }
@@ -100,13 +110,19 @@ def deserialize(store, input):
             user=UserID.from_string(input["user_id"]),
             access_token_id=input["access_token_id"],
             is_guest=input["is_guest"],
+            shadow_banned=input["shadow_banned"],
             device_id=input["device_id"],
             app_service=appservice,
         )
 
 
 def create_requester(
-    user_id, access_token_id=None, is_guest=False, device_id=None, app_service=None
+    user_id,
+    access_token_id=None,
+    is_guest=False,
+    shadow_banned=False,
+    device_id=None,
+    app_service=None,
 ):
     """
     Create a new ``Requester`` object
@@ -116,6 +132,7 @@ def create_requester(
         access_token_id (int|None):  *ID* of the access token used for this
             request, or None if it came via the appservice API or similar
         is_guest (bool):  True if the user making this request is a guest user
+        shadow_banned (bool):  True if the user making this request is shadow-banned.
         device_id (str|None):  device_id which was set at authentication time
         app_service (ApplicationService|None):  the AS requesting on behalf of the user
 
@@ -124,7 +141,9 @@ def create_requester(
     """
     if not isinstance(user_id, UserID):
         user_id = UserID.from_string(user_id)
-    return Requester(user_id, access_token_id, is_guest, device_id, app_service)
+    return Requester(
+        user_id, access_token_id, is_guest, shadow_banned, device_id, app_service
+    )
 
 
 def get_domain_from_id(string):
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index 0bfb86bf1f6d..1fc21ead5192 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -286,6 +286,7 @@ def get_user(tok):
             return {
                 "name": USER_ID,
                 "is_guest": False,
+                "shadow_banned": False,
                 "token_id": 1234,
                 "device_id": "DEVICE",
             }
diff --git a/tests/storage/test_cleanup_extrems.py b/tests/storage/test_cleanup_extrems.py
index 3fab5a524829..8e9a650f9fc2 100644
--- a/tests/storage/test_cleanup_extrems.py
+++ b/tests/storage/test_cleanup_extrems.py
@@ -38,7 +38,7 @@ def prepare(self, reactor, clock, homeserver):
 
         # Create a test user and room
         self.user = UserID("alice", "test")
-        self.requester = Requester(self.user, None, False, None, None)
+        self.requester = Requester(self.user, None, False, False, None, None)
         info, _ = self.get_success(self.room_creator.create_room(self.requester, {}))
         self.room_id = info["room_id"]
 
@@ -260,7 +260,7 @@ def prepare(self, reactor, clock, homeserver):
         # Create a test user and room
         self.user = UserID.from_string(self.register_user("user1", "password"))
         self.token1 = self.login("user1", "password")
-        self.requester = Requester(self.user, None, False, None, None)
+        self.requester = Requester(self.user, None, False, False, None, None)
         info, _ = self.get_success(self.room_creator.create_room(self.requester, {}))
         self.room_id = info["room_id"]
         self.event_creator = homeserver.get_event_creation_handler()
diff --git a/tests/storage/test_event_metrics.py b/tests/storage/test_event_metrics.py
index a7b85004e5d3..949846fe33de 100644
--- a/tests/storage/test_event_metrics.py
+++ b/tests/storage/test_event_metrics.py
@@ -27,7 +27,7 @@ def test_exposed_to_prometheus(self):
         room_creator = self.hs.get_room_creation_handler()
 
         user = UserID("alice", "test")
-        requester = Requester(user, None, False, None, None)
+        requester = Requester(user, None, False, False, None, None)
 
         # Real events, forward extremities
         events = [(3, 2), (6, 2), (4, 6)]
diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py
index 17c9da483867..d98fe8754dab 100644
--- a/tests/storage/test_roommember.py
+++ b/tests/storage/test_roommember.py
@@ -187,7 +187,7 @@ def test_can_rerun_update(self):
 
         # Now let's create a room, which will insert a membership
         user = UserID("alice", "test")
-        requester = Requester(user, None, False, None, None)
+        requester = Requester(user, None, False, False, None, None)
         self.get_success(self.room_creator.create_room(requester, {}))
 
         # Register the background update to run again.
diff --git a/tests/test_federation.py b/tests/test_federation.py
index c2f12c2741e3..3f1b13daca4d 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -26,7 +26,7 @@ def setUp(self):
         )
 
         user_id = UserID("us", "test")
-        our_user = Requester(user_id, None, False, None, None)
+        our_user = Requester(user_id, None, False, False, None, None)
         room_creator = self.homeserver.get_room_creation_handler()
         room_deferred = ensureDeferred(
             room_creator.create_room(
diff --git a/tests/unittest.py b/tests/unittest.py
index 2152c693f2e3..6ae16d9fed49 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -253,7 +253,11 @@ def get_user_by_access_token(token=None, allow_guest=False):
                 def get_user_by_req(request, allow_guest=False, rights="access"):
                     return succeed(
                         create_requester(
-                            UserID.from_string(self.helper.auth_user_id), 1, False, None
+                            UserID.from_string(self.helper.auth_user_id),
+                            1,
+                            False,
+                            False,
+                            None,
                         )
                     )
 
@@ -544,7 +548,7 @@ def create_and_send_event(
         """
         event_creator = self.hs.get_event_creation_handler()
         secrets = self.hs.get_secrets()
-        requester = Requester(user, None, False, None, None)
+        requester = Requester(user, None, False, False, None, None)
 
         event, context = self.get_success(
             event_creator.create_event(

From 739be9e9b5d5814f09ea3931881302a58b1a68b8 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 11 Aug 2020 12:59:33 -0400
Subject: [PATCH 10/12] Create fake responses closer to the real responses.

---
 synapse/api/errors.py                         |  20 +--
 synapse/handlers/directory.py                 |   2 +
 synapse/handlers/message.py                   |   8 +-
 synapse/handlers/profile.py                   |  32 ++++-
 synapse/handlers/room.py                      |  27 +++-
 synapse/handlers/room_member.py               |  45 +++++-
 synapse/rest/admin/rooms.py                   |   4 +-
 synapse/rest/client/v1/directory.py           |   7 +-
 synapse/rest/client/v1/profile.py             |  20 ++-
 synapse/rest/client/v1/room.py                | 129 ++++++++++--------
 synapse/rest/client/v2_alpha/relations.py     |  15 +-
 .../v2_alpha/room_upgrade_rest_servlet.py     |  13 +-
 12 files changed, 222 insertions(+), 100 deletions(-)

diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 787082c9681a..60c1bcd29839 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -145,24 +145,12 @@ def error_dict(self):
         return cs_error(self.msg, self.errcode)
 
 
-class ShadowBanError(SynapseError):
-    """An exception that returns a 200 OK with a fake body.
+class ShadowBanError(Exception):
     """
+    Raised when a shadow-banned user attempts to perform an action.
 
-    def __init__(self, body: Optional[Dict] = None):
-        """
-        Args:
-            body: The fake body to return as a successful response.
-        """
-        super().__init__(200, "")
-
-        if body is None:
-            body = {}
-
-        self.body = body
-
-    def error_dict(self):
-        return self.body
+    This should be caught and a proper "fake" success response send to the user.
+    """
 
 
 class ProxiedRequestError(SynapseError):
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 79a2df62015a..0a0c29e46506 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -174,6 +174,8 @@ async def delete_association(
                 is neither the creator of the alias, nor a server admin.
 
             SynapseError: if the alias belongs to an AS
+
+            ShadowBanError: if the requester is shadow-banned.
         """
         user_id = requester.user.to_string()
 
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 62898ecaceea..374e0716de7a 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -60,7 +60,6 @@
 from synapse.util.async_helpers import Linearizer
 from synapse.util.frozenutils import frozendict_json_encoder
 from synapse.util.metrics import measure_func
-from synapse.util.stringutils import random_string
 from synapse.visibility import filter_events_for_client
 
 from ._base import BaseHandler
@@ -719,14 +718,15 @@ async def create_and_send_nonmember_event(
         Creates an event, then sends it.
 
         See self.create_event and self.send_nonmember_event.
+
+        Raises:
+            ShadowBanError if the requester has been shadow-banned.
         """
 
         if not ignore_shadow_ban and requester.shadow_banned:
             # We randomly sleep a bit just to annoy the requester a bit.
             await self.clock.sleep(random.randint(1, 10))
-
-            # We return a response that looks roughly legit.
-            raise ShadowBanError({"event_id": "$" + random_string(43)})
+            raise ShadowBanError()
 
         # We limit the number of concurrent event sends in a room so that we
         # don't fork the DAG too much. If we don't limit then we can end up in
diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index 31a2e5ea1871..a6f5438a333c 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -14,12 +14,14 @@
 # limitations under the License.
 
 import logging
+import random
 
 from synapse.api.errors import (
     AuthError,
     Codes,
     HttpResponseException,
     RequestSendFailed,
+    ShadowBanError,
     StoreError,
     SynapseError,
 )
@@ -144,6 +146,9 @@ async def set_displayname(
             requester (Requester): The user attempting to make this change.
             new_displayname (str): The displayname to give this user.
             by_admin (bool): Whether this change was made by an administrator.
+
+        Raises:
+            ShadowBanError if the requester is shadow-banned.
         """
         if not self.hs.is_mine(target_user):
             raise SynapseError(400, "User is not hosted on this homeserver")
@@ -168,6 +173,13 @@ async def set_displayname(
         if new_displayname == "":
             new_displayname = None
 
+        # Since the requester can get overwritten below, check if the real
+        # requester is shadow-banned.
+        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()
+
         # If the admin changes the display name of a user, the requesting user cannot send
         # the join event to update the displayname in the rooms.
         # This must be done by the target user himself.
@@ -213,8 +225,17 @@ async def get_avatar_url(self, target_user):
     async def set_avatar_url(
         self, target_user, requester, new_avatar_url, by_admin=False
     ):
-        """target_user is the user whose avatar_url is to be changed;
-        auth_user is the user attempting to make this change."""
+        """Set a new avatar URL for a user.
+
+        Args:
+            target_user (UserID): the user whose displayname is to be changed.
+            requester (Requester): The user attempting to make this change.
+            new_displayname (str): The displayname to give this user.
+            by_admin (bool): Whether this change was made by an administrator.
+
+        Raises:
+            ShadowBanError if the requester is shadow-banned.
+        """
         if not self.hs.is_mine(target_user):
             raise SynapseError(400, "User is not hosted on this homeserver")
 
@@ -233,6 +254,13 @@ async def set_avatar_url(
                 400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN,)
             )
 
+        # Since the requester can get overwritten below, check if the real
+        # requester is shadow-banned.
+        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()
+
         # Same like set_displayname
         if by_admin:
             requester = create_requester(target_user)
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 2d597dcd4b57..f8f25f777291 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -130,6 +130,9 @@ async def upgrade_room(
 
         Returns:
             the new room id
+
+        Raises:
+            ShadowBanError if the requester is shadow-banned.
         """
         await self.ratelimit(requester)
 
@@ -248,6 +251,9 @@ async def _update_upgraded_room_pls(
             old_room_id: the id of the room to be replaced
             new_room_id: the id of the replacement room
             old_room_state: the state map for the old room
+
+        Raises:
+            ShadowBanError if the requester is shadow-banned.
         """
         old_room_pl_event_id = old_room_state.get((EventTypes.PowerLevels, ""))
 
@@ -1117,7 +1123,7 @@ def __init__(self, hs):
     async def shutdown_room(
         self,
         room_id: str,
-        requester_user_id: str,
+        requester: Requester,
         new_room_user_id: Optional[str] = None,
         new_room_name: Optional[str] = None,
         message: Optional[str] = None,
@@ -1138,7 +1144,7 @@ async def shutdown_room(
 
         Args:
             room_id: The ID of the room to shut down.
-            requester_user_id:
+            requester:
                 User who requested the action and put the room on the
                 blocking list.
             new_room_user_id:
@@ -1180,8 +1186,25 @@ async def shutdown_room(
         if not await self.store.get_room(room_id):
             raise NotFoundError("Unknown room id %s" % (room_id,))
 
+        # Check if the user is shadow-banned before any mutations occur.
+        if requester.shadow_banned:
+            # We randomly sleep a bit just to annoy the requester a bit.
+            await self.clock.sleep(random.randint(1, 10))
+
+            # Since this usually returns the response sent to the client,
+            # assemble a fake response instead.
+            return {
+                "kicked_users": [],
+                "failed_to_kick_users": [],
+                "local_aliases": [],
+                "new_room_id": stringutils.random_string(18)
+                if new_room_user_id
+                else None,
+            }
+
         # This will work even if the room is already blocked, but that is
         # desirable in case the first attempt at blocking the room failed below.
+        requester_user_id = requester.user.to_string()
         if block:
             await self.store.block_room(room_id, requester_user_id)
 
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 68ef9e917284..aefd743195f8 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -288,11 +288,30 @@ 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))
-            # We return a response that looks roughly legit.
-            raise ShadowBanError({})
+            raise ShadowBanError()
 
         key = (room_id,)
 
@@ -782,6 +801,24 @@ 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:
@@ -792,9 +829,7 @@ async def do_3pid_invite(
         if requester.shadow_banned:
             # We randomly sleep a bit just to annoy the requester a bit.
             await self.clock.sleep(random.randint(1, 10))
-
-            # We return a response that looks roughly legit.
-            raise ShadowBanError({})
+            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.
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 7c292ef3f939..0c2d53ce8409 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -60,10 +60,10 @@ async def on_POST(self, request, room_id):
 
         ret = await self.room_shutdown_handler.shutdown_room(
             room_id=room_id,
+            requester=requester,
             new_room_user_id=content["new_room_user_id"],
             new_room_name=content.get("room_name"),
             message=content.get("message"),
-            requester_user_id=requester.user.to_string(),
             block=True,
         )
 
@@ -113,10 +113,10 @@ async def on_POST(self, request, room_id):
 
         ret = await self.room_shutdown_handler.shutdown_room(
             room_id=room_id,
+            requester=requester,
             new_room_user_id=content.get("new_room_user_id"),
             new_room_name=content.get("room_name"),
             message=content.get("message"),
-            requester_user_id=requester.user.to_string(),
             block=block,
         )
 
diff --git a/synapse/rest/client/v1/directory.py b/synapse/rest/client/v1/directory.py
index 5934b1fe8bdc..2a55bb86f1c2 100644
--- a/synapse/rest/client/v1/directory.py
+++ b/synapse/rest/client/v1/directory.py
@@ -21,6 +21,7 @@
     Codes,
     InvalidClientCredentialsError,
     NotFoundError,
+    ShadowBanError,
     SynapseError,
 )
 from synapse.http.servlet import RestServlet, parse_json_object_from_request
@@ -107,7 +108,11 @@ async def on_DELETE(self, request, room_alias):
 
         room_alias = RoomAlias.from_string(room_alias)
 
-        await dir_handler.delete_association(requester, room_alias)
+        try:
+            await dir_handler.delete_association(requester, room_alias)
+        except ShadowBanError:
+            # Pretend the request was successful.
+            pass
 
         logger.info(
             "User %s deleted alias %s", user.to_string(), room_alias.to_string()
diff --git a/synapse/rest/client/v1/profile.py b/synapse/rest/client/v1/profile.py
index e7fe50ed72c4..96f87017bf76 100644
--- a/synapse/rest/client/v1/profile.py
+++ b/synapse/rest/client/v1/profile.py
@@ -15,7 +15,7 @@
 
 """ This module contains REST servlets to do with profile: /profile/<paths> """
 
-from synapse.api.errors import Codes, SynapseError
+from synapse.api.errors import Codes, ShadowBanError, SynapseError
 from synapse.http.servlet import RestServlet, parse_json_object_from_request
 from synapse.rest.client.v2_alpha._base import client_patterns
 from synapse.types import UserID
@@ -61,7 +61,13 @@ async def on_PUT(self, request, user_id):
         except Exception:
             return 400, "Unable to parse name"
 
-        await self.profile_handler.set_displayname(user, requester, new_name, is_admin)
+        try:
+            await self.profile_handler.set_displayname(
+                user, requester, new_name, is_admin
+            )
+        except ShadowBanError:
+            # Pretend it was modified successfully.
+            pass
 
         return 200, {}
 
@@ -110,9 +116,13 @@ async def on_PUT(self, request, user_id):
                 400, "Missing key 'avatar_url'", errcode=Codes.MISSING_PARAM
             )
 
-        await self.profile_handler.set_avatar_url(
-            user, requester, new_avatar_url, is_admin
-        )
+        try:
+            await self.profile_handler.set_avatar_url(
+                user, requester, new_avatar_url, is_admin
+            )
+        except ShadowBanError:
+            # Pretend it was modified successfully.
+            pass
 
         return 200, {}
 
diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 2ab30ce8977d..3692b2fd5d39 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:
@@ -200,23 +202,26 @@ async def on_PUT(self, request, room_id, event_type, state_key, txn_id=None):
         if state_key is not 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,
-            )
-        else:
-            (
-                event,
-                _,
-            ) = await self.event_creation_handler.create_and_send_nonmember_event(
-                requester, event_dict, txn_id=txn_id
-            )
-            event_id = event.event_id
+        try:
+            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,
+                )
+            else:
+                (
+                    event,
+                    _,
+                ) = await self.event_creation_handler.create_and_send_nonmember_event(
+                    requester, event_dict, txn_id=txn_id
+                )
+                event_id = event.event_id
+        except ShadowBanError:
+            event_id = "$" + random_string(43)
 
         set_tag("event_id", event_id)
         ret = {"event_id": event_id}
@@ -249,12 +254,16 @@ async def on_POST(self, request, room_id, event_type, txn_id=None):
         if b"ts" in request.args and requester.app_service:
             event_dict["origin_server_ts"] = parse_integer(request, "ts", 0)
 
-        event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
-            requester, event_dict, txn_id=txn_id
-        )
+        try:
+            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+                requester, event_dict, txn_id=txn_id
+            )
+            event_id = event.event_id
+        except ShadowBanError:
+            event_id = "$" + random_string(43)
 
-        set_tag("event_id", event.event_id)
-        return 200, {"event_id": event.event_id}
+        set_tag("event_id", event_id)
+        return 200, {"event_id": event_id}
 
     def on_GET(self, request, room_id, event_type, txn_id):
         return 200, "Not implemented"
@@ -716,16 +725,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 +750,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 = {}
 
@@ -783,20 +800,24 @@ async def on_POST(self, request, room_id, event_id, txn_id=None):
         requester = await self.auth.get_user_by_req(request)
         content = parse_json_object_from_request(request)
 
-        event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
-            requester,
-            {
-                "type": EventTypes.Redaction,
-                "content": content,
-                "room_id": room_id,
-                "sender": requester.user.to_string(),
-                "redacts": event_id,
-            },
-            txn_id=txn_id,
-        )
+        try:
+            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+                requester,
+                {
+                    "type": EventTypes.Redaction,
+                    "content": content,
+                    "room_id": room_id,
+                    "sender": requester.user.to_string(),
+                    "redacts": event_id,
+                },
+                txn_id=txn_id,
+            )
+            event_id = event.event_id
+        except ShadowBanError:
+            event_id = "$" + random_string(43)
 
-        set_tag("event_id", event.event_id)
-        return 200, {"event_id": event.event_id}
+        set_tag("event_id", event_id)
+        return 200, {"event_id": event_id}
 
     def on_PUT(self, request, room_id, event_id, txn_id):
         set_tag("txn_id", txn_id)
diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py
index 89002ffbffdd..7cd7d29b4b84 100644
--- a/synapse/rest/client/v2_alpha/relations.py
+++ b/synapse/rest/client/v2_alpha/relations.py
@@ -22,7 +22,7 @@
 import logging
 
 from synapse.api.constants import EventTypes, RelationTypes
-from synapse.api.errors import SynapseError
+from synapse.api.errors import ShadowBanError, SynapseError
 from synapse.http.servlet import (
     RestServlet,
     parse_integer,
@@ -35,6 +35,7 @@
     PaginationChunk,
     RelationPaginationToken,
 )
+from synapse.util.stringutils import random_string
 
 from ._base import client_patterns
 
@@ -111,11 +112,15 @@ async def on_PUT_or_POST(
             "sender": requester.user.to_string(),
         }
 
-        event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
-            requester, event_dict=event_dict, txn_id=txn_id
-        )
+        try:
+            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+                requester, event_dict=event_dict, txn_id=txn_id
+            )
+            event_id = event.event_id
+        except ShadowBanError:
+            event_id = "$" + random_string(43)
 
-        return 200, {"event_id": event.event_id}
+        return 200, {"event_id": event_id}
 
 
 class RelationPaginationServlet(RestServlet):
diff --git a/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py b/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
index f357015a7001..b4e48755905f 100644
--- a/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
+++ b/synapse/rest/client/v2_alpha/room_upgrade_rest_servlet.py
@@ -15,13 +15,14 @@
 
 import logging
 
-from synapse.api.errors import Codes, SynapseError
+from synapse.api.errors import Codes, ShadowBanError, SynapseError
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
 from synapse.http.servlet import (
     RestServlet,
     assert_params_in_dict,
     parse_json_object_from_request,
 )
+from synapse.util import stringutils
 
 from ._base import client_patterns
 
@@ -72,9 +73,13 @@ async def on_POST(self, request, room_id):
                 Codes.UNSUPPORTED_ROOM_VERSION,
             )
 
-        new_room_id = await self._room_creation_handler.upgrade_room(
-            requester, room_id, new_version
-        )
+        try:
+            new_room_id = await self._room_creation_handler.upgrade_room(
+                requester, room_id, new_version
+            )
+        except ShadowBanError:
+            # Generate a random room ID.
+            new_room_id = stringutils.random_string(18)
 
         ret = {"replacement_room": new_room_id}
 

From 505fde29fba24da9a7e73bc7530dc578f286f756 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 11 Aug 2020 13:37:53 -0400
Subject: [PATCH 11/12] Lint.

---
 synapse/rest/client/v1/room.py            | 10 ++++++++--
 synapse/rest/client/v2_alpha/relations.py |  5 ++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py
index 3692b2fd5d39..1ee4dbbe3189 100644
--- a/synapse/rest/client/v1/room.py
+++ b/synapse/rest/client/v1/room.py
@@ -255,7 +255,10 @@ async def on_POST(self, request, room_id, event_type, txn_id=None):
             event_dict["origin_server_ts"] = parse_integer(request, "ts", 0)
 
         try:
-            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+            (
+                event,
+                _,
+            ) = await self.event_creation_handler.create_and_send_nonmember_event(
                 requester, event_dict, txn_id=txn_id
             )
             event_id = event.event_id
@@ -801,7 +804,10 @@ async def on_POST(self, request, room_id, event_id, txn_id=None):
         content = parse_json_object_from_request(request)
 
         try:
-            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+            (
+                event,
+                _,
+            ) = await self.event_creation_handler.create_and_send_nonmember_event(
                 requester,
                 {
                     "type": EventTypes.Redaction,
diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py
index 7cd7d29b4b84..e29f49f7f57d 100644
--- a/synapse/rest/client/v2_alpha/relations.py
+++ b/synapse/rest/client/v2_alpha/relations.py
@@ -113,7 +113,10 @@ async def on_PUT_or_POST(
         }
 
         try:
-            event, _ = await self.event_creation_handler.create_and_send_nonmember_event(
+            (
+                event,
+                _,
+            ) = await self.event_creation_handler.create_and_send_nonmember_event(
                 requester, event_dict=event_dict, txn_id=txn_id
             )
             event_id = event.event_id

From 5adcc56c8f9d6d38657c21f3634f143c2b29b004 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 11 Aug 2020 14:16:36 -0400
Subject: [PATCH 12/12] Expand comments and add a test.

---
 synapse/handlers/room.py           |  4 ++++
 tests/rest/client/v1/test_rooms.py | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index f8f25f777291..b70c1f232277 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -813,6 +813,8 @@ def create(etype, content, **kwargs):
         async def send(etype, content, **kwargs) -> int:
             event = create(etype, content, **kwargs)
             logger.debug("Sending %s in new room", etype)
+            # Ignore whether the user is shadow-banned to allow the room
+            # creation to complete.
             (
                 _,
                 last_stream_id,
@@ -831,6 +833,8 @@ async def send(etype, content, **kwargs) -> int:
         await send(etype=EventTypes.Create, content=creation_content)
 
         logger.debug("Sending %s in new room", EventTypes.Member)
+        # Shadow-banning won't interfere with the join, so this should complete
+        # fine.
         await self.room_member_handler.update_membership(
             creator,
             creator.user,
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index f399e7a38770..51e3d9fa9c40 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -2048,3 +2048,17 @@ def test_message(self):
             self.store.get_latest_event_ids_in_room(room_id)
         )
         self.assertNotIn(event_id, latest_events)
+
+    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"])