Remove user's avatar URL and displayname when deactivated. (#8932)

This only applies if the user's data is to be erased.
This commit is contained in:
Dirk Klimpel 2021-01-12 22:30:15 +01:00 committed by GitHub
parent 789d9ebad3
commit 7a2e9b549d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 351 additions and 17 deletions

1
changelog.d/8932.feature Normal file
View File

@ -0,0 +1 @@
Remove a user's avatar URL and display name when deactivated with the Admin API.

View File

@ -98,6 +98,8 @@ Body parameters:
- ``deactivated``, optional. If unspecified, deactivation state will be left - ``deactivated``, optional. If unspecified, deactivation state will be left
unchanged on existing accounts and set to ``false`` for new accounts. 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. If the user already exists then optional parameters default to the current value.
@ -248,6 +250,25 @@ server admin: see `README.rst <README.rst>`_.
The erase parameter is optional and defaults to ``false``. The erase parameter is optional and defaults to ``false``.
An empty body may be passed for backwards compatibility. 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 Reset password
============== ==============

View File

@ -18,7 +18,7 @@ from typing import TYPE_CHECKING, Optional
from synapse.api.errors import SynapseError from synapse.api.errors import SynapseError
from synapse.metrics.background_process_metrics import run_as_background_process 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 from ._base import BaseHandler
@ -38,6 +38,7 @@ class DeactivateAccountHandler(BaseHandler):
self._device_handler = hs.get_device_handler() self._device_handler = hs.get_device_handler()
self._room_member_handler = hs.get_room_member_handler() self._room_member_handler = hs.get_room_member_handler()
self._identity_handler = hs.get_identity_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.user_directory_handler = hs.get_user_directory_handler()
self._server_name = hs.hostname self._server_name = hs.hostname
@ -52,16 +53,23 @@ class DeactivateAccountHandler(BaseHandler):
self._account_validity_enabled = hs.config.account_validity.enabled self._account_validity_enabled = hs.config.account_validity.enabled
async def deactivate_account( 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: ) -> bool:
"""Deactivate a user's account """Deactivate a user's account
Args: Args:
user_id: ID of user to be deactivated user_id: ID of user to be deactivated
erase_data: whether to GDPR-erase the user's data 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 id_server: Use the given identity server when unbinding
any threepids. If None then will attempt to unbind using the any threepids. If None then will attempt to unbind using the
identity server specified when binding (if known). identity server specified when binding (if known).
by_admin: Whether this change was made by an administrator.
Returns: Returns:
True if identity server supports removing threepids, otherwise False. True if identity server supports removing threepids, otherwise False.
@ -121,6 +129,12 @@ class DeactivateAccountHandler(BaseHandler):
# Mark the user as erased, if they asked for that # Mark the user as erased, if they asked for that
if erase_data: 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) logger.info("Marking %s as erased", user_id)
await self.store.mark_user_erased(user_id) await self.store.mark_user_erased(user_id)

View File

@ -286,13 +286,19 @@ class ProfileHandler(BaseHandler):
400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN,) 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 # Same like set_displayname
if by_admin: if by_admin:
requester = create_requester( requester = create_requester(
target_user, authenticated_entity=requester.authenticated_entity 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: if self.hs.config.user_directory_search_all_users:
profile = await self.store.get_profileinfo(target_user.localpart) profile = await self.store.get_profileinfo(target_user.localpart)

View File

@ -244,7 +244,7 @@ class UserRestServletV2(RestServlet):
if deactivate and not user["deactivated"]: if deactivate and not user["deactivated"]:
await self.deactivate_account_handler.deactivate_account( 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"]: elif not deactivate and user["deactivated"]:
if "password" not in body: if "password" not in body:
@ -486,12 +486,22 @@ class WhoisRestServlet(RestServlet):
class DeactivateAccountRestServlet(RestServlet): class DeactivateAccountRestServlet(RestServlet):
PATTERNS = admin_patterns("/deactivate/(?P<target_user_id>[^/]*)") 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._deactivate_account_handler = hs.get_deactivate_account_handler()
self.auth = hs.get_auth() 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) body = parse_json_object_from_request(request, allow_empty_body=True)
erase = body.get("erase", False) erase = body.get("erase", False)
if not isinstance(erase, bool): if not isinstance(erase, bool):
@ -501,10 +511,8 @@ class DeactivateAccountRestServlet(RestServlet):
Codes.BAD_JSON, Codes.BAD_JSON,
) )
UserID.from_string(target_user_id)
result = await self._deactivate_account_handler.deactivate_account( result = await self._deactivate_account_handler.deactivate_account(
target_user_id, erase target_user_id, erase, requester, by_admin=True
) )
if result: if result:
id_server_unbind_result = "success" id_server_unbind_result = "success"

View File

@ -305,7 +305,7 @@ class DeactivateAccountRestServlet(RestServlet):
# allow ASes to deactivate their own users # allow ASes to deactivate their own users
if requester.app_service: if requester.app_service:
await self._deactivate_account_handler.deactivate_account( await self._deactivate_account_handler.deactivate_account(
requester.user.to_string(), erase requester.user.to_string(), erase, requester
) )
return 200, {} return 200, {}
@ -313,7 +313,10 @@ class DeactivateAccountRestServlet(RestServlet):
requester, request, body, "deactivate your account", requester, request, body, "deactivate your account",
) )
result = await self._deactivate_account_handler.deactivate_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: if result:
id_server_unbind_result = "success" id_server_unbind_result = "success"

View File

@ -501,7 +501,7 @@ class HomeServer(metaclass=abc.ABCMeta):
return InitialSyncHandler(self) return InitialSyncHandler(self)
@cache_in_self @cache_in_self
def get_profile_handler(self): def get_profile_handler(self) -> ProfileHandler:
return ProfileHandler(self) return ProfileHandler(self)
@cache_in_self @cache_in_self

View File

@ -82,7 +82,7 @@ class ProfileWorkerStore(SQLBaseStore):
) )
async def set_profile_avatar_url( async def set_profile_avatar_url(
self, user_localpart: str, new_avatar_url: str self, user_localpart: str, new_avatar_url: Optional[str]
) -> None: ) -> None:
await self.db_pool.simple_update_one( await self.db_pool.simple_update_one(
table="profiles", table="profiles",

View File

@ -105,6 +105,21 @@ class ProfileTestCase(unittest.TestCase):
"Frank", "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 @defer.inlineCallbacks
def test_set_my_name_if_disabled(self): def test_set_my_name_if_disabled(self):
self.hs.config.enable_set_displayname = False self.hs.config.enable_set_displayname = False
@ -223,6 +238,21 @@ class ProfileTestCase(unittest.TestCase):
"http://my.server/me.png", "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 @defer.inlineCallbacks
def test_set_my_avatar_if_disabled(self): def test_set_my_avatar_if_disabled(self):
self.hs.config.enable_set_avatar_url = False self.hs.config.enable_set_avatar_url = False

View File

@ -588,6 +588,200 @@ class UsersListTestCase(unittest.HomeserverTestCase):
_search_test(None, "bar", "user_id") _search_test(None, "bar", "user_id")
class DeactivateAccountTestCase(unittest.HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
login.register_servlets,
]
def prepare(self, reactor, clock, hs):
self.store = hs.get_datastore()
self.admin_user = self.register_user("admin", "pass", admin=True)
self.admin_user_tok = self.login("admin", "pass")
self.other_user = self.register_user("user", "pass", displayname="User1")
self.other_user_token = self.login("user", "pass")
self.url_other_user = "/_synapse/admin/v2/users/%s" % urllib.parse.quote(
self.other_user
)
self.url = "/_synapse/admin/v1/deactivate/%s" % urllib.parse.quote(
self.other_user
)
# set attributes for user
self.get_success(
self.store.set_profile_avatar_url("user", "mxc://servername/mediaid")
)
self.get_success(
self.store.user_add_threepid("@user:test", "email", "foo@bar.com", 0, 0)
)
def test_no_auth(self):
"""
Try to deactivate users without authentication.
"""
channel = self.make_request("POST", self.url, b"{}")
self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
def test_requester_is_not_admin(self):
"""
If the user is not a server admin, an error is returned.
"""
url = "/_synapse/admin/v1/deactivate/@bob:test"
channel = self.make_request("POST", url, access_token=self.other_user_token)
self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("You are not a server admin", channel.json_body["error"])
channel = self.make_request(
"POST", url, access_token=self.other_user_token, content=b"{}",
)
self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("You are not a server admin", channel.json_body["error"])
def test_user_does_not_exist(self):
"""
Tests that deactivation for a user that does not exist returns a 404
"""
channel = self.make_request(
"POST",
"/_synapse/admin/v1/deactivate/@unknown_person:test",
access_token=self.admin_user_tok,
)
self.assertEqual(404, channel.code, msg=channel.json_body)
self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
def test_erase_is_not_bool(self):
"""
If parameter `erase` is not boolean, return an error
"""
body = json.dumps({"erase": "False"})
channel = self.make_request(
"POST",
self.url,
content=body.encode(encoding="utf_8"),
access_token=self.admin_user_tok,
)
self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"])
def test_user_is_not_local(self):
"""
Tests that deactivation for a user that is not a local returns a 400
"""
url = "/_synapse/admin/v1/deactivate/@unknown_person:unknown_domain"
channel = self.make_request("POST", url, access_token=self.admin_user_tok)
self.assertEqual(400, channel.code, msg=channel.json_body)
self.assertEqual("Can only deactivate local users", channel.json_body["error"])
def test_deactivate_user_erase_true(self):
"""
Test deactivating an user and set `erase` to `true`
"""
# Get user
channel = self.make_request(
"GET", self.url_other_user, access_token=self.admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])
# Deactivate user
body = json.dumps({"erase": True})
channel = self.make_request(
"POST",
self.url,
access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"),
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
# Get user
channel = self.make_request(
"GET", self.url_other_user, access_token=self.admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertIsNone(channel.json_body["avatar_url"])
self.assertIsNone(channel.json_body["displayname"])
self._is_erased("@user:test", True)
def test_deactivate_user_erase_false(self):
"""
Test deactivating an user and set `erase` to `false`
"""
# Get user
channel = self.make_request(
"GET", self.url_other_user, access_token=self.admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])
# Deactivate user
body = json.dumps({"erase": False})
channel = self.make_request(
"POST",
self.url,
access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"),
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
# Get user
channel = self.make_request(
"GET", self.url_other_user, access_token=self.admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])
self._is_erased("@user:test", False)
def _is_erased(self, user_id: str, expect: bool) -> None:
"""Assert that the user is erased or not
"""
d = self.store.is_user_erased(user_id)
if expect:
self.assertTrue(self.get_success(d))
else:
self.assertFalse(self.get_success(d))
class UserRestTestCase(unittest.HomeserverTestCase): class UserRestTestCase(unittest.HomeserverTestCase):
servlets = [ servlets = [
@ -987,6 +1181,26 @@ class UserRestTestCase(unittest.HomeserverTestCase):
Test deactivating another user. Test deactivating another user.
""" """
# set attributes for user
self.get_success(
self.store.set_profile_avatar_url("user", "mxc://servername/mediaid")
)
self.get_success(
self.store.user_add_threepid("@user:test", "email", "foo@bar.com", 0, 0)
)
# Get user
channel = self.make_request(
"GET", self.url_other_user, access_token=self.admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User", channel.json_body["displayname"])
# Deactivate user # Deactivate user
body = json.dumps({"deactivated": True}) body = json.dumps({"deactivated": True})
@ -1000,6 +1214,9 @@ class UserRestTestCase(unittest.HomeserverTestCase):
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"]) self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User", channel.json_body["displayname"])
# the user is deactivated, the threepid will be deleted # the user is deactivated, the threepid will be deleted
# Get user # Get user
@ -1010,6 +1227,9 @@ class UserRestTestCase(unittest.HomeserverTestCase):
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"]) self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User", channel.json_body["displayname"])
@override_config({"user_directory": {"enabled": True, "search_all_users": True}}) @override_config({"user_directory": {"enabled": True, "search_all_users": True}})
def test_change_name_deactivate_user_user_directory(self): def test_change_name_deactivate_user_user_directory(self):

View File

@ -30,6 +30,7 @@ from synapse.rest.client.v1 import login, logout
from synapse.rest.client.v2_alpha import devices, register from synapse.rest.client.v2_alpha import devices, register
from synapse.rest.client.v2_alpha.account import WhoamiRestServlet from synapse.rest.client.v2_alpha.account import WhoamiRestServlet
from synapse.rest.synapse.client.pick_idp import PickIdpResource from synapse.rest.synapse.client.pick_idp import PickIdpResource
from synapse.types import create_requester
from tests import unittest from tests import unittest
from tests.handlers.test_oidc import HAS_OIDC from tests.handlers.test_oidc import HAS_OIDC
@ -667,7 +668,9 @@ class CASTestCase(unittest.HomeserverTestCase):
# Deactivate the account. # Deactivate the account.
self.get_success( self.get_success(
self.deactivate_account_handler.deactivate_account(self.user_id, False) self.deactivate_account_handler.deactivate_account(
self.user_id, False, create_requester(self.user_id)
)
) )
# Request the CAS ticket. # Request the CAS ticket.

View File

@ -29,7 +29,7 @@ from synapse.handlers.pagination import PurgeStatus
from synapse.rest import admin from synapse.rest import admin
from synapse.rest.client.v1 import directory, login, profile, room from synapse.rest.client.v1 import directory, login, profile, room
from synapse.rest.client.v2_alpha import account from synapse.rest.client.v2_alpha import account
from synapse.types import JsonDict, RoomAlias, UserID from synapse.types import JsonDict, RoomAlias, UserID, create_requester
from synapse.util.stringutils import random_string from synapse.util.stringutils import random_string
from tests import unittest from tests import unittest
@ -1687,7 +1687,9 @@ class ContextTestCase(unittest.HomeserverTestCase):
deactivate_account_handler = self.hs.get_deactivate_account_handler() deactivate_account_handler = self.hs.get_deactivate_account_handler()
self.get_success( self.get_success(
deactivate_account_handler.deactivate_account(self.user_id, erase_data=True) deactivate_account_handler.deactivate_account(
self.user_id, True, create_requester(self.user_id)
)
) )
# Invite another user in the room. This is needed because messages will be # Invite another user in the room. This is needed because messages will be

View File

@ -48,6 +48,19 @@ class ProfileStoreTestCase(unittest.TestCase):
), ),
) )
# test set to None
yield defer.ensureDeferred(
self.store.set_profile_displayname(self.u_frank.localpart, None)
)
self.assertIsNone(
(
yield defer.ensureDeferred(
self.store.get_profile_displayname(self.u_frank.localpart)
)
)
)
@defer.inlineCallbacks @defer.inlineCallbacks
def test_avatar_url(self): def test_avatar_url(self):
yield defer.ensureDeferred(self.store.create_profile(self.u_frank.localpart)) yield defer.ensureDeferred(self.store.create_profile(self.u_frank.localpart))
@ -66,3 +79,16 @@ class ProfileStoreTestCase(unittest.TestCase):
) )
), ),
) )
# test set to None
yield defer.ensureDeferred(
self.store.set_profile_avatar_url(self.u_frank.localpart, None)
)
self.assertIsNone(
(
yield defer.ensureDeferred(
self.store.get_profile_avatar_url(self.u_frank.localpart)
)
)
)