From 308ce68f9f31fb6ae59075665c06ea35b6a6c6dc Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Mon, 3 Apr 2023 12:14:16 +0100
Subject: [PATCH 01/17] Implement MSC3882 revision 1
---
synapse/rest/client/capabilities.py | 5 ++++
synapse/rest/client/login.py | 16 ++++++++--
synapse/rest/client/login_token_request.py | 10 +++----
synapse/rest/client/versions.py | 2 --
tests/rest/client/test_capabilities.py | 30 +++++++++++++++++++
tests/rest/client/test_login.py | 23 ++++++++++++++
tests/rest/client/test_login_token_request.py | 8 ++---
7 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/synapse/rest/client/capabilities.py b/synapse/rest/client/capabilities.py
index e84dde31b118..11fc0b067899 100644
--- a/synapse/rest/client/capabilities.py
+++ b/synapse/rest/client/capabilities.py
@@ -82,6 +82,11 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"enabled": self.config.experimental.msc3664_enabled,
}
+ if self.config.experimental.msc3882_enabled:
+ response["capabilities"]["org.matrix.msc3882.get_login_token"] = {
+ "enabled": True,
+ }
+
return HTTPStatus.OK, response
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index b7e9c8f6b572..896cf2cdbe47 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -107,6 +107,9 @@ def __init__(self, hs: "HomeServer"):
and hs.config.experimental.msc3866.require_approval_for_new_accounts
)
+ # Whether MSC3882 get login token is enabled.
+ self._get_login_token_enabled = hs.config.experimental.msc3882_enabled
+
self.auth = hs.get_auth()
self.clock = hs.get_clock()
@@ -145,7 +148,12 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
# to SSO.
flows.append({"type": LoginRestServlet.CAS_TYPE})
- if self.cas_enabled or self.saml2_enabled or self.oidc_enabled:
+ if (
+ self.cas_enabled
+ or self.saml2_enabled
+ or self.oidc_enabled
+ or self._get_login_token_enabled
+ ):
flows.append(
{
"type": LoginRestServlet.SSO_TYPE,
@@ -163,7 +171,11 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
# don't know how to implement, since they (currently) will always
# fall back to the fallback API if they don't understand one of the
# login flow types returned.
- flows.append({"type": LoginRestServlet.TOKEN_TYPE})
+ tokenTypeFlow: Dict[str, Any] = {"type": LoginRestServlet.TOKEN_TYPE}
+ # If MSC3882 is enabled we advertise the get_login_token flag.
+ if self._get_login_token_enabled:
+ tokenTypeFlow["org.matrix.msc3882.get_login_token"] = True
+ flows.append(tokenTypeFlow)
flows.extend({"type": t} for t in self.auth_handler.get_supported_login_types())
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index 43ea21d5e6ac..2d8726ac4cf1 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -33,7 +33,7 @@ class LoginTokenRequestServlet(RestServlet):
Request:
- POST /login/token HTTP/1.1
+ POST /login/get_token HTTP/1.1
Content-Type: application/json
{}
@@ -43,12 +43,12 @@ class LoginTokenRequestServlet(RestServlet):
HTTP/1.1 200 OK
{
"login_token": "ABDEFGH",
- "expires_in": 3600,
+ "expires_in_ms": 3600000,
}
"""
PATTERNS = client_patterns(
- "/org.matrix.msc3882/login/token$", releases=[], v1=False, unstable=True
+ "/org.matrix.msc3882/login/get_token$", releases=[], v1=False, unstable=True
)
def __init__(self, hs: "HomeServer"):
@@ -77,7 +77,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
login_token = await self.auth_handler.create_login_token_for_user_id(
user_id=requester.user.to_string(),
- auth_provider_id="org.matrix.msc3882.login_token_request",
+ auth_provider_id="org.matrix.msc3882.get_login_token",
duration_ms=self.token_timeout,
)
@@ -85,7 +85,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
200,
{
"login_token": login_token,
- "expires_in": self.token_timeout // 1000,
+ "expires_in_ms": self.token_timeout,
},
)
diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py
index 59aed66464e5..ecd84f435f4f 100644
--- a/synapse/rest/client/versions.py
+++ b/synapse/rest/client/versions.py
@@ -112,8 +112,6 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
"fi.mau.msc2815": self.config.experimental.msc2815_enabled,
# Adds a ping endpoint for appservices to check HS->AS connection
"fi.mau.msc2659": self.config.experimental.msc2659_enabled,
- # Adds support for login token requests as per MSC3882
- "org.matrix.msc3882": self.config.experimental.msc3882_enabled,
# Adds support for remotely enabling/disabling pushers, as per MSC3881
"org.matrix.msc3881": self.config.experimental.msc3881_enabled,
# Adds support for filtering /messages by event relation.
diff --git a/tests/rest/client/test_capabilities.py b/tests/rest/client/test_capabilities.py
index c16e8d43f419..bc33854b229b 100644
--- a/tests/rest/client/test_capabilities.py
+++ b/tests/rest/client/test_capabilities.py
@@ -186,3 +186,33 @@ def test_get_does_include_msc3244_fields_when_enabled(self) -> None:
self.assertGreater(len(details["support"]), 0)
for room_version in details["support"]:
self.assertTrue(room_version in KNOWN_ROOM_VERSIONS, str(room_version))
+
+ def test_get_does_not_include_msc3882_fields_when_disabled(self) -> None:
+ access_token = self.get_success(
+ self.auth_handler.create_access_token_for_user_id(
+ self.user, device_id=None, valid_until_ms=None
+ )
+ )
+
+ channel = self.make_request("GET", self.url, access_token=access_token)
+ capabilities = channel.json_body["capabilities"]
+
+ self.assertEqual(channel.code, HTTPStatus.OK)
+ self.assertTrue(
+ "org.matrix.msc3882.get_login_token" not in capabilities
+ or not capabilities["org.matrix.msc3882.get_login_token"]["enabled"]
+ )
+
+ @override_config({"experimental_features": {"msc3882_enabled": True}})
+ def test_get_does_include_msc3882_fields_when_enabled(self) -> None:
+ access_token = self.get_success(
+ self.auth_handler.create_access_token_for_user_id(
+ self.user, device_id=None, valid_until_ms=None
+ )
+ )
+
+ channel = self.make_request("GET", self.url, access_token=access_token)
+ capabilities = channel.json_body["capabilities"]
+
+ self.assertEqual(channel.code, HTTPStatus.OK)
+ self.assertTrue(capabilities["org.matrix.msc3882.get_login_token"]["enabled"])
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
index 62acf4f44ed0..69b463890047 100644
--- a/tests/rest/client/test_login.py
+++ b/tests/rest/client/test_login.py
@@ -446,6 +446,29 @@ def test_require_approval(self) -> None:
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)
+ def test_get_login_flows_with_msc3882_disabled(self) -> None:
+ """GET /login should return m.login.token without get_login_token true"""
+ channel = self.make_request("GET", "/_matrix/client/r0/login")
+ self.assertEqual(channel.code, 200, channel.result)
+
+ flows = {flow["type"]: flow for flow in channel.json_body["flows"]}
+ self.assertTrue(
+ "m.login.token" not in flows
+ or "org.matrix.msc3882.get_login_token" not in flows["m.login.token"]
+ or not flows["m.login.token"]["org.matrix.msc3882.get_login_token"]
+ )
+
+ @override_config({"experimental_features": {"msc3882_enabled": True}})
+ def test_get_login_flows_with_msc3882_enabled(self) -> None:
+ """GET /login should return m.login.token without get_login_token true"""
+ channel = self.make_request("GET", "/_matrix/client/r0/login")
+ self.assertEqual(channel.code, 200, channel.result)
+
+ print(channel.json_body)
+
+ flows = {flow["type"]: flow for flow in channel.json_body["flows"]}
+ self.assertTrue(flows["m.login.token"]["org.matrix.msc3882.get_login_token"])
+
@skip_unless(has_saml2 and HAS_OIDC, "Requires SAML2 and OIDC")
class MultiSSOTestCase(unittest.HomeserverTestCase):
diff --git a/tests/rest/client/test_login_token_request.py b/tests/rest/client/test_login_token_request.py
index b8187db982bf..cdf4134cbed1 100644
--- a/tests/rest/client/test_login_token_request.py
+++ b/tests/rest/client/test_login_token_request.py
@@ -22,7 +22,7 @@
from tests import unittest
from tests.unittest import override_config
-endpoint = "/_matrix/client/unstable/org.matrix.msc3882/login/token"
+endpoint = "/_matrix/client/unstable/org.matrix.msc3882/login/get_token"
class LoginTokenRequestServletTestCase(unittest.HomeserverTestCase):
@@ -82,7 +82,7 @@ def test_uia_on(self) -> None:
channel = self.make_request("POST", endpoint, uia, access_token=token)
self.assertEqual(channel.code, 200)
- self.assertEqual(channel.json_body["expires_in"], 300)
+ self.assertEqual(channel.json_body["expires_in_ms"], 300000)
login_token = channel.json_body["login_token"]
@@ -103,7 +103,7 @@ def test_uia_off(self) -> None:
channel = self.make_request("POST", endpoint, {}, access_token=token)
self.assertEqual(channel.code, 200)
- self.assertEqual(channel.json_body["expires_in"], 300)
+ self.assertEqual(channel.json_body["expires_in_ms"], 300000)
login_token = channel.json_body["login_token"]
@@ -130,4 +130,4 @@ def test_expires_in(self) -> None:
channel = self.make_request("POST", endpoint, {}, access_token=token)
self.assertEqual(channel.code, 200)
- self.assertEqual(channel.json_body["expires_in"], 15)
+ self.assertEqual(channel.json_body["expires_in_ms"], 15000)
From 97e4775c6cf04c23a8b0e790907ceb5d49b26d73 Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Tue, 4 Apr 2023 16:54:20 +0100
Subject: [PATCH 02/17] Changelog
---
changelog.d/15388.feature | 1 +
1 file changed, 1 insertion(+)
create mode 100644 changelog.d/15388.feature
diff --git a/changelog.d/15388.feature b/changelog.d/15388.feature
new file mode 100644
index 000000000000..cdfd11164786
--- /dev/null
+++ b/changelog.d/15388.feature
@@ -0,0 +1 @@
+Experimental implementation of revision 1 of [MSC3882](https://github.com/matrix-org/matrix-spec-proposals/pull/3882) to allow an existing device/session to generate a login token for use on a new device/session.
\ No newline at end of file
From 54fe012f8e2ae2cda7dc1ec12c922870aedf8e24 Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Tue, 4 Apr 2023 17:38:47 +0100
Subject: [PATCH 03/17] Fix advertised flows when SSO is not in use
---
synapse/rest/client/login.py | 21 ++++++++-------------
tests/rest/client/test_login.py | 12 ++++++++----
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index 896cf2cdbe47..e04d4f242527 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -148,12 +148,10 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
# to SSO.
flows.append({"type": LoginRestServlet.CAS_TYPE})
- if (
- self.cas_enabled
- or self.saml2_enabled
- or self.oidc_enabled
- or self._get_login_token_enabled
- ):
+ # MSC3882 requires m.login.token to be advertised
+ supportLoginTokenFlow = self._get_login_token_enabled
+
+ if self.cas_enabled or self.saml2_enabled or self.oidc_enabled:
flows.append(
{
"type": LoginRestServlet.SSO_TYPE,
@@ -164,13 +162,10 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
}
)
- # While it's valid for us to advertise this login type generally,
- # synapse currently only gives out these tokens as part of the
- # SSO login flow.
- # Generally we don't want to advertise login flows that clients
- # don't know how to implement, since they (currently) will always
- # fall back to the fallback API if they don't understand one of the
- # login flow types returned.
+ # SSO requires a login token to be generated, so we need to advertise that flow
+ supportLoginTokenFlow = True
+
+ if supportLoginTokenFlow:
tokenTypeFlow: Dict[str, Any] = {"type": LoginRestServlet.TOKEN_TYPE}
# If MSC3882 is enabled we advertise the get_login_token flag.
if self._get_login_token_enabled:
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
index 69b463890047..6f4135eea0d5 100644
--- a/tests/rest/client/test_login.py
+++ b/tests/rest/client/test_login.py
@@ -464,10 +464,14 @@ def test_get_login_flows_with_msc3882_enabled(self) -> None:
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)
- print(channel.json_body)
-
- flows = {flow["type"]: flow for flow in channel.json_body["flows"]}
- self.assertTrue(flows["m.login.token"]["org.matrix.msc3882.get_login_token"])
+ self.assertCountEqual(
+ channel.json_body["flows"],
+ [
+ {"type": "m.login.token", "org.matrix.msc3882.get_login_token": True},
+ {"type": "m.login.password"},
+ {"type": "m.login.application_service"},
+ ],
+ )
@skip_unless(has_saml2 and HAS_OIDC, "Requires SAML2 and OIDC")
From 5d75875c588f3af67b5b15e36de2f0d32a2d4db6 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 24 May 2023 10:06:04 -0400
Subject: [PATCH 04/17] Update config.
---
synapse/config/auth.py | 10 ++++++++++
synapse/config/experimental.py | 7 -------
synapse/rest/client/capabilities.py | 8 +++-----
synapse/rest/client/login.py | 16 ++++++++--------
synapse/rest/client/login_token_request.py | 8 ++++----
tests/rest/client/test_capabilities.py | 14 ++++++--------
tests/rest/client/test_login.py | 14 +++++---------
tests/rest/client/test_login_token_request.py | 14 +++++++-------
8 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/synapse/config/auth.py b/synapse/config/auth.py
index 35774962c0be..92fffef1c36d 100644
--- a/synapse/config/auth.py
+++ b/synapse/config/auth.py
@@ -53,3 +53,13 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.ui_auth_session_timeout = self.parse_duration(
ui_auth.get("session_timeout", 0)
)
+
+ # Logging in with an existing session.
+ login_via_existing = config.get("login_via_existing_session", {})
+ self.login_via_existing_enabled = login_via_existing.get("enabled", False)
+ self.login_via_existing_require_ui_auth = login_via_existing.get(
+ "require_ui_auth", True
+ )
+ self.login_via_existing_token_timeout = self.parse_duration(
+ login_via_existing.get("token_timeout", "5m")
+ )
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index d769b7f6686e..8daa23721972 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -118,13 +118,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC3881: Remotely toggle push notifications for another client
self.msc3881_enabled: bool = experimental.get("msc3881_enabled", False)
- # MSC3882: Allow an existing session to sign in a new session
- self.msc3882_enabled: bool = experimental.get("msc3882_enabled", False)
- self.msc3882_ui_auth: bool = experimental.get("msc3882_ui_auth", True)
- self.msc3882_token_timeout = self.parse_duration(
- experimental.get("msc3882_token_timeout", "5m")
- )
-
# MSC3874: Filtering /messages with rel_types / not_rel_types.
self.msc3874_enabled: bool = experimental.get("msc3874_enabled", False)
diff --git a/synapse/rest/client/capabilities.py b/synapse/rest/client/capabilities.py
index a77b0697b786..3154b9f77e95 100644
--- a/synapse/rest/client/capabilities.py
+++ b/synapse/rest/client/capabilities.py
@@ -65,6 +65,9 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"m.3pid_changes": {
"enabled": self.config.registration.enable_3pid_changes
},
+ "m.get_login_token": {
+ "enabled": self.config.auth.login_via_existing_enabled,
+ },
}
}
@@ -83,11 +86,6 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"enabled": self.config.experimental.msc3664_enabled,
}
- if self.config.experimental.msc3882_enabled:
- response["capabilities"]["org.matrix.msc3882.get_login_token"] = {
- "enabled": True,
- }
-
return HTTPStatus.OK, response
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index 7d7714abb8d9..fed61abecbd5 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -104,8 +104,8 @@ def __init__(self, hs: "HomeServer"):
and hs.config.experimental.msc3866.require_approval_for_new_accounts
)
- # Whether MSC3882 get login token is enabled.
- self._get_login_token_enabled = hs.config.experimental.msc3882_enabled
+ # Whether get login token is enabled.
+ self._get_login_token_enabled = hs.config.auth.login_via_existing_enabled
self.auth = hs.get_auth()
@@ -145,8 +145,8 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
# to SSO.
flows.append({"type": LoginRestServlet.CAS_TYPE})
- # MSC3882 requires m.login.token to be advertised
- supportLoginTokenFlow = self._get_login_token_enabled
+ # The login token flow requires m.login.token to be advertised.
+ support_login_token_flow = self._get_login_token_enabled
if self.cas_enabled or self.saml2_enabled or self.oidc_enabled:
flows.append(
@@ -160,13 +160,13 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
)
# SSO requires a login token to be generated, so we need to advertise that flow
- supportLoginTokenFlow = True
+ support_login_token_flow = True
- if supportLoginTokenFlow:
+ if support_login_token_flow:
tokenTypeFlow: Dict[str, Any] = {"type": LoginRestServlet.TOKEN_TYPE}
- # If MSC3882 is enabled we advertise the get_login_token flag.
+ # If the login token flow is enabled advertise the get_login_token flag.
if self._get_login_token_enabled:
- tokenTypeFlow["org.matrix.msc3882.get_login_token"] = True
+ tokenTypeFlow["m.get_login_token"] = True
flows.append(tokenTypeFlow)
flows.extend({"type": t} for t in self.auth_handler.get_supported_login_types())
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index 2d8726ac4cf1..44501f3614a7 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -58,15 +58,15 @@ def __init__(self, hs: "HomeServer"):
self.clock = hs.get_clock()
self.server_name = hs.config.server.server_name
self.auth_handler = hs.get_auth_handler()
- self.token_timeout = hs.config.experimental.msc3882_token_timeout
- self.ui_auth = hs.config.experimental.msc3882_ui_auth
+ self.token_timeout = hs.config.auth.login_via_existing_token_timeout
+ self._require_ui_auth = hs.config.auth.login_via_existing_require_ui_auth
@interactive_auth_handler
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)
body = parse_json_object_from_request(request)
- if self.ui_auth:
+ if self._require_ui_auth:
await self.auth_handler.validate_user_via_ui_auth(
requester,
request,
@@ -91,5 +91,5 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
- if hs.config.experimental.msc3882_enabled:
+ if hs.config.auth.login_via_existing_enabled:
LoginTokenRequestServlet(hs).register(http_server)
diff --git a/tests/rest/client/test_capabilities.py b/tests/rest/client/test_capabilities.py
index bc33854b229b..cf23430f6ad4 100644
--- a/tests/rest/client/test_capabilities.py
+++ b/tests/rest/client/test_capabilities.py
@@ -187,7 +187,8 @@ def test_get_does_include_msc3244_fields_when_enabled(self) -> None:
for room_version in details["support"]:
self.assertTrue(room_version in KNOWN_ROOM_VERSIONS, str(room_version))
- def test_get_does_not_include_msc3882_fields_when_disabled(self) -> None:
+ def test_get_get_token_login_fields_when_disabled(self) -> None:
+ """By default login via an existing session is disabled."""
access_token = self.get_success(
self.auth_handler.create_access_token_for_user_id(
self.user, device_id=None, valid_until_ms=None
@@ -198,13 +199,10 @@ def test_get_does_not_include_msc3882_fields_when_disabled(self) -> None:
capabilities = channel.json_body["capabilities"]
self.assertEqual(channel.code, HTTPStatus.OK)
- self.assertTrue(
- "org.matrix.msc3882.get_login_token" not in capabilities
- or not capabilities["org.matrix.msc3882.get_login_token"]["enabled"]
- )
+ self.assertFalse(capabilities["m.get_login_token"]["enabled"])
- @override_config({"experimental_features": {"msc3882_enabled": True}})
- def test_get_does_include_msc3882_fields_when_enabled(self) -> None:
+ @override_config({"login_via_existing_session": {"enabled": True}})
+ def test_get_get_token_login_fields_when_enabled(self) -> None:
access_token = self.get_success(
self.auth_handler.create_access_token_for_user_id(
self.user, device_id=None, valid_until_ms=None
@@ -215,4 +213,4 @@ def test_get_does_include_msc3882_fields_when_enabled(self) -> None:
capabilities = channel.json_body["capabilities"]
self.assertEqual(channel.code, HTTPStatus.OK)
- self.assertTrue(capabilities["org.matrix.msc3882.get_login_token"]["enabled"])
+ self.assertTrue(capabilities["m.get_login_token"]["enabled"])
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
index 41c1a16e6eff..8f518761700c 100644
--- a/tests/rest/client/test_login.py
+++ b/tests/rest/client/test_login.py
@@ -446,20 +446,16 @@ def test_require_approval(self) -> None:
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)
- def test_get_login_flows_with_msc3882_disabled(self) -> None:
+ def test_get_login_flows_with_login_via_existing_disabled(self) -> None:
"""GET /login should return m.login.token without get_login_token true"""
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)
flows = {flow["type"]: flow for flow in channel.json_body["flows"]}
- self.assertTrue(
- "m.login.token" not in flows
- or "org.matrix.msc3882.get_login_token" not in flows["m.login.token"]
- or not flows["m.login.token"]["org.matrix.msc3882.get_login_token"]
- )
+ self.assertNotIn("m.login.token", flows)
- @override_config({"experimental_features": {"msc3882_enabled": True}})
- def test_get_login_flows_with_msc3882_enabled(self) -> None:
+ @override_config({"login_via_existing_session": {"enabled": True}})
+ def test_get_login_flows_with_login_via_existing_enabled(self) -> None:
"""GET /login should return m.login.token without get_login_token true"""
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)
@@ -467,7 +463,7 @@ def test_get_login_flows_with_msc3882_enabled(self) -> None:
self.assertCountEqual(
channel.json_body["flows"],
[
- {"type": "m.login.token", "org.matrix.msc3882.get_login_token": True},
+ {"type": "m.login.token", "m.get_login_token": True},
{"type": "m.login.password"},
{"type": "m.login.application_service"},
],
diff --git a/tests/rest/client/test_login_token_request.py b/tests/rest/client/test_login_token_request.py
index cdf4134cbed1..c7a848671525 100644
--- a/tests/rest/client/test_login_token_request.py
+++ b/tests/rest/client/test_login_token_request.py
@@ -55,12 +55,12 @@ def test_disabled(self) -> None:
channel = self.make_request("POST", endpoint, {}, access_token=token)
self.assertEqual(channel.code, 404)
- @override_config({"experimental_features": {"msc3882_enabled": True}})
+ @override_config({"login_via_existing_session": {"enabled": True}})
def test_require_auth(self) -> None:
channel = self.make_request("POST", endpoint, {}, access_token=None)
self.assertEqual(channel.code, 401)
- @override_config({"experimental_features": {"msc3882_enabled": True}})
+ @override_config({"login_via_existing_session": {"enabled": True}})
def test_uia_on(self) -> None:
user_id = self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
@@ -95,7 +95,7 @@ def test_uia_on(self) -> None:
self.assertEqual(channel.json_body["user_id"], user_id)
@override_config(
- {"experimental_features": {"msc3882_enabled": True, "msc3882_ui_auth": False}}
+ {"login_via_existing_session": {"enabled": True, "require_ui_auth": False}}
)
def test_uia_off(self) -> None:
user_id = self.register_user(self.user, self.password)
@@ -117,10 +117,10 @@ def test_uia_off(self) -> None:
@override_config(
{
- "experimental_features": {
- "msc3882_enabled": True,
- "msc3882_ui_auth": False,
- "msc3882_token_timeout": "15s",
+ "login_via_existing_session": {
+ "enabled": True,
+ "require_ui_auth": False,
+ "token_timeout": "15s",
}
}
)
From ed9c582eaed1a99d179da2267a7d391b58b8f667 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 24 May 2023 10:06:53 -0400
Subject: [PATCH 05/17] Update other unstable identifiers.
---
synapse/rest/client/login_token_request.py | 3 +--
tests/rest/client/test_login_token_request.py | 16 ++++++++--------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index 44501f3614a7..b6c6d8f2b041 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -48,7 +48,7 @@ class LoginTokenRequestServlet(RestServlet):
"""
PATTERNS = client_patterns(
- "/org.matrix.msc3882/login/get_token$", releases=[], v1=False, unstable=True
+ "/login/get_token$", releases=["v1"], v1=False, unstable=False
)
def __init__(self, hs: "HomeServer"):
@@ -77,7 +77,6 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
login_token = await self.auth_handler.create_login_token_for_user_id(
user_id=requester.user.to_string(),
- auth_provider_id="org.matrix.msc3882.get_login_token",
duration_ms=self.token_timeout,
)
diff --git a/tests/rest/client/test_login_token_request.py b/tests/rest/client/test_login_token_request.py
index c7a848671525..d14612cafa84 100644
--- a/tests/rest/client/test_login_token_request.py
+++ b/tests/rest/client/test_login_token_request.py
@@ -22,7 +22,7 @@
from tests import unittest
from tests.unittest import override_config
-endpoint = "/_matrix/client/unstable/org.matrix.msc3882/login/get_token"
+GET_TOKEN_ENDPOINT = "/_matrix/client/v1/login/get_token"
class LoginTokenRequestServletTestCase(unittest.HomeserverTestCase):
@@ -46,18 +46,18 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.password = "password"
def test_disabled(self) -> None:
- channel = self.make_request("POST", endpoint, {}, access_token=None)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=None)
self.assertEqual(channel.code, 404)
self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
- channel = self.make_request("POST", endpoint, {}, access_token=token)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
self.assertEqual(channel.code, 404)
@override_config({"login_via_existing_session": {"enabled": True}})
def test_require_auth(self) -> None:
- channel = self.make_request("POST", endpoint, {}, access_token=None)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=None)
self.assertEqual(channel.code, 401)
@override_config({"login_via_existing_session": {"enabled": True}})
@@ -65,7 +65,7 @@ def test_uia_on(self) -> None:
user_id = self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
- channel = self.make_request("POST", endpoint, {}, access_token=token)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
self.assertEqual(channel.code, 401)
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
@@ -80,7 +80,7 @@ def test_uia_on(self) -> None:
},
}
- channel = self.make_request("POST", endpoint, uia, access_token=token)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, uia, access_token=token)
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["expires_in_ms"], 300000)
@@ -101,7 +101,7 @@ def test_uia_off(self) -> None:
user_id = self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
- channel = self.make_request("POST", endpoint, {}, access_token=token)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["expires_in_ms"], 300000)
@@ -128,6 +128,6 @@ def test_expires_in(self) -> None:
self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
- channel = self.make_request("POST", endpoint, {}, access_token=token)
+ channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["expires_in_ms"], 15000)
From ec64ac80896045422a276dfff8c14714da5ac789 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 24 May 2023 10:06:33 -0400
Subject: [PATCH 06/17] Add ratelimiting.
---
synapse/rest/client/login_token_request.py | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index b6c6d8f2b041..a3c2704493e2 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -15,6 +15,7 @@
import logging
from typing import TYPE_CHECKING, Tuple
+from synapse.api.ratelimiting import Ratelimiter
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
from synapse.http.site import SynapseRequest
@@ -54,13 +55,19 @@ class LoginTokenRequestServlet(RestServlet):
def __init__(self, hs: "HomeServer"):
super().__init__()
self.auth = hs.get_auth()
- self.store = hs.get_datastores().main
- self.clock = hs.get_clock()
- self.server_name = hs.config.server.server_name
+ self._main_store = hs.get_datastores().main
self.auth_handler = hs.get_auth_handler()
self.token_timeout = hs.config.auth.login_via_existing_token_timeout
self._require_ui_auth = hs.config.auth.login_via_existing_require_ui_auth
+ # An aggressive ratelimiter.
+ self._ratelimiter = Ratelimiter(
+ store=self._main_store,
+ clock=hs.get_clock(),
+ rate_hz=1 / 60,
+ burst_count=1,
+ )
+
@interactive_auth_handler
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)
@@ -75,6 +82,10 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
can_skip_ui_auth=False, # Don't allow skipping of UI auth
)
+ # Ensure that this endpoint isn't being used too often. (Ensure this is
+ # done *after* UI auth.)
+ await self._ratelimiter.ratelimit(None, requester.user.to_string().lower())
+
login_token = await self.auth_handler.create_login_token_for_user_id(
user_id=requester.user.to_string(),
duration_ms=self.token_timeout,
From 75b0e1a79ff2422f8b0d8acdf9036ef347c013e1 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 24 May 2023 10:41:07 -0400
Subject: [PATCH 07/17] Move ui_auth docs under session management.
---
.../configuration/config_documentation.md | 43 +++++++++----------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md
index 93b132b6e4cd..d61492ba6a34 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -2570,7 +2570,28 @@ Example configuration:
```yaml
nonrefreshable_access_token_lifetime: 24h
```
+---
+### `ui_auth`
+
+The amount of time to allow a user-interactive authentication session to be active.
+
+This defaults to 0, meaning the user is queried for their credentials
+before every action, but this can be overridden to allow a single
+validation to be re-used. This weakens the protections afforded by
+the user-interactive authentication process, by allowing for multiple
+(and potentially different) operations to use the same validation session.
+
+This is ignored for potentially "dangerous" operations (including
+deactivating an account, modifying an account password, and
+adding a 3PID).
+Use the `session_timeout` sub-option here to change the time allowed for credential validation.
+
+Example configuration:
+```yaml
+ui_auth:
+ session_timeout: "15s"
+```
---
## Metrics
Config options related to metrics.
@@ -3415,28 +3436,6 @@ password_config:
require_uppercase: true
```
---
-### `ui_auth`
-
-The amount of time to allow a user-interactive authentication session to be active.
-
-This defaults to 0, meaning the user is queried for their credentials
-before every action, but this can be overridden to allow a single
-validation to be re-used. This weakens the protections afforded by
-the user-interactive authentication process, by allowing for multiple
-(and potentially different) operations to use the same validation session.
-
-This is ignored for potentially "dangerous" operations (including
-deactivating an account, modifying an account password, and
-adding a 3PID).
-
-Use the `session_timeout` sub-option here to change the time allowed for credential validation.
-
-Example configuration:
-```yaml
-ui_auth:
- session_timeout: "15s"
-```
----
## Push
Configuration settings related to push notifications
From ac0fb703bdb67bba4c4128543526850a52609fa4 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 24 May 2023 10:44:57 -0400
Subject: [PATCH 08/17] Add documentation.
---
.../configuration/config_documentation.md | 25 +++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md
index d61492ba6a34..abeed923b768 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -2582,8 +2582,8 @@ the user-interactive authentication process, by allowing for multiple
(and potentially different) operations to use the same validation session.
This is ignored for potentially "dangerous" operations (including
-deactivating an account, modifying an account password, and
-adding a 3PID).
+deactivating an account, modifying an account password, adding a 3PID,
+and minting additional login tokens).
Use the `session_timeout` sub-option here to change the time allowed for credential validation.
@@ -2593,6 +2593,27 @@ ui_auth:
session_timeout: "15s"
```
---
+### `login_via_existing_session`
+
+Matrix supports the ability of an existing session to mint a login token for
+another client.
+
+Synapse disables this by default as it has security ramifications.
+
+The duration of time the generated token is valid for can be configured with the
+`token_timeout` sub-option.
+
+User-interactive authentication is required when this is enabled unless the
+`require_ui_auth` sub-option is set to `False`.
+
+Example configuration:
+```yaml
+login_via_existing_session:
+ enabled: true
+ require_ui_auth: false
+ token_timeout: "5m"
+```
+---
## Metrics
Config options related to metrics.
From c17e16dff89ac6d5f8f2390b71c6f3ed2056bcff Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Thu, 25 May 2023 13:13:04 +0100
Subject: [PATCH 09/17] Reference stable feature in changelog
---
changelog.d/15388.feature | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/changelog.d/15388.feature b/changelog.d/15388.feature
index cdfd11164786..6cc55cafa2e1 100644
--- a/changelog.d/15388.feature
+++ b/changelog.d/15388.feature
@@ -1 +1 @@
-Experimental implementation of revision 1 of [MSC3882](https://github.com/matrix-org/matrix-spec-proposals/pull/3882) to allow an existing device/session to generate a login token for use on a new device/session.
\ No newline at end of file
+Stable support for [MSC3882](https://github.com/matrix-org/matrix-spec-proposals/pull/3882) to allow an existing device/session to generate a login token for use on a new device/session.
\ No newline at end of file
From 73e0ca2f85bd113bac1cee4ce9999c00ff3b476b Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Thu, 25 May 2023 13:28:38 +0100
Subject: [PATCH 10/17] Reinstate support for unstable revision 0 of MSC3882
---
synapse/rest/client/login_token_request.py | 12 +++++++++---
synapse/rest/client/versions.py | 2 ++
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index a3c2704493e2..8cfc2fc48990 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -48,9 +48,13 @@ class LoginTokenRequestServlet(RestServlet):
}
"""
- PATTERNS = client_patterns(
- "/login/get_token$", releases=["v1"], v1=False, unstable=False
- )
+ PATTERNS = [
+ *client_patterns("/login/get_token$", releases=["v1"], v1=False, unstable=False),
+ # TODO: this is no longer needed once unstable MSC3882 does not need to be supported:
+ *client_patterns(
+ "/org.matrix.msc3882/login/token$", releases=[], v1=False, unstable=True
+ ),
+ ]
def __init__(self, hs: "HomeServer"):
super().__init__()
@@ -95,6 +99,8 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
200,
{
"login_token": login_token,
+ # TODO: this is no longer needed once unstable MSC3882 does not need to be supported:
+ "expires_in": self.token_timeout // 1000,
"expires_in_ms": self.token_timeout,
},
)
diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py
index 1eb11081a07f..c9f062ae7957 100644
--- a/synapse/rest/client/versions.py
+++ b/synapse/rest/client/versions.py
@@ -115,6 +115,8 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
"fi.mau.msc2659.stable": True, # TODO: remove when "v1.7" is added above
# Adds support for remotely enabling/disabling pushers, as per MSC3881
"org.matrix.msc3881": self.config.experimental.msc3881_enabled,
+ # TODO: this is no longer needed once unstable MSC3882 does not need to be supported:
+ "org.matrix.msc3882": self.config.auth.login_via_existing_enabled,
# Adds support for filtering /messages by event relation.
"org.matrix.msc3874": self.config.experimental.msc3874_enabled,
# Adds support for simple HTTP rendezvous as per MSC3886
From 1d41bf1f78bf93a973c0e65b7262701b6f52207a Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Thu, 25 May 2023 13:31:04 +0100
Subject: [PATCH 11/17] Cleaner diff
---
synapse/rest/client/versions.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py
index c9f062ae7957..547bf34df15e 100644
--- a/synapse/rest/client/versions.py
+++ b/synapse/rest/client/versions.py
@@ -113,10 +113,10 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
"fi.mau.msc2815": self.config.experimental.msc2815_enabled,
# Adds a ping endpoint for appservices to check HS->AS connection
"fi.mau.msc2659.stable": True, # TODO: remove when "v1.7" is added above
- # Adds support for remotely enabling/disabling pushers, as per MSC3881
- "org.matrix.msc3881": self.config.experimental.msc3881_enabled,
# TODO: this is no longer needed once unstable MSC3882 does not need to be supported:
"org.matrix.msc3882": self.config.auth.login_via_existing_enabled,
+ # Adds support for remotely enabling/disabling pushers, as per MSC3881
+ "org.matrix.msc3881": self.config.experimental.msc3881_enabled,
# Adds support for filtering /messages by event relation.
"org.matrix.msc3874": self.config.experimental.msc3874_enabled,
# Adds support for simple HTTP rendezvous as per MSC3886
From 17f73cd70251832334b9ebf532c790c19b7c36af Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Thu, 25 May 2023 13:39:06 +0100
Subject: [PATCH 12/17] Add tests for unstable support whilst still relied on
---
tests/rest/client/test_login_token_request.py | 28 ++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/tests/rest/client/test_login_token_request.py b/tests/rest/client/test_login_token_request.py
index d14612cafa84..d2344aa9ab4a 100644
--- a/tests/rest/client/test_login_token_request.py
+++ b/tests/rest/client/test_login_token_request.py
@@ -15,7 +15,7 @@
from twisted.test.proto_helpers import MemoryReactor
from synapse.rest import admin
-from synapse.rest.client import login, login_token_request
+from synapse.rest.client import login, login_token_request, versions
from synapse.server import HomeServer
from synapse.util import Clock
@@ -30,6 +30,7 @@ class LoginTokenRequestServletTestCase(unittest.HomeserverTestCase):
login.register_servlets,
admin.register_servlets,
login_token_request.register_servlets,
+ versions.register_servlets, # TODO: remove once unstable revision 0 support is removed
]
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
@@ -131,3 +132,28 @@ def test_expires_in(self) -> None:
channel = self.make_request("POST", GET_TOKEN_ENDPOINT, {}, access_token=token)
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["expires_in_ms"], 15000)
+
+ @override_config(
+ {
+ "login_via_existing_session": {
+ "enabled": True,
+ "require_ui_auth": False,
+ "token_timeout": "15s",
+ }
+ }
+ )
+ def test_unstable_support(self) -> None:
+ # TODO: remove support for unstable MSC3882 is no longer needed
+
+ # check feature is advertised in versions response:
+ channel = self.make_request("GET", "/_matrix/client/versions", {}, access_token=None)
+ self.assertEqual(channel.code, 200)
+ self.assertEqual(channel.json_body["unstable_features"]["org.matrix.msc3882"], True)
+
+ self.register_user(self.user, self.password)
+ token = self.login(self.user, self.password)
+
+ # check feature is available via the unstable endpoint and returns an expires_in value in seconds
+ channel = self.make_request("POST", "/_matrix/client/unstable/org.matrix.msc3882/login/token", {}, access_token=token)
+ self.assertEqual(channel.code, 200)
+ self.assertEqual(channel.json_body["expires_in"], 15)
From 77a6ee6bd8f521fa9113f537e68e9d42a6c07ba6 Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Thu, 25 May 2023 13:40:11 +0100
Subject: [PATCH 13/17] Lint
---
synapse/rest/client/login_token_request.py | 4 +++-
tests/rest/client/test_login_token_request.py | 17 +++++++++++++----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index 8cfc2fc48990..282d2d308f3f 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -49,7 +49,9 @@ class LoginTokenRequestServlet(RestServlet):
"""
PATTERNS = [
- *client_patterns("/login/get_token$", releases=["v1"], v1=False, unstable=False),
+ *client_patterns(
+ "/login/get_token$", releases=["v1"], v1=False, unstable=False
+ ),
# TODO: this is no longer needed once unstable MSC3882 does not need to be supported:
*client_patterns(
"/org.matrix.msc3882/login/token$", releases=[], v1=False, unstable=True
diff --git a/tests/rest/client/test_login_token_request.py b/tests/rest/client/test_login_token_request.py
index d2344aa9ab4a..f05e619aa86d 100644
--- a/tests/rest/client/test_login_token_request.py
+++ b/tests/rest/client/test_login_token_request.py
@@ -30,7 +30,7 @@ class LoginTokenRequestServletTestCase(unittest.HomeserverTestCase):
login.register_servlets,
admin.register_servlets,
login_token_request.register_servlets,
- versions.register_servlets, # TODO: remove once unstable revision 0 support is removed
+ versions.register_servlets, # TODO: remove once unstable revision 0 support is removed
]
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
@@ -146,14 +146,23 @@ def test_unstable_support(self) -> None:
# TODO: remove support for unstable MSC3882 is no longer needed
# check feature is advertised in versions response:
- channel = self.make_request("GET", "/_matrix/client/versions", {}, access_token=None)
+ channel = self.make_request(
+ "GET", "/_matrix/client/versions", {}, access_token=None
+ )
self.assertEqual(channel.code, 200)
- self.assertEqual(channel.json_body["unstable_features"]["org.matrix.msc3882"], True)
+ self.assertEqual(
+ channel.json_body["unstable_features"]["org.matrix.msc3882"], True
+ )
self.register_user(self.user, self.password)
token = self.login(self.user, self.password)
# check feature is available via the unstable endpoint and returns an expires_in value in seconds
- channel = self.make_request("POST", "/_matrix/client/unstable/org.matrix.msc3882/login/token", {}, access_token=token)
+ channel = self.make_request(
+ "POST",
+ "/_matrix/client/unstable/org.matrix.msc3882/login/token",
+ {},
+ access_token=token,
+ )
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["expires_in"], 15)
From 210c22dabeb8dd3930470a9a9da238ef8750e694 Mon Sep 17 00:00:00 2001
From: Hugh Nimmo-Smith <hughns@element.io>
Date: Tue, 30 May 2023 17:36:51 +0100
Subject: [PATCH 14/17] Fix login flow parameter name
It is `get_login_flow` not `m.get_login_flow` ref https://spec.matrix.org/v1.7/client-server-api/#get_matrixclientv3login
---
synapse/rest/client/login.py | 2 +-
tests/rest/client/test_login.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index fed61abecbd5..7db5ce899422 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -166,7 +166,7 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
tokenTypeFlow: Dict[str, Any] = {"type": LoginRestServlet.TOKEN_TYPE}
# If the login token flow is enabled advertise the get_login_token flag.
if self._get_login_token_enabled:
- tokenTypeFlow["m.get_login_token"] = True
+ tokenTypeFlow["get_login_token"] = True
flows.append(tokenTypeFlow)
flows.extend({"type": t} for t in self.auth_handler.get_supported_login_types())
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
index 8f518761700c..ef292dff3835 100644
--- a/tests/rest/client/test_login.py
+++ b/tests/rest/client/test_login.py
@@ -463,7 +463,7 @@ def test_get_login_flows_with_login_via_existing_enabled(self) -> None:
self.assertCountEqual(
channel.json_body["flows"],
[
- {"type": "m.login.token", "m.get_login_token": True},
+ {"type": "m.login.token", "get_login_token": True},
{"type": "m.login.password"},
{"type": "m.login.application_service"},
],
From b9ab999750cf93db6a31a55adafaefd5082b2e4e Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Tue, 30 May 2023 13:06:49 -0400
Subject: [PATCH 15/17] Fixes for develop
Fixes for compatibility with #15582.
---
synapse/config/experimental.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index 29c57f77704b..a9e002cf08f8 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -192,10 +192,10 @@ def check_config_conflicts(self, root: RootConfig) -> None:
("captcha", "enable_registration_captcha"),
)
- if root.experimental.msc3882_enabled:
+ if root.auth.login_via_existing_enabled:
raise ConfigError(
- "MSC3882 cannot be enabled when OAuth delegation is enabled",
- ("experimental_features", "msc3882_enabled"),
+ "Login via existing session cannot be enabled when OAuth delegation is enabled",
+ ("login_via_existing_session", "enabled"),
)
if root.registration.refresh_token_lifetime:
From 88c70598a07215e82b8febea3e46ed07987417fd Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 30 May 2023 14:02:51 -0400
Subject: [PATCH 16/17] Fix-up test.
---
tests/config/test_oauth_delegation.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/config/test_oauth_delegation.py b/tests/config/test_oauth_delegation.py
index 2ead721b00fc..f57c813a581e 100644
--- a/tests/config/test_oauth_delegation.py
+++ b/tests/config/test_oauth_delegation.py
@@ -228,8 +228,8 @@ def test_jwt_auth_cannot_be_enabled(self) -> None:
with self.assertRaises(ConfigError):
self.parse_config()
- def test_msc3882_auth_cannot_be_enabled(self) -> None:
- self.config_dict["experimental_features"]["msc3882_enabled"] = True
+ def test_login_via_existing_session_cannot_be_enabled(self) -> None:
+ self.config_dict["login_via_existing_session"] = {"enabled": True}
with self.assertRaises(ConfigError):
self.parse_config()
From 2bd6f72b3c01ceba012ac1c006c6d93dc3bed03b Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 1 Jun 2023 07:56:13 -0400
Subject: [PATCH 17/17] Additional comments and clarifications.
---
docs/usage/configuration/config_documentation.md | 3 ++-
synapse/rest/client/login.py | 8 ++++++++
synapse/rest/client/login_token_request.py | 5 ++++-
tests/rest/client/test_login.py | 4 ++--
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md
index d9a3537ed9f0..0cf6e075ff11 100644
--- a/docs/usage/configuration/config_documentation.md
+++ b/docs/usage/configuration/config_documentation.md
@@ -2598,7 +2598,8 @@ ui_auth:
Matrix supports the ability of an existing session to mint a login token for
another client.
-Synapse disables this by default as it has security ramifications.
+Synapse disables this by default as it has security ramifications -- a malicious
+client could use the mechanism to spawn more than one session.
The duration of time the generated token is valid for can be configured with the
`token_timeout` sub-option.
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index 37a065932cfc..6493b00bb803 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -162,6 +162,14 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
# SSO requires a login token to be generated, so we need to advertise that flow
support_login_token_flow = True
+ # While it's valid for us to advertise this login type generally,
+ # synapse currently only gives out these tokens as part of the
+ # SSO login flow or as part of login via an existing session.
+ #
+ # Generally we don't want to advertise login flows that clients
+ # don't know how to implement, since they (currently) will always
+ # fall back to the fallback API if they don't understand one of the
+ # login flow types returned.
if support_login_token_flow:
tokenTypeFlow: Dict[str, Any] = {"type": LoginRestServlet.TOKEN_TYPE}
# If the login token flow is enabled advertise the get_login_token flag.
diff --git a/synapse/rest/client/login_token_request.py b/synapse/rest/client/login_token_request.py
index 282d2d308f3f..b1629f94a5f8 100644
--- a/synapse/rest/client/login_token_request.py
+++ b/synapse/rest/client/login_token_request.py
@@ -66,7 +66,10 @@ def __init__(self, hs: "HomeServer"):
self.token_timeout = hs.config.auth.login_via_existing_token_timeout
self._require_ui_auth = hs.config.auth.login_via_existing_require_ui_auth
- # An aggressive ratelimiter.
+ # Ratelimit aggressively to a maxmimum of 1 request per minute.
+ #
+ # This endpoint can be used to spawn additional sessions and could be
+ # abused by a malicious client to create many sessions.
self._ratelimiter = Ratelimiter(
store=self._main_store,
clock=hs.get_clock(),
diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py
index ef292dff3835..f3c3bc69a97a 100644
--- a/tests/rest/client/test_login.py
+++ b/tests/rest/client/test_login.py
@@ -447,7 +447,7 @@ def test_require_approval(self) -> None:
)
def test_get_login_flows_with_login_via_existing_disabled(self) -> None:
- """GET /login should return m.login.token without get_login_token true"""
+ """GET /login should return m.login.token without get_login_token"""
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)
@@ -456,7 +456,7 @@ def test_get_login_flows_with_login_via_existing_disabled(self) -> None:
@override_config({"login_via_existing_session": {"enabled": True}})
def test_get_login_flows_with_login_via_existing_enabled(self) -> None:
- """GET /login should return m.login.token without get_login_token true"""
+ """GET /login should return m.login.token with get_login_token true"""
channel = self.make_request("GET", "/_matrix/client/r0/login")
self.assertEqual(channel.code, 200, channel.result)