From 9743a552de68eb8f94f12e293c54043aabc87031 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 15 Sep 2020 17:22:13 +0100 Subject: [PATCH 01/12] Add ability for ASes to login --- synapse/api/constants.py | 1 + synapse/rest/client/v1/login.py | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 46013cde15a4..28f8285da959 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -57,6 +57,7 @@ class JoinRules: class LoginType: PASSWORD = "m.login.password" + APPSERVICE = "uk.half-shot.unstable.login.appservice" EMAIL_IDENTITY = "m.login.email.identity" MSISDN = "m.login.msisdn" RECAPTCHA = "m.login.recaptcha" diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index a14618ac84fb..e8b5fc9a4ed6 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -16,8 +16,10 @@ import logging from typing import Awaitable, Callable, Dict, Optional +from synapse.api.constants import LoginType from synapse.api.errors import Codes, LoginError, SynapseError from synapse.api.ratelimiting import Ratelimiter +from synapse.appservice import ApplicationService from synapse.handlers.auth import ( convert_client_dict_legacy_fields_to_identifier, login_id_phone_to_thirdparty, @@ -61,6 +63,8 @@ def __init__(self, hs): self.cas_enabled = hs.config.cas_enabled self.oidc_enabled = hs.config.oidc_enabled + self.auth = hs.get_auth() + self.auth_handler = self.hs.get_auth_handler() self.registration_handler = hs.get_registration_handler() self.handlers = hs.get_handlers() @@ -116,6 +120,11 @@ async def on_POST(self, request: SynapseRequest): self._address_ratelimiter.ratelimit(request.getClientIP()) login_submission = parse_json_object_from_request(request) + + appservice = None + if self.auth.has_access_token(request): + appservice = self.auth.get_appservice_by_req(request) + try: if self.jwt_enabled and ( login_submission["type"] == LoginRestServlet.JWT_TYPE @@ -125,7 +134,7 @@ async def on_POST(self, request: SynapseRequest): elif login_submission["type"] == LoginRestServlet.TOKEN_TYPE: result = await self._do_token_login(login_submission) else: - result = await self._do_other_login(login_submission) + result = await self._do_other_login(login_submission, appservice) except KeyError: raise SynapseError(400, "Missing JSON keys.") @@ -134,7 +143,9 @@ async def on_POST(self, request: SynapseRequest): result["well_known"] = well_known_data return 200, result - async def _do_other_login(self, login_submission: JsonDict) -> Dict[str, str]: + async def _do_other_login( + self, login_submission: JsonDict, appservice: ApplicationService + ) -> Dict[str, str]: """Handle non-token/saml/jwt logins Args: @@ -229,6 +240,10 @@ async def _do_other_login(self, login_submission: JsonDict) -> Dict[str, str]: else: qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() + if login_submission["type"] == LoginType.APPSERVICE and appservice is not None: + result = await self._complete_login(qualified_user_id, login_submission) + return result + # Check if we've hit the failed ratelimit (but don't update it) self._failed_attempts_ratelimiter.ratelimit( qualified_user_id.lower(), update=False From 5c04407cdfda2905d0d8f306f97aa6783f2d7e45 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 15 Sep 2020 17:57:36 +0100 Subject: [PATCH 02/12] changelog --- changelog.d/8320.feature | 1 + synapse/rest/client/v1/login.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/8320.feature diff --git a/changelog.d/8320.feature b/changelog.d/8320.feature new file mode 100644 index 000000000000..3afd1dc39e9e --- /dev/null +++ b/changelog.d/8320.feature @@ -0,0 +1 @@ +Add `uk.half-shot.unstable.login.appservice` login type to allow appservices to login. diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index e8b5fc9a4ed6..7db7ce319752 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -144,7 +144,7 @@ async def on_POST(self, request: SynapseRequest): return 200, result async def _do_other_login( - self, login_submission: JsonDict, appservice: ApplicationService + self, login_submission: JsonDict, appservice: Optional[ApplicationService] ) -> Dict[str, str]: """Handle non-token/saml/jwt logins From c3c1cc71623309c1f324855909a0844cee78b34d Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 16 Sep 2020 18:20:07 +0100 Subject: [PATCH 03/12] Match MSC --- synapse/api/constants.py | 2 +- synapse/rest/client/v1/login.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 28f8285da959..c39268f60768 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -57,7 +57,7 @@ class JoinRules: class LoginType: PASSWORD = "m.login.password" - APPSERVICE = "uk.half-shot.unstable.login.appservice" + APPSERVICE = "uk.half-shot.msc2778.login.application_service" EMAIL_IDENTITY = "m.login.email.identity" MSISDN = "m.login.msisdn" RECAPTCHA = "m.login.recaptcha" diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 7db7ce319752..d3ed8c83d0a0 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -240,7 +240,12 @@ async def _do_other_login( else: qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() - if login_submission["type"] == LoginType.APPSERVICE and appservice is not None: + if login_submission["type"] == LoginType.APPSERVICE: + if appservice is None or not appservice.is_interested_in_user( + qualified_user_id + ): + raise LoginError(403, "Invalid access_token", errcode=Codes.FORBIDDEN) + result = await self._complete_login(qualified_user_id, login_submission) return result From bcb63c8ef5e6832b7227e14de5846ace8c083a49 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 16 Sep 2020 18:57:45 +0100 Subject: [PATCH 04/12] Move constant to correct location --- synapse/api/constants.py | 1 - synapse/rest/client/v1/login.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index c39268f60768..46013cde15a4 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -57,7 +57,6 @@ class JoinRules: class LoginType: PASSWORD = "m.login.password" - APPSERVICE = "uk.half-shot.msc2778.login.application_service" EMAIL_IDENTITY = "m.login.email.identity" MSISDN = "m.login.msisdn" RECAPTCHA = "m.login.recaptcha" diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index d3ed8c83d0a0..386262e46986 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -41,6 +41,7 @@ class LoginRestServlet(RestServlet): PATTERNS = client_patterns("/login$", v1=True) + APPSERVICE_TYPE = "uk.half-shot.msc2778.login.application_service" CAS_TYPE = "m.login.cas" SSO_TYPE = "m.login.sso" TOKEN_TYPE = "m.login.token" @@ -240,7 +241,7 @@ async def _do_other_login( else: qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() - if login_submission["type"] == LoginType.APPSERVICE: + if login_submission["type"] == LoginRestServlet.APPSERVICE_TYPE: if appservice is None or not appservice.is_interested_in_user( qualified_user_id ): From b9b1d5bcb5368f21bc957126b2210ddf491044e5 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 17 Sep 2020 12:17:26 +0100 Subject: [PATCH 05/12] Add test --- synapse/rest/client/v1/login.py | 1 - tests/rest/client/v1/test_login.py | 74 +++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 386262e46986..d450a65e170e 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -16,7 +16,6 @@ import logging from typing import Awaitable, Callable, Dict, Optional -from synapse.api.constants import LoginType from synapse.api.errors import Codes, LoginError, SynapseError from synapse.api.ratelimiting import Ratelimiter from synapse.appservice import ApplicationService diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index 2668662c9e51..7a27869c7885 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -7,8 +7,9 @@ import jwt import synapse.rest.admin +from synapse.appservice import ApplicationService from synapse.rest.client.v1 import login, logout -from synapse.rest.client.v2_alpha import devices +from synapse.rest.client.v2_alpha import devices, register from synapse.rest.client.v2_alpha.account import WhoamiRestServlet from tests import unittest @@ -748,3 +749,74 @@ def test_login_jwt_invalid_signature(self): channel.json_body["error"], "JWT validation failed: Signature verification failed", ) + + +AS_USER = "as_user_alice" + + +class AppserviceLoginRestServletTestCase(unittest.HomeserverTestCase): + servlets = [ + login.register_servlets, + register.register_servlets, + lambda hs, http_server: WhoamiRestServlet(hs).register(http_server), + ] + + def register_as_user(self, username): + request, channel = self.make_request( + b"POST", + f"/_matrix/client/r0/register?access_token={self.service.token}", + {"username": username}, + ) + self.render(request) + + def make_homeserver(self, reactor, clock): + self.hs = self.setup_test_homeserver() + + self.service = ApplicationService( + id="unique_identifier", + token="some_token", + hostname="example.com", + sender="@asbot:example.com", + namespaces={ + ApplicationService.NS_USERS: [ + {"regex": r"@as_user.*", "exclusive": False} + ], + ApplicationService.NS_ROOMS: [], + ApplicationService.NS_ALIASES: [], + }, + ) + + self.hs.get_datastore().services_cache.append(self.service) + return self.hs + + def test_login_appservice_user(self): + """Test that an appservice user can use /login + """ + self.register_as_user(AS_USER) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": AS_USER}, + } + request, channel = self.make_request( + b"POST", LOGIN_URL, params, access_token=self.service.token + ) + + self.render(request) + self.assertEquals(channel.result["code"], b"200", channel.result) + + def test_login_appservice_user_bot(self): + """Test that the appservice bot can use /login + """ + self.register_as_user(AS_USER) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": self.service.sender}, + } + request, channel = self.make_request( + b"POST", LOGIN_URL, params, access_token=self.service.token + ) + + self.render(request) + self.assertEquals(channel.result["code"], b"200", channel.result) From b5477657ce897d5976334db56f469b351c25b8af Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 17 Sep 2020 12:19:42 +0100 Subject: [PATCH 06/12] Update 8320.feature --- changelog.d/8320.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/8320.feature b/changelog.d/8320.feature index 3afd1dc39e9e..475a5fe62d97 100644 --- a/changelog.d/8320.feature +++ b/changelog.d/8320.feature @@ -1 +1 @@ -Add `uk.half-shot.unstable.login.appservice` login type to allow appservices to login. +Add `uk.half-shot.msc2778.login.application_service` login type to allow appservices to login. From 6c6e4b23b01358ef99556b67dc4fabc415ea76c6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 17 Sep 2020 13:20:21 -0400 Subject: [PATCH 07/12] Fix support for Python 3.5. --- tests/rest/client/v1/test_login.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index 7a27869c7885..3f1f0ae01958 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -764,7 +764,7 @@ class AppserviceLoginRestServletTestCase(unittest.HomeserverTestCase): def register_as_user(self, username): request, channel = self.make_request( b"POST", - f"/_matrix/client/r0/register?access_token={self.service.token}", + "/_matrix/client/r0/register?access_token=%s" % (self.service.token,), {"username": username}, ) self.render(request) From 89e48aba4d1960806523a84ec62415d5a352b7e4 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 18 Sep 2020 12:56:18 +0100 Subject: [PATCH 08/12] Move APPSERVICE_TYPE --- synapse/rest/client/v1/login.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index d450a65e170e..792784a0c2e9 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -40,12 +40,12 @@ class LoginRestServlet(RestServlet): PATTERNS = client_patterns("/login$", v1=True) - APPSERVICE_TYPE = "uk.half-shot.msc2778.login.application_service" CAS_TYPE = "m.login.cas" SSO_TYPE = "m.login.sso" TOKEN_TYPE = "m.login.token" JWT_TYPE = "org.matrix.login.jwt" JWT_TYPE_DEPRECATED = "m.login.jwt" + APPSERVICE_TYPE = "uk.half-shot.msc2778.login.application_service" def __init__(self, hs): super(LoginRestServlet, self).__init__() From ba609226d7f8a3ef9709fcf017e3df02dda2b684 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 18 Sep 2020 12:56:33 +0100 Subject: [PATCH 09/12] define _do_appservice_login --- synapse/rest/client/v1/login.py | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 792784a0c2e9..b0d19e865ed7 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -121,12 +121,11 @@ async def on_POST(self, request: SynapseRequest): login_submission = parse_json_object_from_request(request) - appservice = None - if self.auth.has_access_token(request): - appservice = self.auth.get_appservice_by_req(request) - try: - if self.jwt_enabled and ( + if login_submission["type"] == LoginRestServlet.APPSERVICE_TYPE: + appservice = self.auth.get_appservice_by_req(request) + result = await self._do_appservice_login(login_submission, appservice) + elif self.jwt_enabled and ( login_submission["type"] == LoginRestServlet.JWT_TYPE or login_submission["type"] == LoginRestServlet.JWT_TYPE_DEPRECATED ): @@ -134,7 +133,7 @@ async def on_POST(self, request: SynapseRequest): elif login_submission["type"] == LoginRestServlet.TOKEN_TYPE: result = await self._do_token_login(login_submission) else: - result = await self._do_other_login(login_submission, appservice) + result = await self._do_other_login(login_submission) except KeyError: raise SynapseError(400, "Missing JSON keys.") @@ -143,9 +142,25 @@ async def on_POST(self, request: SynapseRequest): result["well_known"] = well_known_data return 200, result - async def _do_other_login( - self, login_submission: JsonDict, appservice: Optional[ApplicationService] - ) -> Dict[str, str]: + async def _do_appservice_login( + self, login_submission: JsonDict, appservice: ApplicationService + ): + logger.info( + "Got appservice login request with identifier: %r", + login_submission.get("identifier"), + ) + identifier = convert_client_dict_legacy_fields_to_identifier(login_submission) + if identifier["user"].startswith("@"): + qualified_user_id = identifier["user"] + else: + qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() + + if not appservice.is_interested_in_user(qualified_user_id): + raise LoginError(403, "Invalid access_token", errcode=Codes.FORBIDDEN) + + return await self._complete_login(qualified_user_id, login_submission) + + async def _do_other_login(self, login_submission: JsonDict) -> Dict[str, str]: """Handle non-token/saml/jwt logins Args: @@ -240,15 +255,6 @@ async def _do_other_login( else: qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() - if login_submission["type"] == LoginRestServlet.APPSERVICE_TYPE: - if appservice is None or not appservice.is_interested_in_user( - qualified_user_id - ): - raise LoginError(403, "Invalid access_token", errcode=Codes.FORBIDDEN) - - result = await self._complete_login(qualified_user_id, login_submission) - return result - # Check if we've hit the failed ratelimit (but don't update it) self._failed_attempts_ratelimiter.ratelimit( qualified_user_id.lower(), update=False From 4f28592fdcb61a972f519cf45a060164653e66eb Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 18 Sep 2020 13:46:18 +0100 Subject: [PATCH 10/12] Add new login type to flows --- synapse/rest/client/v1/login.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index b0d19e865ed7..bf7f2ea38e86 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -111,6 +111,8 @@ def on_GET(self, request: SynapseRequest): ({"type": t} for t in self.auth_handler.get_supported_login_types()) ) + flows.append({"type": LoginRestServlet.APPSERVICE_TYPE}) + return 200, {"flows": flows} def on_OPTIONS(self, request: SynapseRequest): From 539c77c2c79eb6624d1ec0e41400e5bc459556cb Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 18 Sep 2020 13:46:31 +0100 Subject: [PATCH 11/12] factor out _get_qualified_user_id --- synapse/rest/client/v1/login.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index bf7f2ea38e86..dd8cdc0d9f7e 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -144,6 +144,17 @@ async def on_POST(self, request: SynapseRequest): result["well_known"] = well_known_data return 200, result + def _get_qualified_user_id(self, identifier): + if identifier["type"] != "m.id.user": + raise SynapseError(400, "Unknown login identifier type") + if "user" not in identifier: + raise SynapseError(400, "User identifier is missing 'user' key") + + if identifier["user"].startswith("@"): + return identifier["user"] + else: + return UserID(identifier["user"], self.hs.hostname).to_string() + async def _do_appservice_login( self, login_submission: JsonDict, appservice: ApplicationService ): @@ -151,11 +162,9 @@ async def _do_appservice_login( "Got appservice login request with identifier: %r", login_submission.get("identifier"), ) + identifier = convert_client_dict_legacy_fields_to_identifier(login_submission) - if identifier["user"].startswith("@"): - qualified_user_id = identifier["user"] - else: - qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() + qualified_user_id = self._get_qualified_user_id(identifier) if not appservice.is_interested_in_user(qualified_user_id): raise LoginError(403, "Invalid access_token", errcode=Codes.FORBIDDEN) @@ -247,15 +256,7 @@ async def _do_other_login(self, login_submission: JsonDict) -> Dict[str, str]: # by this point, the identifier should be an m.id.user: if it's anything # else, we haven't understood it. - if identifier["type"] != "m.id.user": - raise SynapseError(400, "Unknown login identifier type") - if "user" not in identifier: - raise SynapseError(400, "User identifier is missing 'user' key") - - if identifier["user"].startswith("@"): - qualified_user_id = identifier["user"] - else: - qualified_user_id = UserID(identifier["user"], self.hs.hostname).to_string() + qualified_user_id = self._get_qualified_user_id(identifier) # Check if we've hit the failed ratelimit (but don't update it) self._failed_attempts_ratelimiter.ratelimit( From 9b59550e71f1d4a9625caa9fe3abefbfb909f25a Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 18 Sep 2020 14:31:27 +0100 Subject: [PATCH 12/12] Add tests for various failure states --- tests/rest/client/v1/test_login.py | 62 +++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index 3f1f0ae01958..5d987a30c7e9 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -758,7 +758,6 @@ class AppserviceLoginRestServletTestCase(unittest.HomeserverTestCase): servlets = [ login.register_servlets, register.register_servlets, - lambda hs, http_server: WhoamiRestServlet(hs).register(http_server), ] def register_as_user(self, username): @@ -785,8 +784,22 @@ def make_homeserver(self, reactor, clock): ApplicationService.NS_ALIASES: [], }, ) + self.another_service = ApplicationService( + id="another__identifier", + token="another_token", + hostname="example.com", + sender="@as2bot:example.com", + namespaces={ + ApplicationService.NS_USERS: [ + {"regex": r"@as2_user.*", "exclusive": False} + ], + ApplicationService.NS_ROOMS: [], + ApplicationService.NS_ALIASES: [], + }, + ) self.hs.get_datastore().services_cache.append(self.service) + self.hs.get_datastore().services_cache.append(self.another_service) return self.hs def test_login_appservice_user(self): @@ -820,3 +833,50 @@ def test_login_appservice_user_bot(self): self.render(request) self.assertEquals(channel.result["code"], b"200", channel.result) + + def test_login_appservice_wrong_user(self): + """Test that non-as users cannot login with the as token + """ + self.register_as_user(AS_USER) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": "fibble_wibble"}, + } + request, channel = self.make_request( + b"POST", LOGIN_URL, params, access_token=self.service.token + ) + + self.render(request) + self.assertEquals(channel.result["code"], b"403", channel.result) + + def test_login_appservice_wrong_as(self): + """Test that as users cannot login with wrong as token + """ + self.register_as_user(AS_USER) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": AS_USER}, + } + request, channel = self.make_request( + b"POST", LOGIN_URL, params, access_token=self.another_service.token + ) + + self.render(request) + self.assertEquals(channel.result["code"], b"403", channel.result) + + def test_login_appservice_no_token(self): + """Test that users must provide a token when using the appservice + login method + """ + self.register_as_user(AS_USER) + + params = { + "type": login.LoginRestServlet.APPSERVICE_TYPE, + "identifier": {"type": "m.id.user", "user": AS_USER}, + } + request, channel = self.make_request(b"POST", LOGIN_URL, params) + + self.render(request) + self.assertEquals(channel.result["code"], b"401", channel.result)