Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Show erasure status when listing users in the Admin API #14205

Merged
merged 15 commits into from
Oct 21, 2022
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/14205.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Show erasure status when listing users in the Admin API.
4 changes: 4 additions & 0 deletions docs/admin_api/user_admin_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ It returns a JSON body like the following:
"is_guest": 0,
"admin": 0,
"deactivated": 0,
"erased": false,
"shadow_banned": 0,
"creation_ts": 1560432506,
"appservice_id": null,
Expand Down Expand Up @@ -167,6 +168,7 @@ A response body like the following is returned:
"admin": 0,
"user_type": null,
"deactivated": 0,
"erased": false,
"shadow_banned": 0,
"displayname": "<User One>",
"avatar_url": null,
Expand All @@ -177,6 +179,7 @@ A response body like the following is returned:
"admin": 1,
"user_type": null,
"deactivated": 0,
"erased": false,
"shadow_banned": 0,
"displayname": "<User Two>",
"avatar_url": "<avatar_url>",
Expand Down Expand Up @@ -247,6 +250,7 @@ The following fields are returned in the JSON response body:
- `user_type` - string - Type of the user. Normal users are type `None`.
This allows user type specific behaviour. There are also types `support` and `bot`.
- `deactivated` - bool - Status if that user has been marked as deactivated.
- `erased` - bool - Status if that user has been marked as erased.
- `shadow_banned` - bool - Status if that user has been marked as shadow banned.
- `displayname` - string - The user's display name if they have set one.
- `avatar_url` - string - The user's avatar URL if they have set one.
Expand Down
1 change: 1 addition & 0 deletions synapse/handlers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ async def get_user(self, user: UserID) -> Optional[JsonDict]:
user_info_dict["avatar_url"] = profile.avatar_url
user_info_dict["threepids"] = threepids
user_info_dict["external_ids"] = external_ids
user_info_dict["erased"] = await self.store.is_user_erased(user.to_string())

return user_info_dict

Expand Down
13 changes: 11 additions & 2 deletions synapse/storage/databases/main/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ async def get_users_paginate(
name: Optional[str] = None,
guests: bool = True,
deactivated: bool = False,
order_by: str = UserSortOrder.USER_ID.value,
order_by: str = UserSortOrder.NAME.value,
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
direction: str = "f",
approved: bool = True,
) -> Tuple[List[JsonDict], int]:
Expand Down Expand Up @@ -261,6 +261,7 @@ def get_users_paginate_txn(
sql_base = f"""
FROM users as u
LEFT JOIN profiles AS p ON u.name = '@' || p.user_id || ':' || ?
LEFT JOIN erased_users AS eu ON u.name = eu.user_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a significant impact on performance with an additional left join on a large list?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a query plan here: #14205 (comment)

There is a sequential scan mentioned under the sorting section, but that mentions the profiles table only, which we already join to before this change. I have a suggestion here: will raise an issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{where_clause}
"""
sql = "SELECT COUNT(*) as total_users " + sql_base
Expand All @@ -269,14 +270,22 @@ def get_users_paginate_txn(

sql = f"""
SELECT name, user_type, is_guest, admin, deactivated, shadow_banned,
displayname, avatar_url, creation_ts * 1000 as creation_ts, approved
displayname, avatar_url, creation_ts * 1000 as creation_ts, approved,
eu.user_id is not null as erased
{sql_base}
ORDER BY {order_by_column} {order}, u.name ASC
LIMIT ? OFFSET ?
"""
args += [limit, start]
txn.execute(sql, args)
users = self.db_pool.cursor_to_dict(txn)

# some of those boolean values are returned as integers when we're on SQLite
columns_to_boolify = ["erased"]
for user in users:
for column in columns_to_boolify:
user[column] = bool(user[column])

return users, count

return await self.db_pool.runInteraction(
Expand Down
35 changes: 34 additions & 1 deletion tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from synapse.rest.client import devices, login, logout, profile, register, room, sync
from synapse.rest.media.v1.filepath import MediaFilePaths
from synapse.server import HomeServer
from synapse.types import JsonDict, UserID
from synapse.types import JsonDict, UserID, create_requester
from synapse.util import Clock

from tests import unittest
Expand Down Expand Up @@ -924,6 +924,36 @@ def test_filter_out_approved(self) -> None:
self.assertEqual(1, len(non_admin_user_ids), non_admin_user_ids)
self.assertEqual(not_approved_user, non_admin_user_ids[0])

def test_erasure_status(self) -> None:
# Create a new user.
user_id = self.register_user("eraseme", "eraseme")
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

# They should appear in the list users API, marked as not erased.
channel = self.make_request(
"GET",
self.url + "?deactivated=true",
access_token=self.admin_user_tok,
)
users = {user["name"]: user for user in channel.json_body["users"]}
self.assertIs(users[user_id]["erased"], False)
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

# Deactivate that user, requesting erasure.
deactivate_account_handler = self.hs.get_deactivate_account_handler()
self.get_success(
deactivate_account_handler.deactivate_account(
user_id, erase_data=True, requester=create_requester(user_id)
)
)

# Repeat the list users query. They should now be marked as erased.
channel = self.make_request(
"GET",
self.url + "?deactivated=true",
access_token=self.admin_user_tok,
)
users = {user["name"]: user for user in channel.json_body["users"]}
self.assertIs(users[user_id]["erased"], True)
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

def _order_test(
self,
expected_user_list: List[str],
Expand Down Expand Up @@ -1195,6 +1225,7 @@ def test_deactivate_user_erase_true(self) -> None:
self.assertEqual("foo@bar.com", channel.json_body["threepids"][0]["address"])
self.assertEqual("mxc://servername/mediaid", channel.json_body["avatar_url"])
self.assertEqual("User1", channel.json_body["displayname"])
self.assertFalse(channel.json_body["erased"])

# Deactivate and erase user
channel = self.make_request(
Expand All @@ -1219,6 +1250,7 @@ def test_deactivate_user_erase_true(self) -> None:
self.assertEqual(0, len(channel.json_body["threepids"]))
self.assertIsNone(channel.json_body["avatar_url"])
self.assertIsNone(channel.json_body["displayname"])
self.assertTrue(channel.json_body["erased"])

self._is_erased("@user:test", True)

Expand Down Expand Up @@ -2757,6 +2789,7 @@ def _check_fields(self, content: JsonDict) -> None:
self.assertIn("avatar_url", content)
self.assertIn("admin", content)
self.assertIn("deactivated", content)
self.assertIn("erased", content)
self.assertIn("shadow_banned", content)
self.assertIn("creation_ts", content)
self.assertIn("appservice_id", content)
Expand Down