Skip to content

Add support for MSC3823 - Account Suspension Part 2 #17255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 24, 2024
Merged

Add support for MSC3823 - Account Suspension Part 2 #17255

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
060ba63
prevent suspended users from sending messages, changing profile data …
H-Shay May 23, 2024
245e28f
add admin API to suspend users + add experimental config option
H-Shay May 31, 2024
07bf069
tests
H-Shay May 31, 2024
fe4edc1
newsfragment
H-Shay May 31, 2024
9b56bf8
Merge branch 'develop' into shay/account_suspension_pt_2
H-Shay May 31, 2024
7975a81
lint
H-Shay May 31, 2024
4b4180b
add pydantic model + comment
H-Shay Jun 6, 2024
2fe7d28
Merge branch 'develop' into shay/account_suspension_pt_2
H-Shay Jun 21, 2024
f93d261
fix import changed by merge
H-Shay Jun 21, 2024
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
11 changes: 11 additions & 0 deletions synapse/handlers/message.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,17 @@ async def create_event(
"""
await self.auth_blocking.check_auth_blocking(requester=requester)

if event_dict["type"] == EventTypes.Message:
requester_suspended = await self.store.get_user_suspended_status(
requester.user.to_string()
)
if requester_suspended:
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to only apply suspensions if the user isn't being puppeted? (I think there are things on requester that would help to check for that. Maybe a helper would be useful to ensure we're consistent about it everywhere)

Just thinking that server admins may still wish to e.g. change the displayname of a suspended user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know much about puppeted accounts - Can a user access an account that is being puppeted?

Copy link
Contributor

Choose a reason for hiding this comment

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

'Puppeted' here only means that a server admin logged in to the user using the Admin API. https://element-hq.github.io/synapse/latest/admin_api/user_admin_api.html?highlight=login%20as%20user#login-as-a-user

Whilst it's the puppeted user that is being made to send the events and stuff, it's a trusted admin who is actually pulling the strings. So it seems logical that there'd be no need to apply any suspensions there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Requester.authenticated_entity is what you want to read to see if a user is being puppeted or not. There should be quite a bit of example code around if it's not clear from the documentation on that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right I checked in with the team during our sync today and posed the question about puppeting - the consensus is we'd like to merge it as is, and possibly follow up with that change in the future depending on the feedback from the original work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that can probably wait until later, it shouldn't break anyone since you have to choose to use the experimental feature first.

raise SynapseError(
403,
"Sending messages while account is suspended is not allowed.",
Codes.USER_ACCOUNT_SUSPENDED,
)

if event_dict["type"] == EventTypes.Create and event_dict["state_key"] == "":
room_version_id = event_dict["content"]["room_version"]
maybe_room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version_id)
Expand Down
26 changes: 26 additions & 0 deletions synapse/rest/client/profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ async def on_PUT(

propagate = _read_propagate(self.hs, request)

requester_suspended = (
await self.hs.get_datastores().main.get_user_suspended_status(
requester.user.to_string()
)
)

if requester_suspended:
raise SynapseError(
403,
"Updating displayname while account is suspended is not allowed.",
Codes.USER_ACCOUNT_SUSPENDED,
)

await self.profile_handler.set_displayname(
user, requester, new_name, is_admin, propagate=propagate
)
Expand Down Expand Up @@ -167,6 +180,19 @@ async def on_PUT(

propagate = _read_propagate(self.hs, request)

requester_suspended = (
await self.hs.get_datastores().main.get_user_suspended_status(
requester.user.to_string()
)
)

if requester_suspended:
raise SynapseError(
403,
"Updating avatar URL while account is suspended is not allowed.",
Codes.USER_ACCOUNT_SUSPENDED,
)

await self.profile_handler.set_avatar_url(
user, requester, new_avatar_url, is_admin, propagate=propagate
)
Expand Down
14 changes: 14 additions & 0 deletions synapse/rest/client/room.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,20 @@ async def _do(
) -> Tuple[int, JsonDict]:
content = parse_json_object_from_request(request)

requester_suspended = await self._store.get_user_suspended_status(
requester.user.to_string()
)

if requester_suspended:
event = await self._store.get_event(event_id, allow_none=True)
if event:
if event.sender != requester.user.to_string():
raise SynapseError(
403,
"Only events created by the requester may be redacted while account is suspended.",
Codes.USER_ACCOUNT_SUSPENDED,
)

# Ensure the redacts property in the content matches the one provided in
# the URL.
room_version = await self._store.get_room_version(room_id)
Expand Down