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

Add a shadow-banned flag to users #8092

Merged
merged 3 commits into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8092.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/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
4 changes: 4 additions & 0 deletions synapse/replication/http/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ async def _serialize_payload(
admin,
user_type,
address,
shadow_banned,
):
"""
Args:
Expand All @@ -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 shadow-ban the user
"""
return {
"password_hash": password_hash,
Expand All @@ -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):
Expand All @@ -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, {}
Expand Down
9 changes: 8 additions & 1 deletion synapse/storage/databases/main/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,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"
Expand Down Expand Up @@ -952,6 +952,7 @@ def register_user(
create_profile_with_displayname=None,
admin=False,
user_type=None,
shadow_banned=False,
):
"""Attempts to register an account.

Expand All @@ -968,6 +969,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 ignore them.

Raises:
StoreError if the user_id could not be registered.
Expand All @@ -986,6 +989,7 @@ def register_user(
create_profile_with_displayname,
admin,
user_type,
shadow_banned,
)

def _register_user(
Expand All @@ -999,6 +1003,7 @@ def _register_user(
create_profile_with_displayname,
admin,
user_type,
shadow_banned,
):
user_id_obj = UserID.from_string(user_id)

Expand Down Expand Up @@ -1028,6 +1033,7 @@ def _register_user(
"appservice_id": appservice_id,
"admin": 1 if admin else 0,
"user_type": user_type,
"shadow_banned": shadow_banned,
},
)
else:
Expand All @@ -1042,6 +1048,7 @@ def _register_user(
"appservice_id": appservice_id,
"admin": 1 if admin else 0,
"user_type": user_type,
"shadow_banned": shadow_banned,
},
)

Expand Down
18 changes: 18 additions & 0 deletions synapse/storage/databases/main/schema/delta/58/09shadow_ban.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* 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.
*/

-- A shadow-banned user may be told that their requests succeeded when they were
-- actually ignored.
ALTER TABLE users ADD COLUMN shadow_banned BOOLEAN;
25 changes: 22 additions & 3 deletions synapse/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,15 @@ class Collection(Iterable[T_co], Container[T_co], Sized): # type: ignore

class Requester(
Copy link
Member

Choose a reason for hiding this comment

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

tempted to say that we should take the opportunity to convert this to an @attr.s, but I guess we shouldn't scope-creep this more than it has already.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would be a good change, I think we should do that with a bunch of the namedtuples we have throughout the code-base though. Seems like a good refactoring to start after the async/await work.

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",
],
)
):
"""
Expand All @@ -62,6 +70,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
"""
Expand All @@ -77,6 +86,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,
}
Expand All @@ -101,13 +111,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
Expand All @@ -117,6 +133,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

Expand All @@ -125,7 +142,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):
Expand Down
4 changes: 2 additions & 2 deletions tests/storage/test_cleanup_extrems.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test_event_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test_roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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(
Expand Down
8 changes: 6 additions & 2 deletions tests/unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,11 @@ async def get_user_by_access_token(token=None, allow_guest=False):

async def get_user_by_req(request, allow_guest=False, rights="access"):
return 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,
)

self.hs.get_auth().get_user_by_req = get_user_by_req
Expand Down Expand Up @@ -540,7 +544,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(
Expand Down