From f7bc63ef5787f20b63586bc7d8665cd2ce76c268 Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Tue, 18 Feb 2025 15:47:35 +0100
Subject: [PATCH] Make sure we advertise registration as disabled when MSC3861
 is enabled (#17661)

This has been a problem with Element Web, as it will proble /register
with an empty body, which gave this error:

```
curl -d '{}' -HContent-Type:application/json /_matrix/client/v3/register

{"errcode": "M_UNKNOWN",
 "error": "Invalid username"}
```

And Element Web would choke on it. This changes that so we reply
instead:

```
{"errcode": "M_FORBIDDEN",
 "error": "Registration has been disabled. Only m.login.application_service registrations are allowed."}
```

Also adds a test for this.

See https://github.com/element-hq/element-web/issues/27993

---------

Co-authored-by: Andrew Morgan <andrew@amorgan.xyz>
---
 changelog.d/17661.bugfix                |  1 +
 synapse/rest/client/register.py         | 12 +++++---
 tests/handlers/test_oauth_delegation.py | 37 ++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 5 deletions(-)
 create mode 100644 changelog.d/17661.bugfix

diff --git a/changelog.d/17661.bugfix b/changelog.d/17661.bugfix
new file mode 100644
index 0000000000..33881bbc6a
--- /dev/null
+++ b/changelog.d/17661.bugfix
@@ -0,0 +1 @@
+Make sure we advertise registration as disabled when MSC3861 is enabled.
diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py
index ad76f188ab..58231d2b04 100644
--- a/synapse/rest/client/register.py
+++ b/synapse/rest/client/register.py
@@ -908,6 +908,14 @@ class RegisterAppServiceOnlyRestServlet(RestServlet):
 
         await self.ratelimiter.ratelimit(None, client_addr, update=False)
 
+        # Allow only ASes to use this API.
+        if body.get("type") != APP_SERVICE_REGISTRATION_TYPE:
+            raise SynapseError(
+                403,
+                "Registration has been disabled. Only m.login.application_service registrations are allowed.",
+                errcode=Codes.FORBIDDEN,
+            )
+
         kind = parse_string(request, "kind", default="user")
 
         if kind == "guest":
@@ -923,10 +931,6 @@ class RegisterAppServiceOnlyRestServlet(RestServlet):
         if not isinstance(desired_username, str) or len(desired_username) > 512:
             raise SynapseError(400, "Invalid username")
 
-        # Allow only ASes to use this API.
-        if body.get("type") != APP_SERVICE_REGISTRATION_TYPE:
-            raise SynapseError(403, "Non-application service registration type")
-
         if not self.auth.has_access_token(request):
             raise SynapseError(
                 400,
diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py
index ba2f8ff510..b9d66a6c52 100644
--- a/tests/handlers/test_oauth_delegation.py
+++ b/tests/handlers/test_oauth_delegation.py
@@ -43,6 +43,7 @@ from synapse.api.errors import (
     OAuthInsufficientScopeError,
     SynapseError,
 )
+from synapse.appservice import ApplicationService
 from synapse.http.site import SynapseRequest
 from synapse.rest import admin
 from synapse.rest.client import account, devices, keys, login, logout, register
@@ -575,6 +576,16 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             channel.json_body["errcode"], Codes.UNRECOGNIZED, channel.json_body
         )
 
+    def expect_forbidden(
+        self, method: str, path: str, content: Union[bytes, str, JsonDict] = ""
+    ) -> None:
+        channel = self.make_request(method, path, content)
+
+        self.assertEqual(channel.code, 403, channel.json_body)
+        self.assertEqual(
+            channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body
+        )
+
     def test_uia_endpoints(self) -> None:
         """Test that endpoints that were removed in MSC2964 are no longer available."""
 
@@ -629,11 +640,35 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
 
     def test_registration_endpoints_removed(self) -> None:
         """Test that registration endpoints that were removed in MSC2964 are no longer available."""
+        appservice = ApplicationService(
+            token="i_am_an_app_service",
+            id="1234",
+            namespaces={"users": [{"regex": r"@alice:.+", "exclusive": True}]},
+            sender="@as_main:test",
+        )
+
+        self.hs.get_datastores().main.services_cache = [appservice]
         self.expect_unrecognized(
             "GET", "/_matrix/client/v1/register/m.login.registration_token/validity"
         )
+
+        # Registration is disabled
+        self.expect_forbidden(
+            "POST",
+            "/_matrix/client/v3/register",
+            {"username": "alice", "password": "hunter2"},
+        )
+
         # This is still available for AS registrations
-        # self.expect_unrecognized("POST", "/_matrix/client/v3/register")
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/register",
+            {"username": "alice", "type": "m.login.application_service"},
+            shorthand=False,
+            access_token="i_am_an_app_service",
+        )
+        self.assertEqual(channel.code, 200, channel.json_body)
+
         self.expect_unrecognized("GET", "/_matrix/client/v3/register/available")
         self.expect_unrecognized(
             "POST", "/_matrix/client/v3/register/email/requestToken"
-- 
GitLab