diff options
| -rw-r--r-- | changelog.d/12261.bugfix | 1 | ||||
| -rw-r--r-- | synapse/handlers/profile.py | 6 | ||||
| -rw-r--r-- | tests/handlers/test_profile.py | 6 | ||||
| -rw-r--r-- | tests/rest/admin/test_user.py | 19 |
4 files changed, 32 insertions, 0 deletions
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 @@ -268,6 +268,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 be found. 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` |