feat!: Change admin_path to admin_validator

Author: mzaniolo

README.md

@@ -113,7 +113,7 @@ If `notify_on_registration` is set then `notify_on_registration.url` will be cal
 
 `expose_metadata_resource` must be an object with `name` field. The object will be exposed at `/_famedly/login/{expose_metadata_resource.name}`.
 
-When `registration_enabled` is `false`, new users cannot register through this OAuth flow—except for identities listed under `sysadmins`. A `sysadmins` entry matches when both `external_id` and `issuer` equal the token's resolved `sub` and `iss`.
+When `registration_enabled` is `false`, new users cannot register through this OAuth flow—except for identities listed under `sysadmins`. A `sysadmins` entry matches when both `external_id` and `issuer` equal the token's resolved `sub` and `iss`. On **first registration**, a matching sysadmin is created as a Synapse server admin, in addition to any `admin_validator` outcome.
 
 Example:
 
@@ -131,7 +131,7 @@ oauth:
 
 ### OAuthSysadmin
 
-Each object in `oauth.sysadmins` identifies one IdP subject that may register even when `registration_enabled` is `false`.
+Each object in `oauth.sysadmins` identifies one IdP subject that may register even when `registration_enabled` is `false`, and who is registered as a server admin when first created via this flow.
 
 | Parameter     | Description |
 | ------------- | ----------- |
@@ -150,7 +150,7 @@ Each object in `oauth.sysadmins` identifies one IdP subject that may register ev
 | `user_id_path`     | [`Path`](#path) (optional)                                |
 | `fq_uid_path`      | [`Path`](#path) (optional)                                |
 | `displayname_path` | [`Path`](#path) (optional)                                |
-| `admin_path`       | [`PathList`](#pathlist) (optional)                        |
+| `admin_validator`  | [`Validator`](#validator) (optional)                      |
 | `email_path`       | [`Path`](#path) (optional)                                |
 | `required_scopes`  | Space separated string or a list of strings (optional)    |
 | `jwk_set`          | [JWKSet](https://datatracker.ietf.org/doc/html/rfc7517#section-5) or [JWK](https://datatracker.ietf.org/doc/html/rfc7517#section-4) (optional) |
@@ -159,6 +159,8 @@ Each object in `oauth.sysadmins` identifies one IdP subject that may register ev
 
 Either `jwk_set` or `jwk_file` or `jwks_endpoint` must be specified.
 
+If `admin_validator` is set, it is run against the decoded JWT claims when registering a new user. If it returns true, the user is created as a server admin.
+
 ### IntrospectionValidationConfig
 
 [RFC 7662 - OAuth 2.0 Token Introspection](https://datatracker.ietf.org/doc/html/rfc7662)
@@ -172,10 +174,12 @@ Either `jwk_set` or `jwk_file` or `jwks_endpoint` must be specified.
 | `user_id_path`     | [`Path`](#path) (optional)                                |
 | `fq_uid_path`      | [`Path`](#path) (optional)                                |
 | `displayname_path` | [`Path`](#path) (optional)                                |
-| `admin_path`       | [`PathList`](#pathlist) (optional)                        |
+| `admin_validator`  | [`Validator`](#validator) (optional)                      |
 | `email_path`       | [`Path`](#path) (optional)                                |
 | `required_scopes`  | Space separated string or a list of strings (optional)    |
 
+If `admin_validator` is set, it is run against the introspection JSON when registering a new user; a true result creates the user as admin.
+
 Keep in mind, that default validator will always pass. According to the [spec](https://datatracker.ietf.org/doc/html/rfc7662), you probably want at least
 
 ```yaml

synapse_token_authenticator/config.py

@@ -58,7 +58,6 @@ def __init__(self, other: dict):
         if config := other.get("oauth"):
 
             Path: TypeAlias = Union[str, List[str]]
-            PathList: TypeAlias = Union[Path, List[List[str]]]
 
             @dataclass
             class JwtValidationConfig:
@@ -68,7 +67,7 @@ class JwtValidationConfig:
                 user_id_path: Path | None = None
                 fq_uid_path: Path | None = None
                 displayname_path: Path | None = None
-                admin_path: PathList | None = None
+                admin_validator: Validator | None = None
                 email_path: Path | None = None
                 required_scopes: str | List[str] | None = None
                 jwk_set: JWKSet | JWK | None = None
@@ -79,6 +78,9 @@ def __post_init__(self):
                     if not isinstance(self.validator, Exist):
                         self.validator = parse_validator(self.validator)
 
+                    if self.admin_validator is not None:
+                        self.admin_validator = parse_validator(self.admin_validator)
+
                     if self.jwk_set and ("keys" in self.jwk_set):
                         self.jwk_set = JWKSet(**self.jwk_set)
                     elif self.jwk_set:
@@ -98,14 +100,17 @@ class IntrospectionValidationConfig:
                 user_id_path: Path | None = None
                 fq_uid_path: Path | None = None
                 displayname_path: Path | None = None
-                admin_path: PathList | None = None
+                admin_validator: Validator | None = None
                 email_path: Path | None = None
                 required_scopes: str | List[str] | None = None
 
                 def __post_init__(self):
                     if not isinstance(self.validator, Exist):
                         self.validator = parse_validator(self.validator)
 
+                    if self.admin_validator is not None:
+                        self.admin_validator = parse_validator(self.admin_validator)
+
                     if not isinstance(self.auth, NoAuth):
                         self.auth = parse_auth(self.auth)
 
@@ -138,9 +143,11 @@ class OAuthConfig:
                 def __post_init__(self):
                     if self.sysadmins:
                         self.sysadmins = [
-                            OAuthSysadminConfig(**entry)
-                            if isinstance(entry, dict)
-                            else entry
+                            (
+                                OAuthSysadminConfig(**entry)
+                                if isinstance(entry, dict)
+                                else entry
+                            )
                             for entry in self.sysadmins
                         ]
                     if self.notify_on_registration:

synapse_token_authenticator/token_authenticator.py

@@ -497,18 +497,22 @@ def get_from_set(set_):
             return None
 
         try:
-            get_admin_mb = if_not_none(lambda x: x.admin_path)
+            get_admin_mb = if_not_none(lambda x: x.admin_validator)
+
+            def admin_result(claims: dict, validation_cfg) -> bool | None:
+                v = get_admin_mb(validation_cfg)
+                return v.validate(claims) if v is not None else None
+
             admin = all_list_elems_are_equal_return_the_elem(
                 [
-                    get_from_set(jwt_claims)(get_admin_mb(config.jwt_validation)),
-                    get_from_set(introspection_claims)(
-                        get_admin_mb(config.introspection_validation)
-                    ),
+                    admin_result(jwt_claims, config.jwt_validation),
+                    admin_result(introspection_claims, config.introspection_validation),
                 ]
             )
         except Exception as e:
             logger.info(e)
             return None
+        admin = bool(admin)
 
         try:
             get_email_mb = if_not_none(lambda x: x.email_path)
@@ -554,15 +558,14 @@ def get_from_set(set_):
 
         user_exists = await self.api.check_user_exists(fully_qualified_uid)
 
-        registration_allowed = config.registration_enabled or (
-            config.sysadmins is not None
-            and any(
-                str(s.external_id) == str(external_id)
-                and str(s.issuer) == str(auth_provider)
-                for s in config.sysadmins
-            )
+        oauth_sysadmin_match = config.sysadmins is not None and any(
+            str(s.external_id) == str(external_id)
+            and str(s.issuer) == str(auth_provider)
+            for s in config.sysadmins
         )
 
+        registration_allowed = config.registration_enabled or oauth_sysadmin_match
+
         if not user_exists and not registration_allowed:
             logger.info("User doesn't exist and registration is disabled")
             return None
@@ -587,9 +590,10 @@ def get_from_set(set_):
                     if config.notify_on_registration.interrupt_on_error:
                         return None
 
-            user_id = await self.api.register_user(localpart, admin=bool(admin))
+            register_as_admin = bool(admin) or oauth_sysadmin_match
+            user_id = await self.api.register_user(localpart, admin=register_as_admin)
             logger.debug(
-                f"User '{localpart}' created as '{'Admin' if bool(admin) else 'User'}'"
+                f"User '{localpart}' created as '{'Admin' if register_as_admin else 'User'}'"
             )
 
             if email:

tests/test_oauth.py

@@ -149,12 +149,16 @@ async def test_valid_login_registration_disabled(self, *args):
         "synapse.module_api.ModuleApi.record_user_external_id",
         new_callable=mock.AsyncMock,
     )
-    async def test_sysadmin_registration_allowed_when_jwt_sub_and_iss_match(self, *args):
+    @mock.patch("synapse.module_api.ModuleApi.register_user")
+    async def test_sysadmin_registration_allowed_when_jwt_sub_and_iss_match(
+        self, register_user_mock, *args
+    ):
         """Registration is allowed when ``sysadmins`` matches JWT ``sub`` / ``iss``."""
         token = get_jwt_token("aliceid", claims=default_claims)
         result = await self.hs.mockmod.check_oauth(
             "alice", "com.famedly.login.token.oauth", {"token": token}
         )
+        register_user_mock.assert_called_with("alice", admin=True)
         self.assertEqual(result[0], "@alice:example.test")
 
     config_for_jwt_reg_disabled_sysadmin_wrong_sub = deepcopy(
@@ -251,8 +255,8 @@ async def test_fetch_jwks(self, *args):
 
     config_for_jwt_admin_path = deepcopy(config_for_jwt)
     config_for_jwt_admin_path["modules"][0]["config"]["oauth"]["jwt_validation"][
-        "admin_path"
-    ] = ["roles", "Admin"]
+        "admin_validator"
+    ] = {"type": "in", "path": ["roles", "Admin"]}
     config_for_jwt_admin_path["modules"][0]["config"]["oauth"][
         "registration_enabled"
     ] = True
@@ -278,8 +282,14 @@ async def test_login_register_admin(self, register_user_mock, *args):
 
     config_for_jwt_admin_paths = deepcopy(config_for_jwt)
     config_for_jwt_admin_paths["modules"][0]["config"]["oauth"]["jwt_validation"][
-        "admin_path"
-    ] = [["roles", "NotAdmin"], ["roles", "MatrixAdmin"]]
+        "admin_validator"
+    ] = {
+        "type": "any_of",
+        "validators": [
+            {"type": "in", "path": ["roles", "NotAdmin"]},
+            {"type": "in", "path": ["roles", "MatrixAdmin"]},
+        ],
+    }
     config_for_jwt_admin_paths["modules"][0]["config"]["oauth"][
         "registration_enabled"
     ] = True
@@ -305,8 +315,8 @@ async def test_login_register_multiple_admin_paths(self, register_user_mock, *ar
 
     config_for_jwt_admin_path_wrong = deepcopy(config_for_jwt_admin_path)
     config_for_jwt_admin_path_wrong["modules"][0]["config"]["oauth"]["jwt_validation"][
-        "admin_path"
-    ] = ["roles", "SomethingAdmin"]
+        "admin_validator"
+    ] = {"type": "in", "path": ["roles", "SomethingAdmin"]}
 
     @synapsetest.override_config(config_for_jwt_admin_path_wrong)
     @mock.patch("synapse.module_api.ModuleApi.check_user_exists", return_value=False)
@@ -467,14 +477,16 @@ async def test_login_check_external_id_disabled(self, *args):
         "synapse.module_api.ModuleApi.record_user_external_id",
         new_callable=mock.AsyncMock,
     )
+    @mock.patch("synapse.module_api.ModuleApi.register_user")
     async def test_sysadmin_registration_allowed_when_introspection_sub_and_iss_match(
-        self, *args
+        self, register_user_mock, *args
     ):
         """``sysadmins`` can match introspection ``sub`` / ``iss`` when JWT is not validated."""
         token = get_jwt_token("aliceid", claims=default_claims)
         result = await self.hs.mockmod.check_oauth(
             "alice", "com.famedly.login.token.oauth", {"token": token}
         )
+        register_user_mock.assert_called_with("alice", admin=True)
         self.assertEqual(result[0], "@alice:example.test")
 
     config_for_introspection_reg_disabled_sysadmin_wrong = deepcopy(
@@ -577,7 +589,7 @@ async def test_login_introspection_invalid_scope(self, *args):
     config_for_introspection_admin_path = deepcopy(config_for_introspection)
     config_for_introspection_admin_path["modules"][0]["config"]["oauth"][
         "introspection_validation"
-    ]["admin_path"] = ["roles", "Admin"]
+    ]["admin_validator"] = {"type": "in", "path": ["roles", "Admin"]}
 
     @synapsetest.override_config(config_for_introspection_admin_path)
     @mock.patch(
@@ -600,7 +612,13 @@ async def test_login_introspection_register_admin(self, register_user_mock, *arg
     config_for_introspection_admin_paths = deepcopy(config_for_introspection)
     config_for_introspection_admin_paths["modules"][0]["config"]["oauth"][
         "introspection_validation"
-    ]["admin_path"] = [["roles", "AnotherAdmin"], ["roles", "MatrixAdmin"]]
+    ]["admin_validator"] = {
+        "type": "any_of",
+        "validators": [
+            {"type": "in", "path": ["roles", "AnotherAdmin"]},
+            {"type": "in", "path": ["roles", "MatrixAdmin"]},
+        ],
+    }
 
     @synapsetest.override_config(config_for_introspection_admin_paths)
     @mock.patch(