Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow marking users as shadow banned #8028

Closed
wants to merge 14 commits into from
1 change: 1 addition & 0 deletions changelog.d/8028.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for shadow-banning users (ignoring any message send requests).
12 changes: 11 additions & 1 deletion synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ async 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:
Expand Down Expand Up @@ -252,7 +253,12 @@ async 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()
Expand Down Expand Up @@ -297,6 +303,7 @@ async def get_user_by_access_token(
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:
Expand Down Expand Up @@ -356,6 +363,7 @@ async 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,
Expand All @@ -365,6 +373,7 @@ async def get_user_by_access_token(
ret = {
"user": user,
"is_guest": False,
"shadow_banned": False,
"token_id": None,
"device_id": None,
}
Expand Down Expand Up @@ -488,6 +497,7 @@ async 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"),
}
Expand Down
8 changes: 8 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ def error_dict(self):
return cs_error(self.msg, self.errcode)


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.
"""


class ProxiedRequestError(SynapseError):
"""An error from a general matrix endpoint, eg. from a proxied Matrix API call.

Expand Down
2 changes: 2 additions & 0 deletions synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +177 to +178
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this coming from _update_canonical_alias? should maybe just ignore the exception if so, as we do with AuthError.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also: would be nice to stick the Raises docstring on _update_canonical_alias, even though it's internal.

"""
user_id = requester.user.to_string()

Expand Down
11 changes: 11 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,6 +35,7 @@
Codes,
ConsentNotGivenError,
NotFoundError,
ShadowBanError,
SynapseError,
)
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
Expand Down Expand Up @@ -710,13 +712,22 @@ 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.

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))
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
# a situation where event persistence can't keep up, causing
Expand Down
32 changes: 30 additions & 2 deletions synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
# limitations under the License.

import logging
import random

from synapse.api.errors import (
AuthError,
Codes,
HttpResponseException,
RequestSendFailed,
ShadowBanError,
StoreError,
SynapseError,
)
Expand Down Expand Up @@ -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")
Expand All @@ -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()

Comment on lines +176 to +182
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is necessary. Shouldn't we let shadow-banned users change their displaynames if they want? It won't make it into any rooms thanks to the check in update_membership.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm....I was under the impression it would still end up in the room, but I think you're right. I'll need to test this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the changes to update_membership won't catch this because those only apply to invite requests.

# 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.
Expand Down Expand Up @@ -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")

Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -529,6 +533,7 @@ def register_with_store(
admin=False,
user_type=None,
address=None,
shadow_banned=False,
):
"""Register user in the datastore.

Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand Down
46 changes: 41 additions & 5 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import itertools
import logging
import math
import random
import string
from collections import OrderedDict
from typing import Awaitable, Optional, Tuple
Expand Down Expand Up @@ -129,6 +130,9 @@ async def upgrade_room(

Returns:
the new room id

Raises:
ShadowBanError if the requester is shadow-banned.
Comment on lines +133 to +135
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again: what is the code path by which this happens? Is it just the final PL send? we should maybe ignore the exception there rather than allow it to propagate? or bail out sooner?

"""
await self.ratelimit(requester)

Expand Down Expand Up @@ -247,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, ""))

Expand Down Expand Up @@ -614,6 +621,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:
Expand All @@ -622,6 +630,15 @@ 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))

# 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")
Expand All @@ -636,8 +653,6 @@ async def create_room(
% (user_id,),
)

invite_3pid_list = config.get("invite_3pid", [])

visibility = config.get("visibility", None)
is_public = visibility == "public"

Expand Down Expand Up @@ -798,11 +813,13 @@ 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,
) = await self.event_creation_handler.create_and_send_nonmember_event(
creator, event, ratelimit=False
creator, event, ratelimit=False, ignore_shadow_ban=True,
clokep marked this conversation as resolved.
Show resolved Hide resolved
)
return last_stream_id

Expand All @@ -816,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,
Expand Down Expand Up @@ -1108,7 +1127,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,
Expand All @@ -1129,7 +1148,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:
Expand Down Expand Up @@ -1171,8 +1190,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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given this can only be called by an admin user, I think it might be a bit redundant to check the shadow-ban.

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)

Expand Down
Loading