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

Commit

Permalink
Remove user's avatar URL and displayname when deactivated. (#8932)
Browse files Browse the repository at this point in the history
This only applies if the user's data is to be erased.
  • Loading branch information
dklimpel authored Jan 12, 2021
1 parent 789d9eb commit 7a2e9b5
Show file tree
Hide file tree
Showing 13 changed files with 351 additions and 17 deletions.
1 change: 1 addition & 0 deletions changelog.d/8932.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove a user's avatar URL and display name when deactivated with the Admin API.
21 changes: 21 additions & 0 deletions docs/admin_api/user_admin_api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ Body parameters:

- ``deactivated``, optional. If unspecified, deactivation state will be left
unchanged on existing accounts and set to ``false`` for new accounts.
A user cannot be erased by deactivating with this API. For details on deactivating users see
`Deactivate Account <#deactivate-account>`_.

If the user already exists then optional parameters default to the current value.

Expand Down Expand Up @@ -248,6 +250,25 @@ server admin: see `README.rst <README.rst>`_.
The erase parameter is optional and defaults to ``false``.
An empty body may be passed for backwards compatibility.

The following actions are performed when deactivating an user:

- Try to unpind 3PIDs from the identity server
- Remove all 3PIDs from the homeserver
- Delete all devices and E2EE keys
- Delete all access tokens
- Delete the password hash
- Removal from all rooms the user is a member of
- Remove the user from the user directory
- Reject all pending invites
- Remove all account validity information related to the user

The following additional actions are performed during deactivation if``erase``
is set to ``true``:

- Remove the user's display name
- Remove the user's avatar URL
- Mark the user as erased


Reset password
==============
Expand Down
18 changes: 16 additions & 2 deletions synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from synapse.api.errors import SynapseError
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.types import UserID, create_requester
from synapse.types import Requester, UserID, create_requester

from ._base import BaseHandler

Expand All @@ -38,6 +38,7 @@ def __init__(self, hs: "HomeServer"):
self._device_handler = hs.get_device_handler()
self._room_member_handler = hs.get_room_member_handler()
self._identity_handler = hs.get_identity_handler()
self._profile_handler = hs.get_profile_handler()
self.user_directory_handler = hs.get_user_directory_handler()
self._server_name = hs.hostname

Expand All @@ -52,16 +53,23 @@ def __init__(self, hs: "HomeServer"):
self._account_validity_enabled = hs.config.account_validity.enabled

async def deactivate_account(
self, user_id: str, erase_data: bool, id_server: Optional[str] = None
self,
user_id: str,
erase_data: bool,
requester: Requester,
id_server: Optional[str] = None,
by_admin: bool = False,
) -> bool:
"""Deactivate a user's account
Args:
user_id: ID of user to be deactivated
erase_data: whether to GDPR-erase the user's data
requester: The user attempting to make this change.
id_server: Use the given identity server when unbinding
any threepids. If None then will attempt to unbind using the
identity server specified when binding (if known).
by_admin: Whether this change was made by an administrator.
Returns:
True if identity server supports removing threepids, otherwise False.
Expand Down Expand Up @@ -121,6 +129,12 @@ async def deactivate_account(

# Mark the user as erased, if they asked for that
if erase_data:
user = UserID.from_string(user_id)
# Remove avatar URL from this user
await self._profile_handler.set_avatar_url(user, requester, "", by_admin)
# Remove displayname from this user
await self._profile_handler.set_displayname(user, requester, "", by_admin)

logger.info("Marking %s as erased", user_id)
await self.store.mark_user_erased(user_id)

Expand Down
8 changes: 7 additions & 1 deletion synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,19 @@ async def set_avatar_url(
400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN,)
)

avatar_url_to_set = new_avatar_url # type: Optional[str]
if new_avatar_url == "":
avatar_url_to_set = None

# Same like set_displayname
if by_admin:
requester = create_requester(
target_user, authenticated_entity=requester.authenticated_entity
)

await self.store.set_profile_avatar_url(target_user.localpart, new_avatar_url)
await self.store.set_profile_avatar_url(
target_user.localpart, avatar_url_to_set
)

if self.hs.config.user_directory_search_all_users:
profile = await self.store.get_profileinfo(target_user.localpart)
Expand Down
22 changes: 15 additions & 7 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ async def on_PUT(self, request, user_id):

if deactivate and not user["deactivated"]:
await self.deactivate_account_handler.deactivate_account(
target_user.to_string(), False
target_user.to_string(), False, requester, by_admin=True
)
elif not deactivate and user["deactivated"]:
if "password" not in body:
Expand Down Expand Up @@ -486,12 +486,22 @@ async def on_GET(self, request, user_id):
class DeactivateAccountRestServlet(RestServlet):
PATTERNS = admin_patterns("/deactivate/(?P<target_user_id>[^/]*)")

def __init__(self, hs):
def __init__(self, hs: "HomeServer"):
self._deactivate_account_handler = hs.get_deactivate_account_handler()
self.auth = hs.get_auth()
self.is_mine = hs.is_mine
self.store = hs.get_datastore()

async def on_POST(self, request: str, target_user_id: str) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)
await assert_user_is_admin(self.auth, requester.user)

if not self.is_mine(UserID.from_string(target_user_id)):
raise SynapseError(400, "Can only deactivate local users")

if not await self.store.get_user_by_id(target_user_id):
raise NotFoundError("User not found")

async def on_POST(self, request, target_user_id):
await assert_requester_is_admin(self.auth, request)
body = parse_json_object_from_request(request, allow_empty_body=True)
erase = body.get("erase", False)
if not isinstance(erase, bool):
Expand All @@ -501,10 +511,8 @@ async def on_POST(self, request, target_user_id):
Codes.BAD_JSON,
)

UserID.from_string(target_user_id)

result = await self._deactivate_account_handler.deactivate_account(
target_user_id, erase
target_user_id, erase, requester, by_admin=True
)
if result:
id_server_unbind_result = "success"
Expand Down
7 changes: 5 additions & 2 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,18 @@ async def on_POST(self, request):
# allow ASes to deactivate their own users
if requester.app_service:
await self._deactivate_account_handler.deactivate_account(
requester.user.to_string(), erase
requester.user.to_string(), erase, requester
)
return 200, {}

await self.auth_handler.validate_user_via_ui_auth(
requester, request, body, "deactivate your account",
)
result = await self._deactivate_account_handler.deactivate_account(
requester.user.to_string(), erase, id_server=body.get("id_server")
requester.user.to_string(),
erase,
requester,
id_server=body.get("id_server"),
)
if result:
id_server_unbind_result = "success"
Expand Down
2 changes: 1 addition & 1 deletion synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ def get_initial_sync_handler(self) -> InitialSyncHandler:
return InitialSyncHandler(self)

@cache_in_self
def get_profile_handler(self):
def get_profile_handler(self) -> ProfileHandler:
return ProfileHandler(self)

@cache_in_self
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/databases/main/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async def set_profile_displayname(
)

async def set_profile_avatar_url(
self, user_localpart: str, new_avatar_url: str
self, user_localpart: str, new_avatar_url: Optional[str]
) -> None:
await self.db_pool.simple_update_one(
table="profiles",
Expand Down
30 changes: 30 additions & 0 deletions tests/handlers/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ def test_set_my_name(self):
"Frank",
)

# Set displayname to an empty string
yield defer.ensureDeferred(
self.handler.set_displayname(
self.frank, synapse.types.create_requester(self.frank), ""
)
)

self.assertIsNone(
(
yield defer.ensureDeferred(
self.store.get_profile_displayname(self.frank.localpart)
)
)
)

@defer.inlineCallbacks
def test_set_my_name_if_disabled(self):
self.hs.config.enable_set_displayname = False
Expand Down Expand Up @@ -223,6 +238,21 @@ def test_set_my_avatar(self):
"http://my.server/me.png",
)

# Set avatar to an empty string
yield defer.ensureDeferred(
self.handler.set_avatar_url(
self.frank, synapse.types.create_requester(self.frank), "",
)
)

self.assertIsNone(
(
yield defer.ensureDeferred(
self.store.get_profile_avatar_url(self.frank.localpart)
)
),
)

@defer.inlineCallbacks
def test_set_my_avatar_if_disabled(self):
self.hs.config.enable_set_avatar_url = False
Expand Down
Loading

0 comments on commit 7a2e9b5

Please sign in to comment.