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

Add not_user_type param to the list accounts admin API #15844

Merged
merged 1 commit into from Jul 4, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15844.feature
@@ -0,0 +1 @@
Add `not_user_type` param to the list accounts admin API.
3 changes: 3 additions & 0 deletions docs/admin_api/user_admin_api.md
Expand Up @@ -242,6 +242,9 @@ The following parameters should be set in the URL:

- `dir` - Direction of media order. Either `f` for forwards or `b` for backwards.
Setting this value to `b` will reverse the above sort order. Defaults to `f`.
- `not_user_type` - Exclude certain user types, such as bot users, from the request.
Can be provided multiple times. Possible values are `bot`, `support` or "empty string".
"empty string" here means to exclude users without a type.

Caution. The database only has indexes on the columns `name` and `creation_ts`.
This means that if a different sort order is used (`is_guest`, `admin`,
Expand Down
9 changes: 9 additions & 0 deletions synapse/rest/admin/users.py
Expand Up @@ -28,6 +28,7 @@
parse_integer,
parse_json_object_from_request,
parse_string,
parse_strings_from_args,
)
from synapse.http.site import SynapseRequest
from synapse.rest.admin._base import (
Expand Down Expand Up @@ -64,6 +65,9 @@ class UsersRestServletV2(RestServlet):
The parameter `guests` can be used to exclude guest users.
The parameter `deactivated` can be used to include deactivated users.
The parameter `order_by` can be used to order the result.
The parameter `not_user_type` can be used to exclude certain user types.
Possible values are `bot`, `support` or "empty string".
"empty string" here means to exclude users without a type.
"""

def __init__(self, hs: "HomeServer"):
Expand Down Expand Up @@ -131,6 +135,10 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:

direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS)

# twisted.web.server.Request.args is incorrectly defined as Optional[Any]
args: Dict[bytes, List[bytes]] = request.args # type: ignore
not_user_types = parse_strings_from_args(args, "not_user_type")

users, total = await self.store.get_users_paginate(
start,
limit,
Expand All @@ -141,6 +149,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
order_by,
direction,
approved,
not_user_types,
)

# If support for MSC3866 is not enabled, don't show the approval flag.
Expand Down
37 changes: 37 additions & 0 deletions synapse/storage/databases/main/__init__.py
Expand Up @@ -19,6 +19,7 @@

from synapse.api.constants import Direction
from synapse.config.homeserver import HomeServerConfig
from synapse.storage._base import make_in_list_sql_clause
from synapse.storage.database import (
DatabasePool,
LoggingDatabaseConnection,
Expand Down Expand Up @@ -170,6 +171,7 @@ async def get_users_paginate(
order_by: str = UserSortOrder.NAME.value,
direction: Direction = Direction.FORWARDS,
approved: bool = True,
not_user_types: Optional[List[str]] = None,
) -> Tuple[List[JsonDict], int]:
"""Function to retrieve a paginated list of users from
users list. This will return a json list of users and the
Expand All @@ -185,6 +187,7 @@ async def get_users_paginate(
order_by: the sort order of the returned list
direction: sort ascending or descending
approved: whether to include approved users
not_user_types: list of user types to exclude
Returns:
A tuple of a list of mappings from user to information and a count of total users.
"""
Expand Down Expand Up @@ -222,6 +225,40 @@ def get_users_paginate_txn(
# be already existing users that we consider as already approved.
filters.append("approved IS FALSE")

if not_user_types:
if len(not_user_types) == 1 and not_user_types[0] == "":
# Only exclude NULL type users
filters.append("user_type IS NOT NULL")
else:
not_user_types_has_empty = False
not_user_types_without_empty = []

for not_user_type in not_user_types:
if not_user_type == "":
not_user_types_has_empty = True
else:
not_user_types_without_empty.append(not_user_type)

not_user_type_clause, not_user_type_args = make_in_list_sql_clause(
self.database_engine,
"u.user_type",
not_user_types_without_empty,
)

if not_user_types_has_empty:
# NULL values should be excluded.
# They evaluate to false > nothing to do here.
filters.append("NOT %s" % (not_user_type_clause))
else:
# NULL values should *not* be excluded.
# Add a special predicate to the query.
filters.append(
"(NOT %s OR %s IS NULL)"
% (not_user_type_clause, "u.user_type")
)

args.extend(not_user_type_args)

where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else ""

sql_base = f"""
Expand Down
78 changes: 78 additions & 0 deletions tests/rest/admin/test_user.py
Expand Up @@ -933,6 +933,84 @@ 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_filter_not_user_types(self) -> None:
"""Tests that the endpoint handles the not_user_types param"""

regular_user_id = self.register_user("normalo", "secret")

bot_user_id = self.register_user("robo", "secret")
self.make_request(
"PUT",
"/_synapse/admin/v2/users/" + urllib.parse.quote(bot_user_id),
{"user_type": UserTypes.BOT},
access_token=self.admin_user_tok,
)

support_user_id = self.register_user("foo", "secret")
self.make_request(
"PUT",
"/_synapse/admin/v2/users/" + urllib.parse.quote(support_user_id),
{"user_type": UserTypes.SUPPORT},
access_token=self.admin_user_tok,
)

def test_user_type(
expected_user_ids: List[str], not_user_types: Optional[List[str]] = None
) -> None:
"""Runs a test for the not_user_types param
Args:
expected_user_ids: Ids of the users that are expected to be returned
not_user_types: List of values for the not_user_types param
"""

user_type_query = ""

if not_user_types is not None:
user_type_query = "&".join(
[f"not_user_type={u}" for u in not_user_types]
)

test_url = f"{self.url}?{user_type_query}"
channel = self.make_request(
"GET",
test_url,
access_token=self.admin_user_tok,
)

self.assertEqual(200, channel.code)
self.assertEqual(channel.json_body["total"], len(expected_user_ids))
self.assertEqual(
expected_user_ids,
[u["name"] for u in channel.json_body["users"]],
)

# Request without user_types → all users expected
test_user_type([self.admin_user, support_user_id, regular_user_id, bot_user_id])

# Request and exclude bot users
test_user_type(
[self.admin_user, support_user_id, regular_user_id],
not_user_types=[UserTypes.BOT],
)

# Request and exclude bot and support users
test_user_type(
[self.admin_user, regular_user_id],
not_user_types=[UserTypes.BOT, UserTypes.SUPPORT],
)

# Request and exclude empty user types → only expected the bot and support user
test_user_type([support_user_id, bot_user_id], not_user_types=[""])

weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
# Request and exclude empty user types and bots → only expected the support user
test_user_type([support_user_id], not_user_types=["", UserTypes.BOT])

# Request and exclude a custom type (neither service nor bot) → expect all users
test_user_type(
[self.admin_user, support_user_id, regular_user_id, bot_user_id],
not_user_types=["custom"],
)

def test_erasure_status(self) -> None:
# Create a new user.
user_id = self.register_user("eraseme", "eraseme")
Expand Down