lint

modules/restricted-guests/synapse/README.md

@@ -47,11 +47,11 @@ options in order to give the module access to [MAS' Admin
 API](https://element-hq.github.io/matrix-authentication-service/topics/admin-api.html).
 
 - `mas` - optional configuration for Matrix Authentication Service (MAS). When set, the module creates users via MAS' admin API.
-  - `admin_api_base_url` - Base URL for MAS' admin API (e.g. `https://mas.example.org`). Trailing slashes will be automatically stripped.
+    - `admin_api_base_url` - Base URL for MAS' admin API (e.g. `https://mas.example.org`). Trailing slashes will be automatically stripped.
-  - `oauth_base_url` - Base URL for MAS' OAuth endpoints (defaults to `admin_api_base_url` if not set). Trailing slashes will be automatically stripped.
+    - `oauth_base_url` - Base URL for MAS' OAuth endpoints (defaults to `admin_api_base_url` if not set). Trailing slashes will be automatically stripped.
-  - `client_id` - client ID for the automated tool. Must be a valid [ULID](https://github.com/ulid/spec). Generate one [here](https://ulidtools.com/).
+    - `client_id` - client ID for the automated tool. Must be a valid [ULID](https://github.com/ulid/spec). Generate one [here](https://ulidtools.com/).
-  - `client_secret` - client secret for the automated tool. Ideally long and cryptographically secure. Keep it a secret!
+    - `client_secret` - client secret for the automated tool. Ideally long and cryptographically secure. Keep it a secret!
-  - `client_secret_filepath` - path to a plaintext file containing the client secret. If set, this is used instead of `client_secret`.
+    - `client_secret_filepath` - path to a plaintext file containing the client secret. If set, this is used instead of `client_secret`.
 
 Example configuration:
 
@@ -81,19 +81,19 @@ Then, add the following to your MAS config file:
 
 ```yaml
 policy:
-  data:
+    data:
-    admin_clients:
+        admin_clients:
-      - 000000000000000000000G0EST
+            - 000000000000000000000G0EST
 
 # ...
 
 clients:
-  # The `client_id` must be a valid ULID https://github.com/ulid/spec
+    # The `client_id` must be a valid ULID https://github.com/ulid/spec
-  # Generate ULID's easily at: https://ulidtools.com/
+    # Generate ULID's easily at: https://ulidtools.com/
-  - client_id: 000000000000000000000G0EST
+    - client_id: 000000000000000000000G0EST
-    # The guest module uses the client_secret_basic authentication method.
+      # The guest module uses the client_secret_basic authentication method.
-    client_auth_method: client_secret_basic
+      client_auth_method: client_secret_basic
-    client_secret: your-client-secret
+      client_secret: your-client-secret
 ```
 
 ## Production installation

modules/restricted-guests/synapse/synapse_guest_module/guest_module.py

@@ -24,9 +24,9 @@ from synapse.module_api.errors import ConfigError
 from synapse.types import UserID
 
 from synapse_guest_module.config import GuestModuleConfig, MasConfig
-from synapse_guest_module.mas_admin_client import MasAdminClient
 from synapse_guest_module.guest_registration_servlet import GuestRegistrationServlet
 from synapse_guest_module.guest_user_reaper import GuestUserReaper
+from synapse_guest_module.mas_admin_client import MasAdminClient
 
 logger = logging.getLogger("synapse.contrib." + __name__)
 
@@ -106,22 +106,30 @@ class GuestModule:
                 raise ConfigError("Config option 'mas' must be an object")
 
             admin_api_base_url = mas_config.get("admin_api_base_url")
-            if not isinstance(admin_api_base_url, str) or len(admin_api_base_url.strip()) == 0:
+            if (
-                raise ConfigError("Config option 'mas.admin_api_base_url' is required and must be a string")
+                not isinstance(admin_api_base_url, str)
+                or len(admin_api_base_url.strip()) == 0
+            ):
+                raise ConfigError(
+                    "Config option 'mas.admin_api_base_url' is required and must be a string"
+                )
 
             oauth_base_url = mas_config.get("oauth_base_url", admin_api_base_url)
             if not isinstance(oauth_base_url, str) or len(oauth_base_url.strip()) == 0:
-                raise ConfigError(
+                raise ConfigError("Config option 'mas.oauth_base_url' must be a string")
-                    "Config option 'mas.oauth_base_url' must be a string"
-                )
 
             client_id = mas_config.get("client_id")
             if not isinstance(client_id, str) or len(client_id.strip()) == 0:
-                raise ConfigError("Config option 'mas.client_id' is required and must be a string")
+                raise ConfigError(
+                    "Config option 'mas.client_id' is required and must be a string"
+                )
 
             client_secret = mas_config.get("client_secret")
             if client_secret is not None:
-                if not isinstance(client_secret, str) or len(client_secret.strip()) == 0:
+                if (
+                    not isinstance(client_secret, str)
+                    or len(client_secret.strip()) == 0
+                ):
                     raise ConfigError(
                         "Config option 'mas.client_secret' must be a string"
                     )
@@ -129,7 +137,10 @@ class GuestModule:
 
             client_secret_filepath = mas_config.get("client_secret_filepath")
             if client_secret_filepath is not None:
-                if not isinstance(client_secret_filepath, str) or len(client_secret_filepath.strip()) == 0:
+                if (
+                    not isinstance(client_secret_filepath, str)
+                    or len(client_secret_filepath.strip()) == 0
+                ):
                     raise ConfigError(
                         "Config option 'mas.client_secret_filepath' must be a string"
                     )

modules/restricted-guests/synapse/synapse_guest_module/guest_registration_servlet.py

@@ -87,9 +87,7 @@ class GuestRegistrationServlet(DirectServeJsonResource):
                     localpart, displayname + self._config.display_name_suffix
                 )
 
-                device_id, access_token, _, _ = await self._api.register_device(
+                device_id, access_token, _, _ = await self._api.register_device(user_id)
-                    user_id
-                )
             else:
                 logger.info("Registering MAS guest user with username '%s'", localpart)
                 mas_user_id = await self._mas_admin_client.create_user(localpart)
@@ -112,10 +110,11 @@ class GuestRegistrationServlet(DirectServeJsonResource):
                     if self._config.enable_user_reaper
                     else 0
                 )
-                device_id, access_token = (
+                (
-                    await self._mas_admin_client.create_personal_session(
+                    device_id,
-                        mas_user_id, expires_in
+                    access_token,
-                    )
+                ) = await self._mas_admin_client.create_personal_session(
+                    mas_user_id, expires_in
                 )
 
             logger.debug("Registered user '%s'", user_id)
@@ -145,6 +144,4 @@ class GuestRegistrationServlet(DirectServeJsonResource):
                 },
             )
 
-        await self._api.run_db_interaction(
+        await self._api.run_db_interaction("guest_module_store_mas_user", store_user)
-            "guest_module_store_mas_user", store_user
-        )

modules/restricted-guests/synapse/synapse_guest_module/guest_user_reaper.py

@@ -106,6 +106,12 @@ class GuestUserReaper:
                     logger.error('Failed to delete user "%s": %s', user_id, e)
 
     async def _deactivate_expired_mas_users(self) -> None:
+        """Deactivate all MAS users that are older than the specified expiration
+        interval. This uses the MAS admin API to disable the user.
+        """
+
+        assert self._mas_admin_client is not None
+
         if self._mas_tables_ready is not None:
             await self._mas_tables_ready.wait()
 

modules/restricted-guests/synapse/synapse_guest_module/mas_admin_client.py

@@ -41,7 +41,7 @@ class MasAdminClient:
             headers={"Authorization": [f"Bearer {token}"]},
         )
 
-        mas_user_id = response.get("data", {}).get("id")
+        mas_user_id: str = response.get("data", {}).get("id")
         if mas_user_id is None or not isinstance(mas_user_id, str):
             raise ValueError("MAS user creation response missing `data.id` field")
 
@@ -141,13 +141,13 @@ class MasAdminClient:
         self, url: str, data: Dict[str, str], headers: Dict[str, Any]
     ) -> Any:
         http_client = self._api.http_client
-        post_urlencoded = getattr(http_client, "post_urlencoded_get_json", None)
+        post_urlencoded: Optional[Awaitable[Any]] = getattr(
-        if callable(post_urlencoded):
+            http_client, "post_urlencoded_get_json", None
+        )
+        if post_urlencoded is not None and callable(post_urlencoded):
             return await post_urlencoded(url, data, headers=headers)
 
-        logger.debug(
+        logger.debug("MAS client falling back to post_json_get_json for %s", url)
-            "MAS client falling back to post_json_get_json for %s", url
-        )
         return await http_client.post_json_get_json(
             uri=url, post_json=data, headers=headers
         )