From 6ae65bcfd91b0edbb0cceb94fe9c0e37029ae74c Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 27 Mar 2026 17:06:12 -0500 Subject: [PATCH 1/3] revert: Partial revert of #84 The 'deactivation' argument was removed from the profile handler setting methods. Will have to use another method Keeping the tests --- synapse/handlers/profile.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index a52b9f8ecf..d123bcdd36 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( From 6b7da5af04ce94a0c27176e13a693338c8b13301 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 27 Mar 2026 17:37:12 -0500 Subject: [PATCH 2/3] fix: Allow account deactivation even with disabled profile changes, take 2 --- synapse/handlers/profile.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index d123bcdd36..c3886795b6 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -406,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( From 6b7201dd317697524fd71b2b610ed6f481f12269 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Fri, 27 Mar 2026 17:38:03 -0500 Subject: [PATCH 3/3] test: adjust testing for deactivation to compensate for new behavior And refactor a test helper that was mostly duplicated code --- tests/rest/client/test_account.py | 90 +++++++++++++++++++------------ 1 file changed, 56 insertions(+), 34 deletions(-) diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py index 122002d55f..2e8089b363 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 @@ -501,6 +501,10 @@ def test_deactivate_account(self) -> None: self.assertEqual(channel.code, 401) def test_deactivate_erase_account(self) -> None: + """ + Test that deactivating the user with the 'erase' option will remove existing + profile data. + """ mxid = self.register_user("kermit", "test") user_id = UserID.from_string(mxid) tok = self.login("kermit", "test") @@ -515,20 +519,35 @@ def test_deactivate_erase_account(self) -> None: user_id, create_requester(user_id), "http://test/Kermit.jpg" ) ) - self.erase(mxid, tok) + # Verify it is set + self.assertEqual( + self.get_success(profile_handler.get_displayname(user_id)), + "Kermit the Frog", + ) + self.assertEqual( + self.get_success(profile_handler.get_avatar_url(user_id)), + "http://test/Kermit.jpg", + ) + + # Deactivate! + 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) @@ -536,6 +555,10 @@ def test_deactivate_erase_account(self) -> None: @override_config({"enable_set_displayname": False, "enable_set_avatar_url": False}) def test_deactivate_erase_account_with_disabled_profile_changes(self) -> None: + """ + Test that deactivating the user with the 'erase' option will remove existing + profile data, even with the Synapse configuration to forbid profile changes + """ mxid = self.register_user("kermit", "test") user_id = UserID.from_string(mxid) tok = self.login("kermit", "test") @@ -544,32 +567,45 @@ def test_deactivate_erase_account_with_disabled_profile_changes(self) -> None: # the database directly store = self.hs.get_datastores().main self.get_success(store.set_profile_displayname(user_id, "Kermit the Frog")) + self.get_success( + store.set_profile_avatar_url(user_id, "http://test/Kermit.jpg") + ) + # Verify it is set self.assertEqual( (self.get_success(store.get_profile_displayname(user_id))), "Kermit the Frog", ) - self.get_success( - store.set_profile_avatar_url(user_id, "http://test/Kermit.jpg") + self.assertEqual( + self.get_success(profile_handler.get_displayname(user_id)), + "Kermit the Frog", ) self.assertEqual( (self.get_success(store.get_profile_avatar_url(user_id))), "http://test/Kermit.jpg", ) + self.assertEqual( + self.get_success(profile_handler.get_avatar_url(user_id)), + "http://test/Kermit.jpg", + ) - # self.get_success(profile_handler.set_displayname(user_id, create_requester(user_id), )) - self.erase(mxid, tok) + # Deactivate! + 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 +809,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