diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index a52b9f8ecf..c3886795b6 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -220,9 +220,7 @@ async def set_displayname( if not by_admin and target_user != requester.user: raise AuthError(400, "Cannot set another user's displayname") - if ( - not by_admin and not self.hs.config.registration.enable_set_displayname - ) and not (deactivation and new_displayname == ""): + if not by_admin and not self.hs.config.registration.enable_set_displayname: profile = await self.store.get_profileinfo(target_user) if profile.display_name: raise SynapseError( @@ -333,9 +331,7 @@ async def set_avatar_url( if not by_admin and target_user != requester.user: raise AuthError(400, "Cannot set another user's avatar_url") - if ( - not by_admin and not self.hs.config.registration.enable_set_avatar_url - ) and not (deactivation and new_avatar_url == ""): + if not by_admin and not self.hs.config.registration.enable_set_avatar_url: profile = await self.store.get_profileinfo(target_user) if profile.avatar_url: raise SynapseError( @@ -410,28 +406,6 @@ async def delete_profile_upon_deactivation( # have it. raise AuthError(400, "Cannot remove another user's profile") - if not by_admin: - current_profile = await self.store.get_profileinfo(target_user) - if not self.hs.config.registration.enable_set_displayname: - if current_profile.display_name: - # SUSPICIOUS: It seems strange to block deactivation on this, - # though this is preserving previous behaviour. - raise SynapseError( - 400, - "Changing display name is disabled on this server", - Codes.FORBIDDEN, - ) - - if not self.hs.config.registration.enable_set_avatar_url: - if current_profile.avatar_url: - # SUSPICIOUS: It seems strange to block deactivation on this, - # though this is preserving previous behaviour. - raise SynapseError( - 400, - "Changing avatar is disabled on this server", - Codes.FORBIDDEN, - ) - await self.store.delete_profile(target_user) await self._third_party_rules.on_profile_update( diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index 122002d55f..9b74e847ff 100644 --- a/tests/rest/client/test_account.py +++ b/tests/rest/client/test_account.py @@ -31,7 +31,7 @@ import synapse.rest.admin from synapse.api.constants import LoginType, Membership -from synapse.api.errors import Codes, HttpResponseException +from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.appservice import ApplicationService from synapse.rest import admin from synapse.rest.client import account, login, register, room @@ -515,20 +515,24 @@ def test_deactivate_erase_account(self) -> None: user_id, create_requester(user_id), "http://test/Kermit.jpg" ) ) - self.erase(mxid, tok) + self.deactivate(mxid, tok, erase=True) store = self.hs.get_datastores().main # Check that the user has been marked as deactivated. self.assertTrue(self.get_success(store.get_user_deactivated_status(mxid))) - # On deactivation with 'erase', a displayname and avatar_url are set to an empty - # string through the handler, but are turned into `None` for the database - display_name = self.get_success(profile_handler.get_displayname(user_id)) - assert display_name is None, f"{display_name}" + # On deactivation with 'erase', the entire database row is erased. Both of these + # should raise a 404(Not Found) SynapseError + display_name_failure = self.get_failure( + profile_handler.get_displayname(user_id), SynapseError + ) + assert display_name_failure.value.code == HTTPStatus.NOT_FOUND - avatar_url = self.get_success(profile_handler.get_avatar_url(user_id)) - assert avatar_url is None, f"{avatar_url}" + avatar_url_failure = self.get_failure( + profile_handler.get_avatar_url(user_id), SynapseError + ) + assert avatar_url_failure.value.code == HTTPStatus.NOT_FOUND # Check that this access token has been invalidated. channel = self.make_request("GET", "account/whoami", access_token=tok) @@ -558,18 +562,22 @@ def test_deactivate_erase_account_with_disabled_profile_changes(self) -> None: ) # self.get_success(profile_handler.set_displayname(user_id, create_requester(user_id), )) - self.erase(mxid, tok) + self.deactivate(mxid, tok, erase=True) # Check that the user has been marked as deactivated. self.assertTrue(self.get_success(store.get_user_deactivated_status(mxid))) - # On deactivation with 'erase', a displayname and avatar_url are set to an empty - # string through the handler, but are turned into `None` for the database - display_name = self.get_success(profile_handler.get_displayname(user_id)) - assert display_name is None, f"{display_name}" + # On deactivation with 'erase', the entire database row is erased. Both of these + # should raise a 404(Not Found) SynapseError + display_name_failure = self.get_failure( + profile_handler.get_displayname(user_id), SynapseError + ) + assert display_name_failure.value.code == HTTPStatus.NOT_FOUND - avatar_url = self.get_success(profile_handler.get_avatar_url(user_id)) - assert avatar_url is None, f"{avatar_url}" + avatar_url_failure = self.get_failure( + profile_handler.get_avatar_url(user_id), SynapseError + ) + assert avatar_url_failure.value.code == HTTPStatus.NOT_FOUND # Check that this access token has been invalidated. channel = self.make_request("GET", "account/whoami", access_token=tok) @@ -773,28 +781,14 @@ def test_background_update_deletes_deactivated_users_server_side_backup_keys( ) self.assertEqual(len(res2), 4) - def deactivate(self, user_id: str, tok: str) -> None: - request_data = { - "auth": { - "type": "m.login.password", - "user": user_id, - "password": "test", - }, - "erase": False, - } - channel = self.make_request( - "POST", "account/deactivate", request_data, access_token=tok - ) - self.assertEqual(channel.code, 200, channel.json_body) - - def erase(self, user_id: str, tok: str) -> None: + def deactivate(self, user_id: str, tok: str, erase: bool = False) -> None: request_data = { "auth": { "type": "m.login.password", "user": user_id, "password": "test", }, - "erase": True, + "erase": erase, } channel = self.make_request( "POST", "account/deactivate", request_data, access_token=tok