From d4ce14ee434e794741fa7093d2c702786bdba186 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 4 Mar 2021 17:27:21 +0000 Subject: [PATCH 01/15] Require AppserviceRegistrationType --- synapse/api/constants.py | 5 +++++ synapse/rest/client/v2_alpha/register.py | 15 ++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 691f8f9adf07..297cd097ba40 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -70,6 +70,11 @@ class LoginType: DUMMY = "m.login.dummy" +# This is not a login type as we cannot login with it, however +# the spec says that this is a valid registration type for appservices. +AppserviceRegistrationType = "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 8f68d8dfc8fa..16889898b0d7 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 @@ import synapse import synapse.api.auth import synapse.types -from synapse.api.constants import LoginType +from synapse.api.constants import AppserviceRegistrationType, LoginType from synapse.api.errors import ( Codes, InteractiveAuthIncompleteError, @@ -428,15 +427,17 @@ async def on_POST(self, request): 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") == AppserviceRegistrationType: + 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", + ) + # 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. From 9415c6ef6c5b4ac3dec4538d1dd2dd52835ef42f Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 4 Mar 2021 17:31:06 +0000 Subject: [PATCH 02/15] Fixup test --- tests/rest/client/v2_alpha/test_register.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 27db4f551e2e..26acff0dc3b0 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 pkg_resources import synapse.rest.admin -from synapse.api.constants import LoginType +from synapse.api.constants import AppserviceRegistrationType, LoginType from synapse.api.errors import Codes from synapse.appservice import ApplicationService from synapse.rest.client.v1 import login, logout @@ -59,7 +58,9 @@ def test_POST_appservice_registration_valid(self): ) 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": AppserviceRegistrationType} + ) channel = self.make_request( b"POST", self.url + b"?access_token=i_am_an_app_service", request_data From 775f282b0fb5a30fda2b05f24629e9dc88d3d156 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 4 Mar 2021 17:32:37 +0000 Subject: [PATCH 03/15] Also add a test to ensure that omitting a type fails --- tests/rest/client/v2_alpha/test_register.py | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 26acff0dc3b0..de6fdf1b5465 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -70,9 +70,31 @@ def test_POST_appservice_registration_valid(self): 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": AppserviceRegistrationType} + ) channel = self.make_request( b"POST", self.url + b"?access_token=i_am_an_app_service", request_data ) From 6d28ee89755416822f1fd79f217f0adbdd9bc355 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 4 Mar 2021 17:37:51 +0000 Subject: [PATCH 04/15] changelog --- changelog.d/9548.bugfix | 1 + synapse/rest/client/v2_alpha/register.py | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 changelog.d/9548.bugfix diff --git a/changelog.d/9548.bugfix b/changelog.d/9548.bugfix new file mode 100644 index 000000000000..86c25d4ae814 --- /dev/null +++ b/changelog.d/9548.bugfix @@ -0,0 +1 @@ +/register now requires a `body.type` value of `m.login.appservice` when registering appservice users. \ No newline at end of file diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 16889898b0d7..46fe2dd89d9b 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -438,6 +438,9 @@ async def on_POST(self, request): "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. From dba6ff47c6c14cbf0fca164bfaf1ec9c4d258792 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 5 Mar 2021 11:48:03 +0000 Subject: [PATCH 05/15] Fix test --- tests/test_mau.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/test_mau.py b/tests/test_mau.py index 75d28a42dfe5..3369422f0fa7 100644 --- a/tests/test_mau.py +++ b/tests/test_mau.py @@ -17,7 +17,7 @@ import json -from synapse.api.constants import LoginType +from synapse.api.constants import AppserviceRegistrationType, 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 +113,7 @@ def test_as_ignores_mau(self): ) ) - 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,19 +232,20 @@ def test_tracked_but_not_limited(self): 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"] = AppserviceRegistrationType channel = self.make_request( "POST", "/register", - request_data, + json.dumps(request_data), access_token=token, ) From 8bd7eb6be83458ad0ba23a04fcf469d9ef8ff88f Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 8 Mar 2021 11:04:12 +0000 Subject: [PATCH 06/15] Throw a specific error for when the access token is provided without a type --- synapse/rest/client/v2_alpha/register.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 46fe2dd89d9b..4fe2fd284d10 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -461,6 +461,11 @@ async def on_POST(self, request): ) return 200, result + else if self.auth.has_access_token(request): + raise SynapseError( + 400, + "A type of m.login.application_service must be provided when registering as an appservice", + ) # == Normal User Registration == (everyone else) if not self._registration_enabled: From 2b86a5ffc058ae22610f47365db506bdf7e4e3f8 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 8 Mar 2021 11:11:05 +0000 Subject: [PATCH 07/15] Update register.py --- synapse/rest/client/v2_alpha/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 4fe2fd284d10..6878873ba585 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -461,7 +461,7 @@ async def on_POST(self, request): ) return 200, result - else if self.auth.has_access_token(request): + elif self.auth.has_access_token(request): raise SynapseError( 400, "A type of m.login.application_service must be provided when registering as an appservice", From ca4213abebb48168bde277015fc9424b527e3e3e Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 8 Mar 2021 15:39:42 +0000 Subject: [PATCH 08/15] Constant naming fix --- synapse/api/constants.py | 2 +- synapse/rest/client/v2_alpha/register.py | 4 ++-- tests/rest/client/v2_alpha/test_register.py | 6 +++--- tests/test_mau.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 297cd097ba40..e8f2e435b483 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -72,7 +72,7 @@ class LoginType: # This is not a login type as we cannot login with it, however # the spec says that this is a valid registration type for appservices. -AppserviceRegistrationType = "m.login.application_service" +APP_SERVICE_REGISTRATION_TYPE = "m.login.application_service" class EventTypes: diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 46fe2dd89d9b..21a9d5e9a3f8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -21,7 +21,7 @@ import synapse import synapse.api.auth import synapse.types -from synapse.api.constants import AppserviceRegistrationType, LoginType +from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.errors import ( Codes, InteractiveAuthIncompleteError, @@ -431,7 +431,7 @@ async def on_POST(self, request): # different registration flows to normal users # == Application Service Registration == - if body.get("type") == AppserviceRegistrationType: + if body.get("type") == APP_SERVICE_REGISTRATION_TYPE: if not self.auth.has_access_token(request): raise SynapseError( 400, diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index de6fdf1b5465..cd60ea70811e 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -21,7 +21,7 @@ import pkg_resources import synapse.rest.admin -from synapse.api.constants import AppserviceRegistrationType, 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 +59,7 @@ def test_POST_appservice_registration_valid(self): self.hs.get_datastore().services_cache.append(appservice) request_data = json.dumps( - {"username": "as_user_kermit", "type": AppserviceRegistrationType} + {"username": "as_user_kermit", "type": APP_SERVICE_REGISTRATION_TYPE} ) channel = self.make_request( @@ -93,7 +93,7 @@ def test_POST_appservice_registration_no_type(self): def test_POST_appservice_registration_invalid(self): self.appservice = None # no application service exists request_data = json.dumps( - {"username": "kermit", "type": AppserviceRegistrationType} + {"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 3369422f0fa7..ff5303909bcf 100644 --- a/tests/test_mau.py +++ b/tests/test_mau.py @@ -17,7 +17,7 @@ import json -from synapse.api.constants import AppserviceRegistrationType, 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 @@ -240,7 +240,7 @@ def create_user(self, localpart, token=None, appservice=False): } if appservice: - request_data["type"] = AppserviceRegistrationType + request_data["type"] = APP_SERVICE_REGISTRATION_TYPE channel = self.make_request( "POST", From 95025179cab7a063ec75f246ce7422d81d98922d Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 14:27:53 +0100 Subject: [PATCH 09/15] Update comment about the `type` parameters Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/api/constants.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index e8f2e435b483..324adef412f7 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -70,8 +70,8 @@ class LoginType: DUMMY = "m.login.dummy" -# This is not a login type as we cannot login with it, however -# the spec says that this is a valid registration type for appservices. +# 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" From 4d6f8444ab78184731765c9210b9e2b6a586bc5d Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 14:30:05 +0100 Subject: [PATCH 10/15] Attempt a different changelog --- changelog.d/9548.bugfix | 1 - changelog.d/9548.removal | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 changelog.d/9548.bugfix create mode 100644 changelog.d/9548.removal diff --git a/changelog.d/9548.bugfix b/changelog.d/9548.bugfix deleted file mode 100644 index 86c25d4ae814..000000000000 --- a/changelog.d/9548.bugfix +++ /dev/null @@ -1 +0,0 @@ -/register now requires a `body.type` value of `m.login.appservice` when registering appservice users. \ No newline at end of file diff --git a/changelog.d/9548.removal b/changelog.d/9548.removal new file mode 100644 index 000000000000..3b8facb04c3e --- /dev/null +++ b/changelog.d/9548.removal @@ -0,0 +1 @@ +/register now expects a `body.type` value of `m.login.appservice` when registering appservice users, to conform to the spec. From 87720de1e0b7f3db5fbfd9a5017a5b8f069849f6 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 16:47:54 +0100 Subject: [PATCH 11/15] Update changelog.d/9548.removal Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/9548.removal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9548.removal b/changelog.d/9548.removal index 3b8facb04c3e..f217e7e81117 100644 --- a/changelog.d/9548.removal +++ b/changelog.d/9548.removal @@ -1 +1 @@ -/register now expects a `body.type` value of `m.login.appservice` when registering appservice users, to conform to the spec. +Make `/_matrix/client/r0/register` expect a login 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). From 9b9603375f96ea8f6d263071dd021e849065b6e4 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 16:49:36 +0100 Subject: [PATCH 12/15] remove json.dumps --- tests/test_mau.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_mau.py b/tests/test_mau.py index ff5303909bcf..a396fe68b91f 100644 --- a/tests/test_mau.py +++ b/tests/test_mau.py @@ -245,7 +245,7 @@ def create_user(self, localpart, token=None, appservice=False): channel = self.make_request( "POST", "/register", - json.dumps(request_data), + request_data, access_token=token, ) From c98ede593603ecec2782a8acf5ecfdaf2e8a3b93 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 17:04:14 +0100 Subject: [PATCH 13/15] Update changelog.d/9548.removal Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/9548.removal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9548.removal b/changelog.d/9548.removal index f217e7e81117..1fb88236c6b5 100644 --- a/changelog.d/9548.removal +++ b/changelog.d/9548.removal @@ -1 +1 @@ -Make `/_matrix/client/r0/register` expect a login 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). +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). From 3d080e62cca70e9b87145113ef202151b03d3423 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 9 Apr 2021 17:05:16 +0100 Subject: [PATCH 14/15] Remove unused json import --- tests/test_mau.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_mau.py b/tests/test_mau.py index a396fe68b91f..7d92a16a8d5a 100644 --- a/tests/test_mau.py +++ b/tests/test_mau.py @@ -15,8 +15,6 @@ """Tests REST events for /rooms paths.""" -import json - from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.appservice import ApplicationService From e2637e56040121a7e567115ba28e4e847ce594b9 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 12 Apr 2021 11:47:37 +0100 Subject: [PATCH 15/15] Update synapse/rest/client/v2_alpha/register.py Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/rest/client/v2_alpha/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 66c1307ed500..4a064849c1b8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -466,7 +466,7 @@ async def on_POST(self, request): elif self.auth.has_access_token(request): raise SynapseError( 400, - "A type of m.login.application_service must be provided when registering as an appservice", + "An access token should not be provided on requests to /register (except if type is m.login.application_service)", ) # == Normal User Registration == (everyone else)