Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10252.bugfix
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 4 additions & 4 deletions synapse/storage/databases/main/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,20 @@ async def create_profile(self, user_localpart: str) -> None:
async def set_profile_displayname(
self, user_localpart: str, new_displayname: Optional[str]
) -> None:
await self.db_pool.simple_update_one(
await self.db_pool.simple_upsert(
table="profiles",
keyvalues={"user_id": user_localpart},
updatevalues={"displayname": new_displayname},
values={"displayname": new_displayname},
desc="set_profile_displayname",
)

async def set_profile_avatar_url(
self, user_localpart: str, new_avatar_url: Optional[str]
) -> None:
await self.db_pool.simple_update_one(
await self.db_pool.simple_upsert(
table="profiles",
keyvalues={"user_id": user_localpart},
updatevalues={"avatar_url": new_avatar_url},
values={"avatar_url": new_avatar_url},
desc="set_profile_avatar_url",
)

Expand Down
86 changes: 68 additions & 18 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ def test_no_auth(self):
"""
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"])

def test_requester_is_not_admin(self):
Expand All @@ -950,7 +950,7 @@ def test_requester_is_not_admin(self):

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"])

channel = self.make_request(
Expand All @@ -960,7 +960,7 @@ def test_requester_is_not_admin(self):
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"])

def test_user_does_not_exist(self):
Expand Down Expand Up @@ -990,7 +990,7 @@ def test_erase_is_not_bool(self):
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"])

def test_user_is_not_local(self):
Expand All @@ -1006,7 +1006,7 @@ def test_user_is_not_local(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
Expand All @@ -1016,24 +1016,22 @@ def test_deactivate_user_erase_true(self):
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(False, channel.json_body["deactivated"])
self.assertEqual("[email protected]", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])

# Deactivate user
body = json.dumps({"erase": True})

# Deactivate and erase user
channel = self.make_request(
"POST",
self.url,
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
channel = self.make_request(
Expand All @@ -1042,7 +1040,7 @@ def test_deactivate_user_erase_true(self):
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(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
Expand All @@ -1053,7 +1051,7 @@ def test_deactivate_user_erase_true(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
Expand All @@ -1063,21 +1061,19 @@ def test_deactivate_user_erase_false(self):
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(False, channel.json_body["deactivated"])
self.assertEqual("[email protected]", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])

# Deactivate user
body = json.dumps({"erase": False})

channel = self.make_request(
"POST",
self.url,
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"])
Expand All @@ -1089,7 +1085,7 @@ def test_deactivate_user_erase_false(self):
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(True, channel.json_body["deactivated"])
self.assertEqual(0, len(channel.json_body["threepids"]))
Expand All @@ -1098,6 +1094,60 @@ def test_deactivate_user_erase_false(self):

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("[email protected]", 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:
"""Assert that the user is erased or not"""
d = self.store.is_user_erased(user_id)
Expand Down