From fffb3c4c8f67c271a723855835c2ea0fb83fc33f Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 25 Mar 2022 13:28:42 +0000
Subject: [PATCH] Always allow the empty string as an avatar_url. (#12261)

Hopefully this fixes #12257.

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
---
 changelog.d/12261.bugfix       |  1 +
 synapse/handlers/profile.py    |  6 ++++++
 tests/handlers/test_profile.py |  6 ++++++
 tests/rest/admin/test_user.py  | 19 +++++++++++++++++++
 4 files changed, 32 insertions(+)
 create mode 100644 changelog.d/12261.bugfix

diff --git a/changelog.d/12261.bugfix b/changelog.d/12261.bugfix
new file mode 100644
index 0000000000..1bfde4c380
--- /dev/null
+++ b/changelog.d/12261.bugfix
@@ -0,0 +1 @@
+Fix a bug introduced in Synapse 1.52 where admins could not deactivate and GDPR-erase a user if Synapse was configured with limits on avatars.
diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index 6554c0d3c2..239b0aa744 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -336,12 +336,18 @@ class ProfileHandler:
         """Check that the size and content type of the avatar at the given MXC URI are
         within the configured limits.
 
+        If the given `mxc` is empty, no checks are performed. (Users are always able to
+        unset their avatar.)
+
         Args:
             mxc: The MXC URI at which the avatar can be found.
 
         Returns:
              A boolean indicating whether the file can be allowed to be set as an avatar.
         """
+        if mxc == "":
+            return True
+
         if not self.max_avatar_size and not self.allowed_avatar_mimetypes:
             return True
 
diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py
index 08733a9f2d..1ec105c373 100644
--- a/tests/handlers/test_profile.py
+++ b/tests/handlers/test_profile.py
@@ -267,6 +267,12 @@ class ProfileTestCase(unittest.HomeserverTestCase):
         )
         self.assertTrue(res)
 
+    @unittest.override_config({"max_avatar_size": 50})
+    def test_avatar_constraints_allow_empty_avatar_url(self) -> None:
+        """An empty avatar is always permitted."""
+        res = self.get_success(self.handler.check_avatar_size_and_mime_type(""))
+        self.assertTrue(res)
+
     @unittest.override_config({"max_avatar_size": 50})
     def test_avatar_constraints_missing(self) -> None:
         """Tests that an avatar isn't allowed if the file at the given MXC URI couldn't
diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py
index a60ea0a563..bef911d5df 100644
--- a/tests/rest/admin/test_user.py
+++ b/tests/rest/admin/test_user.py
@@ -1050,6 +1050,25 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
 
         self._is_erased("@user:test", True)
 
+    @override_config({"max_avatar_size": 1234})
+    def test_deactivate_user_erase_true_avatar_nonnull_but_empty(self) -> None:
+        """Check we can erase a user whose avatar is the empty string.
+
+        Reproduces #12257.
+        """
+        # Patch `self.other_user` to have an empty string as their avatar.
+        self.get_success(self.store.set_profile_avatar_url("user", ""))
+
+        # Check we can still erase them.
+        channel = self.make_request(
+            "POST",
+            self.url,
+            access_token=self.admin_user_tok,
+            content={"erase": True},
+        )
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
+        self._is_erased("@user:test", True)
+
     def test_deactivate_user_erase_false(self) -> None:
         """
         Test deactivating a user and set `erase` to `false`