Require AppserviceRegistrationType (#9548)

This change ensures that the appservice registration behaviour follows the spec. We decided to do this for Dendrite, so it made sense to also make a PR for synapse to correct the behaviour.

5 files changed, 60 insertions, 23 deletions

diff --git a/changelog.d/9548.removal b/changelog.d/9548.removal
new file mode 100644
index 0000000000..1fb88236c6
--- /dev/null
+++ b/changelog.d/9548.removal
@@ -0,0 +1 @@
+Make `/_matrix/client/r0/register` expect a type of `m.login.application_service` when an Application Service registers a user, to align with [the relevant spec](https://spec.matrix.org/unstable/application-service-api/#server-admin-style-permissions).
diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 6856dab06c..a8ae41de48 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -73,6 +73,11 @@ class LoginType:
     DUMMY = "m.login.dummy"
 
 
+# This is used in the `type` parameter for /register when called by
+# an appservice to register a new user.
+APP_SERVICE_REGISTRATION_TYPE = "m.login.application_service"
+
+
 class EventTypes:
     Member = "m.room.member"
     Create = "m.room.create"
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index c212da0cb2..4a064849c1 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -13,7 +13,6 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
 import hmac
 import logging
 import random
@@ -22,7 +21,7 @@ from typing import List, Union
 import synapse
 import synapse.api.auth
 import synapse.types
-from synapse.api.constants import LoginType
+from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
 from synapse.api.errors import (
     Codes,
     InteractiveAuthIncompleteError,
@@ -430,15 +429,20 @@ class RegisterRestServlet(RestServlet):
                 raise SynapseError(400, "Invalid username")
             desired_username = body["username"]
 
-        appservice = None
-        if self.auth.has_access_token(request):
-            appservice = self.auth.get_appservice_by_req(request)
-
         # fork off as soon as possible for ASes which have completely
         # different registration flows to normal users
 
         # == Application Service Registration ==
-        if appservice:
+        if body.get("type") == APP_SERVICE_REGISTRATION_TYPE:
+            if not self.auth.has_access_token(request):
+                raise SynapseError(
+                    400,
+                    "Appservice token must be provided when using a type of m.login.application_service",
+                )
+
+            # Verify the AS
+            self.auth.get_appservice_by_req(request)
+
             # Set the desired user according to the AS API (which uses the
             # 'user' key not 'username'). Since this is a new addition, we'll
             # fallback to 'username' if they gave one.
@@ -459,6 +463,11 @@ class RegisterRestServlet(RestServlet):
             )
 
             return 200, result
+        elif self.auth.has_access_token(request):
+            raise SynapseError(
+                400,
+                "An access token should not be provided on requests to /register (except if type is m.login.application_service)",
+            )
 
         # == Normal User Registration == (everyone else)
         if not self._registration_enabled:
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index 27db4f551e..cd60ea7081 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -14,7 +14,6 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
 import datetime
 import json
 import os
@@ -22,7 +21,7 @@ import os
 import pkg_resources
 
 import synapse.rest.admin
-from synapse.api.constants import LoginType
+from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
 from synapse.api.errors import Codes
 from synapse.appservice import ApplicationService
 from synapse.rest.client.v1 import login, logout
@@ -59,7 +58,9 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
         )
 
         self.hs.get_datastore().services_cache.append(appservice)
-        request_data = json.dumps({"username": "as_user_kermit"})
+        request_data = json.dumps(
+            {"username": "as_user_kermit", "type": APP_SERVICE_REGISTRATION_TYPE}
+        )
 
         channel = self.make_request(
             b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
@@ -69,9 +70,31 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
         det_data = {"user_id": user_id, "home_server": self.hs.hostname}
         self.assertDictContainsSubset(det_data, channel.json_body)
 
+    def test_POST_appservice_registration_no_type(self):
+        as_token = "i_am_an_app_service"
+
+        appservice = ApplicationService(
+            as_token,
+            self.hs.config.server_name,
+            id="1234",
+            namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]},
+            sender="@as:test",
+        )
+
+        self.hs.get_datastore().services_cache.append(appservice)
+        request_data = json.dumps({"username": "as_user_kermit"})
+
+        channel = self.make_request(
+            b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
+        )
+
+        self.assertEquals(channel.result["code"], b"400", channel.result)
+
     def test_POST_appservice_registration_invalid(self):
         self.appservice = None  # no application service exists
-        request_data = json.dumps({"username": "kermit"})
+        request_data = json.dumps(
+            {"username": "kermit", "type": APP_SERVICE_REGISTRATION_TYPE}
+        )
         channel = self.make_request(
             b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
         )
diff --git a/tests/test_mau.py b/tests/test_mau.py
index 75d28a42df..7d92a16a8d 100644
--- a/tests/test_mau.py
+++ b/tests/test_mau.py
@@ -15,9 +15,7 @@
 
 """Tests REST events for /rooms paths."""
 
-import json
-
-from synapse.api.constants import LoginType
+from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
 from synapse.api.errors import Codes, HttpResponseException, SynapseError
 from synapse.appservice import ApplicationService
 from synapse.rest.client.v2_alpha import register, sync
@@ -113,7 +111,7 @@ class TestMauLimit(unittest.HomeserverTestCase):
             )
         )
 
-        self.create_user("as_kermit4", token=as_token)
+        self.create_user("as_kermit4", token=as_token, appservice=True)
 
     def test_allowed_after_a_month_mau(self):
         # Create and sync so that the MAU counts get updated
@@ -232,14 +230,15 @@ class TestMauLimit(unittest.HomeserverTestCase):
         self.reactor.advance(100)
         self.assertEqual(2, self.successResultOf(count))
 
-    def create_user(self, localpart, token=None):
-        request_data = json.dumps(
-            {
-                "username": localpart,
-                "password": "monkey",
-                "auth": {"type": LoginType.DUMMY},
-            }
-        )
+    def create_user(self, localpart, token=None, appservice=False):
+        request_data = {
+            "username": localpart,
+            "password": "monkey",
+            "auth": {"type": LoginType.DUMMY},
+        }
+
+        if appservice:
+            request_data["type"] = APP_SERVICE_REGISTRATION_TYPE
 
         channel = self.make_request(
             "POST",