fix: Allow account deactivation even with disabled profile changes (#84)

Author: jason-famedly

synapse/handlers/profile.py

@@ -192,7 +192,9 @@ 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:
+        if (
+            not by_admin and not self.hs.config.registration.enable_set_displayname
+        ) and not (deactivation and new_displayname == ""):
             profile = await self.store.get_profileinfo(target_user)
             if profile.display_name:
                 raise SynapseError(
@@ -296,7 +298,9 @@ 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:
+        if (
+            not by_admin and not self.hs.config.registration.enable_set_avatar_url
+        ) and not (deactivation and new_avatar_url == ""):
             profile = await self.store.get_profileinfo(target_user)
             if profile.avatar_url:
                 raise SynapseError(

tests/handlers/test_profile.py

@@ -33,6 +33,7 @@
 from synapse.util import Clock
 
 from tests import unittest
+from tests.unittest import override_config
 
 
 class ProfileTestCase(unittest.HomeserverTestCase):
@@ -113,9 +114,8 @@ def test_set_my_name(self) -> None:
             self.get_success(self.store.get_profile_displayname(self.frank))
         )
 
-    def test_set_my_name_if_disabled(self) -> None:
-        self.hs.config.registration.enable_set_displayname = False
-
+    @override_config({"enable_set_displayname": False})
+    def test_set_displayname_if_disabled(self) -> None:
         # Setting displayname for the first time is allowed
         self.get_success(self.store.set_profile_displayname(self.frank, "Frank"))
 
@@ -234,9 +234,8 @@ def test_set_my_avatar(self) -> None:
             (self.get_success(self.store.get_profile_avatar_url(self.frank))),
         )
 
-    def test_set_my_avatar_if_disabled(self) -> None:
-        self.hs.config.registration.enable_set_avatar_url = False
-
+    @override_config({"enable_set_avatar_url": False})
+    def test_set_avatar_url_if_disabled(self) -> None:
         # Setting displayname for the first time is allowed
         self.get_success(
             self.store.set_profile_avatar_url(self.frank, "http://my.server/me.png")

tests/rest/client/test_account.py

@@ -39,7 +39,7 @@
 from synapse.rest.synapse.client.password_reset import PasswordResetSubmitTokenResource
 from synapse.server import HomeServer
 from synapse.storage._base import db_to_json
-from synapse.types import JsonDict, UserID
+from synapse.types import JsonDict, UserID, create_requester
 from synapse.util import Clock
 
 from tests import unittest
@@ -500,6 +500,81 @@ def test_deactivate_account(self) -> None:
         channel = self.make_request("GET", "account/whoami", access_token=tok)
         self.assertEqual(channel.code, 401)
 
+    def test_deactivate_erase_account(self) -> None:
+        mxid = self.register_user("kermit", "test")
+        user_id = UserID.from_string(mxid)
+        tok = self.login("kermit", "test")
+        profile_handler = self.hs.get_profile_handler()
+        self.get_success(
+            profile_handler.set_displayname(
+                user_id, create_requester(user_id), "Kermit the Frog"
+            )
+        )
+        self.get_success(
+            profile_handler.set_avatar_url(
+                user_id, create_requester(user_id), "http://test/Kermit.jpg"
+            )
+        )
+        self.erase(mxid, tok)
+
+        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}"
+
+        avatar_url = self.get_success(profile_handler.get_avatar_url(user_id))
+        assert avatar_url is None, f"{avatar_url}"
+
+        # Check that this access token has been invalidated.
+        channel = self.make_request("GET", "account/whoami", access_token=tok)
+        self.assertEqual(channel.code, 401)
+
+    @override_config({"enable_set_displayname": False, "enable_set_avatar_url": False})
+    def test_deactivate_erase_account_with_disabled_profile_changes(self) -> None:
+        mxid = self.register_user("kermit", "test")
+        user_id = UserID.from_string(mxid)
+        tok = self.login("kermit", "test")
+        profile_handler = self.hs.get_profile_handler()
+        # Can not use the profile handler to set a display name when it is disabled. Use
+        # the database directly
+        store = self.hs.get_datastores().main
+        self.get_success(store.set_profile_displayname(user_id, "Kermit the Frog"))
+
+        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(store.get_profile_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)
+
+        # 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}"
+
+        avatar_url = self.get_success(profile_handler.get_avatar_url(user_id))
+        assert avatar_url is None, f"{avatar_url}"
+
+        # Check that this access token has been invalidated.
+        channel = self.make_request("GET", "account/whoami", access_token=tok)
+        self.assertEqual(channel.code, 401)
+
     def test_pending_invites(self) -> None:
         """Tests that deactivating a user rejects every pending invite for them."""
         store = self.hs.get_datastores().main
@@ -712,6 +787,20 @@ def deactivate(self, user_id: str, tok: str) -> None:
         )
         self.assertEqual(channel.code, 200, channel.json_body)
 
+    def erase(self, user_id: str, tok: str) -> None:
+        request_data = {
+            "auth": {
+                "type": "m.login.password",
+                "user": user_id,
+                "password": "test",
+            },
+            "erase": True,
+        }
+        channel = self.make_request(
+            "POST", "account/deactivate", request_data, access_token=tok
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
 
 class WhoamiTestCase(unittest.HomeserverTestCase):
     servlets = [