Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deactivated flag refactored to filter deactivated users. #16874

Merged
merged 13 commits into from
Mar 11, 2024
1 change: 1 addition & 0 deletions changelog.d/16874.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities.
14 changes: 14 additions & 0 deletions docs/admin_api/user_admin_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ Body parameters:
Other allowed options are: `bot` and `support`.

## List Accounts
### List Accounts (V2)

This API returns all local user accounts.
By default, the response is ordered by ascending user ID.
Expand Down Expand Up @@ -287,6 +288,19 @@ The following fields are returned in the JSON response body:

*Added in Synapse 1.93:* the `locked` query parameter and response field.

### List Accounts (V3)

This API returns all local user accounts (see v2). In contrast to v2, the query parameter `deactivated` is handled differently.

```
GET /_synapse/admin/v3/users
```

**Parameters**
- `deactivated` - Optional flag to filter deactivated users. If `true`, only deactivated users are returned.
If `false`, deactivated users are excluded from the query. When the flag is absent (the default),
users are not filtered by deactivation status.

## Query current sessions for a user

This API returns information about the active sessions for a specific user.
Expand Down
2 changes: 2 additions & 0 deletions synapse/rest/admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
UserReplaceMasterCrossSigningKeyRestServlet,
UserRestServletV2,
UsersRestServletV2,
UsersRestServletV3,
UserTokenRestServlet,
WhoisRestServlet,
)
Expand Down Expand Up @@ -289,6 +290,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
UserTokenRestServlet(hs).register(http_server)
UserRestServletV2(hs).register(http_server)
UsersRestServletV2(hs).register(http_server)
UsersRestServletV3(hs).register(http_server)
UserMediaStatisticsRestServlet(hs).register(http_server)
LargestRoomsStatistics(hs).register(http_server)
EventReportDetailRestServlet(hs).register(http_server)
Expand Down
21 changes: 19 additions & 2 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import logging
import secrets
from http import HTTPStatus
from typing import TYPE_CHECKING, Dict, List, Optional, Tuple
from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Union

import attr

Expand Down Expand Up @@ -118,7 +118,8 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
errcode=Codes.INVALID_PARAM,
)

deactivated = parse_boolean(request, "deactivated", default=False)
deactivated = self._parse_parameter_deactivated(request)

locked = parse_boolean(request, "locked", default=False)
admins = parse_boolean(request, "admins")

Expand Down Expand Up @@ -182,6 +183,22 @@ def _filter(a: attr.Attribute) -> bool:

return HTTPStatus.OK, ret

def _parse_parameter_deactivated(self, request: SynapseRequest) -> Optional[bool]:
"""
Return None (no filtering) if `deactivated` is `true`, otherwise return `False`
(exclude deactivated users from the results).
"""
return None if parse_boolean(request, "deactivated") else False
afechler marked this conversation as resolved.
Show resolved Hide resolved


class UsersRestServletV3(UsersRestServletV2):
PATTERNS = admin_patterns("/users$", "v3")

def _parse_parameter_deactivated(
self, request: SynapseRequest
) -> Union[bool, None]:
return parse_boolean(request, "deactivated")


class UserRestServletV2(RestServlet):
PATTERNS = admin_patterns("/users/(?P<user_id>[^/]*)$", "v2")
Expand Down
9 changes: 6 additions & 3 deletions synapse/storage/databases/main/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ async def get_users_paginate(
user_id: Optional[str] = None,
name: Optional[str] = None,
guests: bool = True,
deactivated: bool = False,
deactivated: Optional[bool] = None,
admins: Optional[bool] = None,
order_by: str = UserSortOrder.NAME.value,
direction: Direction = Direction.FORWARDS,
Expand Down Expand Up @@ -232,8 +232,11 @@ def get_users_paginate_txn(
if not guests:
filters.append("is_guest = 0")

if not deactivated:
filters.append("deactivated = 0")
if deactivated is not None:
if deactivated:
filters.append("deactivated = 1")
else:
filters.append("deactivated = 0")

if not locked:
filters.append("locked IS FALSE")
Expand Down
56 changes: 53 additions & 3 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ def test_all_users(self) -> None:

channel = self.make_request(
"GET",
self.url + "?deactivated=true",
f"{self.url}?deactivated=true",
{},
access_token=self.admin_user_tok,
)
Expand Down Expand Up @@ -982,6 +982,56 @@ def test_filter_admins(self) -> None:
self.assertEqual(1, channel.json_body["total"])
self.assertFalse(channel.json_body["users"][0]["admin"])

def test_filter_deactivated_users(self) -> None:
"""
Tests whether the various values of the query parameter `deactivated` lead to the
expected result set.
"""
users_url_v3 = self.url.replace("v2", "v3")

# Register an additional non admin user
user_id = self.register_user("user", "pass", admin=False)

# 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)
)
)

# Query all users
channel = self.make_request(
"GET",
users_url_v3,
access_token=self.admin_user_tok,
)

self.assertEqual(200, channel.code, channel.result)
self.assertEqual(2, channel.json_body["total"])

# Query deactivated users
channel = self.make_request(
"GET",
f"{users_url_v3}?deactivated=true",
access_token=self.admin_user_tok,
)

self.assertEqual(200, channel.code, channel.result)
self.assertEqual(1, channel.json_body["total"])
self.assertEqual("@user:test", channel.json_body["users"][0]["name"])

# Query non-deactivated users
channel = self.make_request(
"GET",
f"{users_url_v3}?deactivated=false",
access_token=self.admin_user_tok,
)

self.assertEqual(200, channel.code, channel.result)
self.assertEqual(1, channel.json_body["total"])
self.assertEqual("@admin:test", channel.json_body["users"][0]["name"])

@override_config(
{
"experimental_features": {
Expand Down Expand Up @@ -1130,7 +1180,7 @@ def test_erasure_status(self) -> None:
# They should appear in the list users API, marked as not erased.
channel = self.make_request(
"GET",
self.url + "?deactivated=true",
f"{self.url}?deactivated=true",
access_token=self.admin_user_tok,
)
users = {user["name"]: user for user in channel.json_body["users"]}
Expand Down Expand Up @@ -1194,7 +1244,7 @@ def _order_test(
dir: The direction of ordering to give the server
"""

url = self.url + "?deactivated=true&"
url = f"{self.url}?deactivated=true&"
if order_by is not None:
url += "order_by=%s&" % (order_by,)
if dir is not None and dir in ("b", "f"):
Expand Down
Loading