From 08c56c3acca4b3a4207c4a28b2b76b98a07403de Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Tue, 4 Mar 2025 14:08:44 +0100
Subject: [PATCH] Support getting the device ID explicitly from MAS (#18174)
The context for this is that the Matrix spec allows basically anything
in the device ID. With MSC3861, we're restricting this to strings that
can be represented as scopes.
Whilst this works well for next-gen auth sessions, compatibility/legacy
sessions still can have characters that can't be encoded (mainly spaces)
in them.
To work around that, we added in MAS a behaviour where the device_id is
given as an explicit property of the token introspection response, and
remove it from the scope.
Because we don't expect users to rollout new Synapse and MAS versions in
sync, we needed a way to 'advertise' support for this behaviour: the
easiest way to do that was through an extra header in the introspection
response.
On the longer term, I expect MAS and Synapse to move away from the
introspection endpoint, and instead define a specific API for Synapse ->
MAS communication.
PR on the MAS side:
https://github.com/element-hq/matrix-authentication-service/pull/4067
---
changelog.d/18174.misc | 1 +
synapse/api/auth/msc3861_delegated.py | 49 ++++++++++++++++---------
tests/handlers/test_oauth_delegation.py | 38 +++++++++++++++++++
3 files changed, 71 insertions(+), 17 deletions(-)
create mode 100644 changelog.d/18174.misc
diff --git a/changelog.d/18174.misc b/changelog.d/18174.misc
new file mode 100644
index 0000000000..b7c7c4db53
--- /dev/null
+++ b/changelog.d/18174.misc
@@ -0,0 +1 @@
+Support device IDs that can't be represented in a scope when delegating auth to Matrix Authentication Service 0.15.0+.
diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py
index f825b5c95e..e6bf271a1f 100644
--- a/synapse/api/auth/msc3861_delegated.py
+++ b/synapse/api/auth/msc3861_delegated.py
@@ -214,6 +214,9 @@ class MSC3861DelegatedAuth(BaseAuth):
"Content-Type": "application/x-www-form-urlencoded",
"User-Agent": str(self._http_client.user_agent, "utf-8"),
"Accept": "application/json",
+ # Tell MAS that we support reading the device ID as an explicit
+ # value, not encoded in the scope. This is supported by MAS 0.15+
+ "X-MAS-Supports-Device-Id": "1",
}
args = {"token": token, "token_type_hint": "access_token"}
@@ -409,29 +412,41 @@ class MSC3861DelegatedAuth(BaseAuth):
else:
user_id = UserID.from_string(user_id_str)
- # Find device_ids in scope
- # We only allow a single device_id in the scope, so we find them all in the
- # scope list, and raise if there are more than one. The OIDC server should be
- # the one enforcing valid scopes, so we raise a 500 if we find an invalid scope.
- device_ids = [
- tok[len(SCOPE_MATRIX_DEVICE_PREFIX) :]
- for tok in scope
- if tok.startswith(SCOPE_MATRIX_DEVICE_PREFIX)
- ]
-
- if len(device_ids) > 1:
- raise AuthError(
- 500,
- "Multiple device IDs in scope",
- )
+ # MAS 0.15+ will give us the device ID as an explicit value for compatibility sessions
+ # If present, we get it from here, if not we get it in thee scope
+ device_id = introspection_result.get("device_id")
+ if device_id is not None:
+ # We got the device ID explicitly, just sanity check that it's a string
+ if not isinstance(device_id, str):
+ raise AuthError(
+ 500,
+ "Invalid device ID in introspection result",
+ )
+ else:
+ # Find device_ids in scope
+ # We only allow a single device_id in the scope, so we find them all in the
+ # scope list, and raise if there are more than one. The OIDC server should be
+ # the one enforcing valid scopes, so we raise a 500 if we find an invalid scope.
+ device_ids = [
+ tok[len(SCOPE_MATRIX_DEVICE_PREFIX) :]
+ for tok in scope
+ if tok.startswith(SCOPE_MATRIX_DEVICE_PREFIX)
+ ]
+
+ if len(device_ids) > 1:
+ raise AuthError(
+ 500,
+ "Multiple device IDs in scope",
+ )
+
+ device_id = device_ids[0] if device_ids else None
- device_id = device_ids[0] if device_ids else None
if device_id is not None:
# Sanity check the device_id
if len(device_id) > 255 or len(device_id) < 1:
raise AuthError(
500,
- "Invalid device ID in scope",
+ "Invalid device ID in introspection result",
)
# Create the device on the fly if it does not exist
diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py
index b9d66a6c52..5f8c25557a 100644
--- a/tests/handlers/test_oauth_delegation.py
+++ b/tests/handlers/test_oauth_delegation.py
@@ -380,6 +380,44 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
)
self.assertEqual(requester.device_id, DEVICE)
+ def test_active_user_with_device_explicit_device_id(self) -> None:
+ """The handler should return a requester with normal user rights and a device ID, given explicitly, as supported by MAS 0.15+"""
+
+ self.http_client.request = AsyncMock(
+ return_value=FakeResponse.json(
+ code=200,
+ payload={
+ "active": True,
+ "sub": SUBJECT,
+ "scope": " ".join([MATRIX_USER_SCOPE]),
+ "device_id": DEVICE,
+ "username": USERNAME,
+ },
+ )
+ )
+ request = Mock(args={})
+ request.args[b"access_token"] = [b"mockAccessToken"]
+ request.requestHeaders.getRawHeaders = mock_getRawHeaders()
+ requester = self.get_success(self.auth.get_user_by_req(request))
+ self.http_client.get_json.assert_called_once_with(WELL_KNOWN)
+ self.http_client.request.assert_called_once_with(
+ method="POST", uri=INTROSPECTION_ENDPOINT, data=ANY, headers=ANY
+ )
+ # It should have called with the 'X-MAS-Supports-Device-Id: 1' header
+ self.assertEqual(
+ self.http_client.request.call_args[1]["headers"].getRawHeaders(
+ b"X-MAS-Supports-Device-Id",
+ ),
+ [b"1"],
+ )
+ self._assertParams()
+ self.assertEqual(requester.user.to_string(), "@%s:%s" % (USERNAME, SERVER_NAME))
+ self.assertEqual(requester.is_guest, False)
+ self.assertEqual(
+ get_awaitable_result(self.auth.is_server_admin(requester)), False
+ )
+ self.assertEqual(requester.device_id, DEVICE)
+
def test_multiple_devices(self) -> None:
"""The handler should raise an error if multiple devices are found in the scope."""
--
GitLab