Iterate

Author: hughns

changelog.d/18456.feature

@@ -0,0 +1 @@
+Support configuration of default and extra user types.

docs/admin_api/user_admin_api.md

@@ -163,7 +163,8 @@ Body parameters:
 - `locked` - **bool**, optional. If unspecified, locked state will be left unchanged.
 - `user_type` - **string** or null, optional. If not provided, the user type will be
   not be changed. If `null` is given, the user type will be cleared.
-  Other allowed options are: `bot` and `support`. # XXXTODO
+  Other allowed options are: `bot` and `support` and any extra values defined in the homserver
+  [configuration](../usage/configuration/config_documentation.md#user_types).
 
 ## List Accounts
 ### List Accounts (V2)

docs/usage/configuration/config_documentation.md

@@ -834,6 +834,24 @@ Example configuration:
 ```yaml
 max_event_delay_duration: 24h
 ```
+---
+### `user_types`
+
+Configuration settings related to the user types feature.
+
+This setting has the following sub-options:
+* `default_user_type`: The default user type to use for registering new users when no value has been specified.
+  Defaults to none.
+* `extra_user_types`: Array of additional user types to allow. These are treated as real users. Defaults to [].
+
+Example configuration:
+```yaml
+user_types:
+    default_user_type: "custom"
+    extra_user_types:
+      - "custom"
+      - "custom2"
+```
 
 ## Homeserver blocking
 Useful options for Synapse admins.

synapse/config/user_types.py

@@ -17,7 +17,7 @@
 from synapse.api.constants import UserTypes
 from synapse.types import JsonDict
 
-from ._base import Config
+from ._base import Config, ConfigError
 
 
 class UserTypesConfig(Config):
@@ -30,10 +30,15 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
             "default_user_type", None
         )
         self.extra_user_types: List[str] = user_types.get("extra_user_types", [])
-        # XXXTODO: provide a way to set which extra user types should be treated as "real" users
 
         all_user_types: List[str] = []
         all_user_types.extend(UserTypes.ALL_USER_TYPES)
         all_user_types.extend(self.extra_user_types)
 
         self.all_user_types = all_user_types
+
+        if self.default_user_type is not None:
+            if self.default_user_type not in all_user_types:
+                raise ConfigError(
+                    f"Default user type {self.default_user_type} is not in the list of all user types: {all_user_types}"
+                )

synapse/storage/databases/main/registration.py

@@ -685,8 +685,7 @@ def is_real_user_txn(self, txn: LoggingTransaction, user_id: str) -> bool:
             retcol="user_type",
             allow_none=True,
         )
-        # XXXTODO
-        return res is None
+        return res is None or res not in [UserTypes.BOT, UserTypes.SUPPORT]
 
     def is_support_user_txn(self, txn: LoggingTransaction, user_id: str) -> bool:
         res = self.db_pool.simple_select_one_onecol_txn(
@@ -962,11 +961,12 @@ def _count_users(txn: LoggingTransaction) -> int:
         return await self.db_pool.runInteraction("count_users", _count_users)
 
     async def count_real_users(self) -> int:
-        """Counts all users without a special user_type registered on the homeserver."""
+        """Counts all users without the bot or support user_types registered on the homeserver."""
 
-        # XXXTODO
         def _count_users(txn: LoggingTransaction) -> int:
-            txn.execute("SELECT COUNT(*) FROM users where user_type is null")
+            txn.execute(
+                f"SELECT COUNT(*) FROM users WHERE user_type IS NULL OR user_type NOT IN ('{UserTypes.BOT}', '{UserTypes.SUPPORT}')"
+            )
             row = txn.fetchone()
             assert row is not None
             return row[0]

tests/handlers/test_register.py

@@ -738,6 +738,41 @@ def test_spam_checker_receives_sso_type(self) -> None:
             self.handler.register_user(localpart="bobflimflob", auth_provider_id="saml")
         )
 
+    def test_register_default_user_type(self) -> None:
+        """Test that the default user type is none when registering a user."""
+        user_id = self.get_success(self.handler.register_user(localpart="user"))
+        user_info = self.get_success(self.store.get_user_by_id(user_id))
+        assert user_info is not None
+        self.assertEqual(user_info.user_type, None)
+
+    def test_register_extra_user_types_valid(self) -> None:
+        """
+        Test that the specified user type is set correctly when registering a user.
+        n.b. No validation is done on the user type, so this test
+        is only to ensure that the user type can be set to any value.
+        """
+        user_id = self.get_success(
+            self.handler.register_user(localpart="user", user_type="anyvalue")
+        )
+        user_info = self.get_success(self.store.get_user_by_id(user_id))
+        assert user_info is not None
+        self.assertEqual(user_info.user_type, "anyvalue")
+
+    @override_config(
+        {
+            "user_types": {
+                "extra_user_types": ["extra1", "extra2"],
+                "default_user_type": "extra1",
+            }
+        }
+    )
+    def test_register_extra_user_types_with_default(self) -> None:
+        """Test that the default_user_type in config is set correctly when registering a user."""
+        user_id = self.get_success(self.handler.register_user(localpart="user"))
+        user_info = self.get_success(self.store.get_user_by_id(user_id))
+        assert user_info is not None
+        self.assertEqual(user_info.user_type, "extra1")
+
     async def get_or_create_user(
         self,
         requester: Requester,

tests/rest/admin/test_user.py

@@ -328,6 +328,61 @@ def nonce() -> str:
         self.assertEqual(400, channel.code, msg=channel.json_body)
         self.assertEqual("Invalid user type", channel.json_body["error"])
 
+    @override_config(
+        {
+            "user_types": {
+                "extra_user_types": ["extra1", "extra2"],
+            }
+        }
+    )
+    def test_extra_user_type(self) -> None:
+        """
+        Check that the extra user type can be used when registering a user.
+        """
+
+        def nonce_mac(user_type: str) -> tuple[str, str]:
+            """
+            Get a nonce and the expected HMAC for that nonce.
+            """
+            channel = self.make_request("GET", self.url)
+            nonce = channel.json_body["nonce"]
+
+            want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1)
+            want_mac.update(
+                nonce.encode("ascii")
+                + b"\x00alice\x00abc123\x00notadmin\x00"
+                + user_type.encode("ascii")
+            )
+            want_mac_str = want_mac.hexdigest()
+
+            return nonce, want_mac_str
+
+        nonce, mac = nonce_mac("extra1")
+        # Valid user_type
+        body = {
+            "nonce": nonce,
+            "username": "alice",
+            "password": "abc123",
+            "user_type": "extra1",
+            "mac": mac,
+        }
+        channel = self.make_request("POST", self.url, body)
+        self.assertEqual(200, channel.code, msg=channel.json_body)
+
+        nonce, mac = nonce_mac("extra3")
+        # Invalid user_type
+        body = {
+            "nonce": nonce,
+            "username": "alice",
+            "password": "abc123",
+            "user_type": "extra3",
+            "mac": mac,
+        }
+        channel = self.make_request("POST", self.url, body)
+
+        self.assertEqual(400, channel.code, msg=channel.json_body)
+        self.assertEqual("Invalid user type", channel.json_body["error"])
+
     def test_displayname(self) -> None:
         """
         Test that displayname of new user is set
@@ -1186,6 +1241,80 @@ def test_user_type(
             not_user_types=["custom"],
         )
 
+    @override_config(
+        {
+            "user_types": {
+                "extra_user_types": ["extra1", "extra2"],
+            }
+        }
+    )
+    def test_filter_not_user_types_with_extra(self) -> None:
+        """Tests that the endpoint handles the not_user_types param when extra_user_types are configured"""
+
+        regular_user_id = self.register_user("normalo", "secret")
+
+        extra1_user_id = self.register_user("extra1", "secret")
+        self.make_request(
+            "PUT",
+            "/_synapse/admin/v2/users/" + urllib.parse.quote(extra1_user_id),
+            {"user_type": "extra1"},
+            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, extra1_user_id, regular_user_id])
+
+        # Request and exclude extra1 user type
+        test_user_type(
+            [self.admin_user, regular_user_id],
+            not_user_types=["extra1"],
+        )
+
+        # Request and exclude extra1 and extra2 user types
+        test_user_type(
+            [self.admin_user, regular_user_id],
+            not_user_types=["extra1", "extra2"],
+        )
+
+        # Request and exclude empty user types →  only expected the extra1 user
+        test_user_type([extra1_user_id], not_user_types=[""])
+
+        # Request and exclude an unregistered type →  expect all users
+        test_user_type(
+            [self.admin_user, extra1_user_id, regular_user_id],
+            not_user_types=["extra3"],
+        )
+
     def test_erasure_status(self) -> None:
         # Create a new user.
         user_id = self.register_user("eraseme", "eraseme")
@@ -2977,22 +3106,18 @@ def test_set_user_as_admin(self) -> None:
         self.assertEqual("@user:test", channel.json_body["name"])
         self.assertTrue(channel.json_body["admin"])
 
-    def test_set_user_type(self) -> None:
-        """
-        Test changing user type.
-        """
-
-        # Set to support type
+    def set_user_type(self, user_type: Optional[str]) -> None:
+        # Set to user_type
         channel = self.make_request(
             "PUT",
             self.url_other_user,
             access_token=self.admin_user_tok,
-            content={"user_type": UserTypes.SUPPORT},
+            content={"user_type": user_type},
         )
 
         self.assertEqual(200, channel.code, msg=channel.json_body)
         self.assertEqual("@user:test", channel.json_body["name"])
-        self.assertEqual(UserTypes.SUPPORT, channel.json_body["user_type"])
+        self.assertEqual(user_type, channel.json_body["user_type"])
 
         # Get user
         channel = self.make_request(
@@ -3003,30 +3128,44 @@ def test_set_user_type(self) -> None:
 
         self.assertEqual(200, channel.code, msg=channel.json_body)
         self.assertEqual("@user:test", channel.json_body["name"])
-        self.assertEqual(UserTypes.SUPPORT, channel.json_body["user_type"])
+        self.assertEqual(user_type, channel.json_body["user_type"])
+
+    def test_set_user_type(self) -> None:
+        """
+        Test changing user type.
+        """
+
+        # Set to support type
+        self.set_user_type(UserTypes.SUPPORT)
 
         # Change back to a regular user
-        channel = self.make_request(
-            "PUT",
-            self.url_other_user,
-            access_token=self.admin_user_tok,
-            content={"user_type": None},
-        )
+        self.set_user_type(None)
 
-        self.assertEqual(200, channel.code, msg=channel.json_body)
-        self.assertEqual("@user:test", channel.json_body["name"])
-        self.assertIsNone(channel.json_body["user_type"])
+    @override_config({"user_types": {"extra_user_types": ["extra1", "extra2"]}})
+    def test_set_user_type_with_extras(self) -> None:
+        """
+        Test changing user type with extra_user_types configured.
+        """
 
-        # Get user
+        # Check that we can still set to support type
+        self.set_user_type(UserTypes.SUPPORT)
+
+        # Check that we can set to an extra user type
+        self.set_user_type("extra2")
+
+        # Change back to a regular user
+        self.set_user_type(None)
+
+        # Try setting to invalid type
         channel = self.make_request(
-            "GET",
+            "PUT",
             self.url_other_user,
             access_token=self.admin_user_tok,
+            content={"user_type": "extra3"},
         )
 
-        self.assertEqual(200, channel.code, msg=channel.json_body)
-        self.assertEqual("@user:test", channel.json_body["name"])
-        self.assertIsNone(channel.json_body["user_type"])
+        self.assertEqual(400, channel.code, msg=channel.json_body)
+        self.assertEqual("Invalid user type", channel.json_body["error"])
 
     def test_accidental_deactivation_prevention(self) -> None:
         """