Fix deactivate a user if he does not have a profile (#10252)

This commit is contained in:
Dirk Klimpel 2021-07-06 14:08:53 +02:00 committed by GitHub
parent 6655ea5587
commit bcb0962a72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 22 deletions

1
changelog.d/10252.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in v1.26.0 where only users who have set profile information could be deactivated with erasure enabled.

View File

@ -73,20 +73,20 @@ class ProfileWorkerStore(SQLBaseStore):
async def set_profile_displayname( async def set_profile_displayname(
self, user_localpart: str, new_displayname: Optional[str] self, user_localpart: str, new_displayname: Optional[str]
) -> None: ) -> None:
await self.db_pool.simple_update_one( await self.db_pool.simple_upsert(
table="profiles", table="profiles",
keyvalues={"user_id": user_localpart}, keyvalues={"user_id": user_localpart},
updatevalues={"displayname": new_displayname}, values={"displayname": new_displayname},
desc="set_profile_displayname", desc="set_profile_displayname",
) )
async def set_profile_avatar_url( async def set_profile_avatar_url(
self, user_localpart: str, new_avatar_url: Optional[str] self, user_localpart: str, new_avatar_url: Optional[str]
) -> None: ) -> None:
await self.db_pool.simple_update_one( await self.db_pool.simple_upsert(
table="profiles", table="profiles",
keyvalues={"user_id": user_localpart}, keyvalues={"user_id": user_localpart},
updatevalues={"avatar_url": new_avatar_url}, values={"avatar_url": new_avatar_url},
desc="set_profile_avatar_url", desc="set_profile_avatar_url",
) )

View File

@ -939,7 +939,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
""" """
channel = self.make_request("POST", self.url, b"{}") channel = self.make_request("POST", self.url, b"{}")
self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(401, channel.code, msg=channel.json_body)
self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
def test_requester_is_not_admin(self): def test_requester_is_not_admin(self):
@ -950,7 +950,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
channel = self.make_request("POST", url, access_token=self.other_user_token) channel = self.make_request("POST", url, access_token=self.other_user_token)
self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(403, channel.code, msg=channel.json_body)
self.assertEqual("You are not a server admin", channel.json_body["error"]) self.assertEqual("You are not a server admin", channel.json_body["error"])
channel = self.make_request( channel = self.make_request(
@ -960,7 +960,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
content=b"{}", content=b"{}",
) )
self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(403, channel.code, msg=channel.json_body)
self.assertEqual("You are not a server admin", channel.json_body["error"]) self.assertEqual("You are not a server admin", channel.json_body["error"])
def test_user_does_not_exist(self): def test_user_does_not_exist(self):
@ -990,7 +990,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(400, channel.code, msg=channel.json_body)
self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"])
def test_user_is_not_local(self): def test_user_is_not_local(self):
@ -1006,7 +1006,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
def test_deactivate_user_erase_true(self): def test_deactivate_user_erase_true(self):
""" """
Test deactivating an user and set `erase` to `true` Test deactivating a user and set `erase` to `true`
""" """
# Get user # Get user
@ -1016,24 +1016,22 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"]) self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"]) self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"]) self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"]) self.assertEqual("User1", channel.json_body["displayname"])
# Deactivate user # Deactivate and erase user
body = json.dumps({"erase": True})
channel = self.make_request( channel = self.make_request(
"POST", "POST",
self.url, self.url,
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"), content={"erase": True},
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, channel.code, msg=channel.json_body)
# Get user # Get user
channel = self.make_request( channel = self.make_request(
@ -1042,7 +1040,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"]) self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"])) self.assertEqual(0, len(channel.json_body["threepids"]))
@ -1053,7 +1051,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
def test_deactivate_user_erase_false(self): def test_deactivate_user_erase_false(self):
""" """
Test deactivating an user and set `erase` to `false` Test deactivating a user and set `erase` to `false`
""" """
# Get user # Get user
@ -1063,7 +1061,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"]) self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"]) self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
@ -1071,13 +1069,11 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
self.assertEqual("User1", channel.json_body["displayname"]) self.assertEqual("User1", channel.json_body["displayname"])
# Deactivate user # Deactivate user
body = json.dumps({"erase": False})
channel = self.make_request( channel = self.make_request(
"POST", "POST",
self.url, self.url,
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"), content={"erase": False},
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
@ -1089,7 +1085,7 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"]) self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"])) self.assertEqual(0, len(channel.json_body["threepids"]))
@ -1098,6 +1094,60 @@ class DeactivateAccountTestCase(unittest.HomeserverTestCase):
self._is_erased("@user:test", False) self._is_erased("@user:test", False)
def test_deactivate_user_erase_true_no_profile(self):
"""
Test deactivating a user and set `erase` to `true`
if user has no profile information (stored in the database table `profiles`).
"""
# Users normally have an entry in `profiles`, but occasionally they are created without one.
# To test deactivation for users without a profile, we delete the profile information for our user.
self.get_success(
self.store.db_pool.simple_delete_one(
table="profiles", keyvalues={"user_id": "user"}
)
)
# Get user
channel = self.make_request(
"GET",
self.url_other_user,
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(False, channel.json_body["deactivated"])
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertIsNone(channel.json_body["avatar_url"])
self.assertIsNone(channel.json_body["displayname"])
# Deactivate and erase user
channel = self.make_request(
"POST",
self.url,
access_token=self.admin_user_tok,
content={"erase": True},
)
self.assertEqual(200, channel.code, msg=channel.json_body)
# Get user
channel = self.make_request(
"GET",
self.url_other_user,
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertEqual("@user:test", channel.json_body["name"])
self.assertEqual(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertIsNone(channel.json_body["avatar_url"])
self.assertIsNone(channel.json_body["displayname"])
self._is_erased("@user:test", True)
def _is_erased(self, user_id: str, expect: bool) -> None: def _is_erased(self, user_id: str, expect: bool) -> None:
"""Assert that the user is erased or not""" """Assert that the user is erased or not"""
d = self.store.is_user_erased(user_id) d = self.store.is_user_erased(user_id)