deactivated flag refactored to filter deactivated users. (#16874)

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

changelog.d/16874.feature

@@ -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.

docs/admin_api/user_admin_api.md

@@ -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.
@@ -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.

synapse/rest/admin/__init__.py

@@ -109,6 +109,7 @@ from synapse.rest.admin.users import (
     UserReplaceMasterCrossSigningKeyRestServlet,
     UserRestServletV2,
     UsersRestServletV2,
+    UsersRestServletV3,
     UserTokenRestServlet,
     WhoisRestServlet,
 )
@@ -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)

synapse/rest/admin/users.py

@@ -23,7 +23,7 @@ import hmac
 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
 
@@ -118,7 +118,8 @@ class UsersRestServletV2(RestServlet):
                 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")
 
@@ -182,6 +183,22 @@ class UsersRestServletV2(RestServlet):
 
         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
+
+
+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")

synapse/storage/databases/main/__init__.py

@@ -176,7 +176,7 @@ class DataStore(
         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,
@@ -232,7 +232,10 @@ class DataStore(
             if not guests:
                 filters.append("is_guest = 0")
 
-            if not deactivated:
+            if deactivated is not None:
+                if deactivated:
+                    filters.append("deactivated = 1")
+                else:
                     filters.append("deactivated = 0")
 
             if not locked:

tests/rest/admin/test_user.py

@@ -503,7 +503,7 @@ class UsersListTestCase(unittest.HomeserverTestCase):
 
         channel = self.make_request(
             "GET",
-            self.url + "?deactivated=true",
+            f"{self.url}?deactivated=true",
             {},
             access_token=self.admin_user_tok,
         )
@@ -982,6 +982,56 @@ class UsersListTestCase(unittest.HomeserverTestCase):
         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": {
@@ -1130,7 +1180,7 @@ class UsersListTestCase(unittest.HomeserverTestCase):
         # 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"]}
@@ -1194,7 +1244,7 @@ class UsersListTestCase(unittest.HomeserverTestCase):
             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"):