From f41e21da54373acfc5b3bdcb23de15ecf53cfe47 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 13 Jun 2025 10:25:39 +0200 Subject: [PATCH 1/4] Support the stable endpoint for MSC2965 --- synapse/rest/client/auth_metadata.py | 16 +++++++++++----- tests/rest/client/test_auth_metadata.py | 21 +++++++++++++-------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/synapse/rest/client/auth_metadata.py b/synapse/rest/client/auth_metadata.py index 5444a89be67..0691c0a51ca 100644 --- a/synapse/rest/client/auth_metadata.py +++ b/synapse/rest/client/auth_metadata.py @@ -70,11 +70,17 @@ class AuthMetadataServlet(RestServlet): Advertises the OAuth 2.0 server metadata for the homeserver. """ - PATTERNS = client_patterns( - "/org.matrix.msc2965/auth_metadata$", - unstable=True, - releases=(), - ) + PATTERNS = [ + *client_patterns( + "/auth_metadata$", + releases=("v1",), + ), + *client_patterns( + "/org.matrix.msc2965/auth_metadata$", + unstable=True, + releases=(), + ), + ] def __init__(self, hs: "HomeServer"): super().__init__() diff --git a/tests/rest/client/test_auth_metadata.py b/tests/rest/client/test_auth_metadata.py index a935533b099..c13d4106361 100644 --- a/tests/rest/client/test_auth_metadata.py +++ b/tests/rest/client/test_auth_metadata.py @@ -18,8 +18,11 @@ # [This file includes modifications made by New Vector Limited] # from http import HTTPStatus +from typing import ClassVar from unittest.mock import AsyncMock +from parameterized import parameterized_class + from synapse.rest.client import auth_metadata from tests.unittest import HomeserverTestCase, override_config, skip_unless @@ -85,17 +88,22 @@ def test_returns_issuer_when_oidc_enabled(self) -> None: req_mock.assert_not_called() +@parameterized_class( + ("endpoint",), + [ + ("/_matrix/client/unstable/org.matrix.msc2965/auth_metadata",), + ("/_matrix/client/v1/auth_metadata",), + ], +) class AuthMetadataTestCase(HomeserverTestCase): + endpoint: ClassVar[str] servlets = [ auth_metadata.register_servlets, ] def test_returns_404_when_msc3861_disabled(self) -> None: # Make an unauthenticated request for the discovery info. - channel = self.make_request( - "GET", - "/_matrix/client/unstable/org.matrix.msc2965/auth_metadata", - ) + channel = self.make_request("GET", self.endpoint) self.assertEqual(channel.code, HTTPStatus.NOT_FOUND) @skip_unless(HAS_AUTHLIB, "requires authlib") @@ -124,10 +132,7 @@ def test_returns_issuer_when_oidc_enabled(self) -> None: ) self.hs.get_proxied_http_client().get_json = req_mock # type: ignore[method-assign] - channel = self.make_request( - "GET", - "/_matrix/client/unstable/org.matrix.msc2965/auth_metadata", - ) + channel = self.make_request("GET", self.endpoint) self.assertEqual(channel.code, HTTPStatus.OK) self.assertEqual( From 9abd4044b9fc1484058173748c3e109a2e3d36a7 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 13 Jun 2025 11:39:08 +0200 Subject: [PATCH 2/4] Support for the stable scopes in MSC3861 delegation --- synapse/api/auth/msc3861_delegated.py | 34 ++++--- tests/handlers/test_oauth_delegation.py | 119 +++++++----------------- 2 files changed, 50 insertions(+), 103 deletions(-) diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py index ad5d59eef13..22a116b3504 100644 --- a/synapse/api/auth/msc3861_delegated.py +++ b/synapse/api/auth/msc3861_delegated.py @@ -20,7 +20,7 @@ # import logging from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional +from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Set from urllib.parse import urlencode from authlib.oauth2 import ClientAuth @@ -38,7 +38,6 @@ AuthError, HttpResponseException, InvalidClientTokenError, - OAuthInsufficientScopeError, SynapseError, UnrecognizedRequestError, ) @@ -65,9 +64,10 @@ # Scope as defined by MSC2967 # https://github.com/matrix-org/matrix-spec-proposals/pull/2967 -SCOPE_MATRIX_API = "urn:matrix:org.matrix.msc2967.client:api:*" -SCOPE_MATRIX_GUEST = "urn:matrix:org.matrix.msc2967.client:api:guest" -SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:org.matrix.msc2967.client:device:" +UNSTABLE_SCOPE_MATRIX_API = "urn:matrix:org.matrix.msc2967.client:api:*" +UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:org.matrix.msc2967.client:device:" +STABLE_SCOPE_MATRIX_API = "urn:matrix:client:api:*" +STABLE_SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:client:device:" # Scope which allows access to the Synapse admin API SCOPE_SYNAPSE_ADMIN = "urn:synapse:admin:*" @@ -432,9 +432,6 @@ async def _wrapped_get_user_by_req( if access_token != self._admin_token(): await self._record_request(request, requester) - if not allow_guest and requester.is_guest: - raise OAuthInsufficientScopeError([SCOPE_MATRIX_API]) - request.requester = requester return requester @@ -504,10 +501,11 @@ async def get_user_by_access_token( scope: List[str] = introspection_result.get_scope_list() # Determine type of user based on presence of particular scopes - has_user_scope = SCOPE_MATRIX_API in scope - has_guest_scope = SCOPE_MATRIX_GUEST in scope + has_user_scope = ( + UNSTABLE_SCOPE_MATRIX_API in scope or STABLE_SCOPE_MATRIX_API in scope + ) - if not has_user_scope and not has_guest_scope: + if not has_user_scope: raise InvalidClientTokenError("No scope in token granting user rights") # Match via the sub claim @@ -555,11 +553,12 @@ async def get_user_by_access_token( # 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) - ] + device_ids: Set[str] = set() + for tok in scope: + if tok.startswith(UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX): + device_ids.add(tok[len(UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX) :]) + elif tok.startswith(STABLE_SCOPE_MATRIX_DEVICE_PREFIX): + device_ids.add(tok[len(STABLE_SCOPE_MATRIX_DEVICE_PREFIX) :]) if len(device_ids) > 1: raise AuthError( @@ -567,7 +566,7 @@ async def get_user_by_access_token( "Multiple device IDs in scope", ) - device_id = device_ids[0] if device_ids else None + device_id = next(iter(device_ids), None) if device_id is not None: # Sanity check the device_id @@ -593,5 +592,4 @@ async def get_user_by_access_token( user_id=user_id, device_id=device_id, scope=scope, - is_guest=(has_guest_scope and not has_user_scope), ) diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py index 934bfee0bce..8aa890a9857 100644 --- a/tests/handlers/test_oauth_delegation.py +++ b/tests/handlers/test_oauth_delegation.py @@ -21,10 +21,11 @@ from http import HTTPStatus from io import BytesIO -from typing import Any, Dict, Optional, Union +from typing import Any, ClassVar, Dict, Optional, Union from unittest.mock import ANY, AsyncMock, Mock from urllib.parse import parse_qs +from parameterized.parameterized import parameterized_class from signedjson.key import ( encode_verify_key_base64, generate_signing_key, @@ -40,7 +41,6 @@ AuthError, Codes, InvalidClientTokenError, - OAuthInsufficientScopeError, SynapseError, ) from synapse.appservice import ApplicationService @@ -72,11 +72,7 @@ INTROSPECTION_ENDPOINT = ISSUER + "introspect" SYNAPSE_ADMIN_SCOPE = "urn:synapse:admin:*" -MATRIX_USER_SCOPE = "urn:matrix:org.matrix.msc2967.client:api:*" -MATRIX_GUEST_SCOPE = "urn:matrix:org.matrix.msc2967.client:api:guest" -MATRIX_DEVICE_SCOPE_PREFIX = "urn:matrix:org.matrix.msc2967.client:device:" DEVICE = "AABBCCDD" -MATRIX_DEVICE_SCOPE = MATRIX_DEVICE_SCOPE_PREFIX + DEVICE SUBJECT = "abc-def-ghi" USERNAME = "test-user" USER_ID = "@" + USERNAME + ":" + SERVER_NAME @@ -106,7 +102,24 @@ async def get_json(url: str) -> JsonDict: @skip_unless(HAS_AUTHLIB, "requires authlib") +@parameterized_class( + ("device_scope_prefix", "api_scope"), + [ + ("urn:matrix:client:device:", "urn:matrix:client:api:*"), + ( + "urn:matrix:org.matrix.msc2967.client:device:", + "urn:matrix:org.matrix.msc2967.client:api:*", + ), + ], +) class MSC3861OAuthDelegation(HomeserverTestCase): + device_scope_prefix: ClassVar[str] + api_scope: ClassVar[str] + + @property + def device_scope(self) -> str: + return self.device_scope_prefix + DEVICE + servlets = [ account.register_servlets, devices.register_servlets, @@ -208,7 +221,7 @@ def test_active_user_no_subject(self) -> None: self.http_client.request = AsyncMock( return_value=FakeResponse.json( code=200, - payload={"active": True, "scope": " ".join([MATRIX_USER_SCOPE])}, + payload={"active": True, "scope": " ".join([self.api_scope])}, ) ) request = Mock(args={}) @@ -230,7 +243,7 @@ def test_active_no_user_scope(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_DEVICE_SCOPE]), + "scope": " ".join([self.device_scope]), }, ) ) @@ -277,7 +290,7 @@ def test_active_admin(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE]), + "scope": " ".join([SYNAPSE_ADMIN_SCOPE, self.api_scope]), "username": USERNAME, }, ) @@ -307,9 +320,7 @@ def test_active_admin_highest_privilege(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join( - [SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE, MATRIX_GUEST_SCOPE] - ), + "scope": " ".join([SYNAPSE_ADMIN_SCOPE, self.api_scope]), "username": USERNAME, }, ) @@ -339,7 +350,7 @@ def test_active_user(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_USER_SCOPE]), + "scope": " ".join([self.api_scope]), "username": USERNAME, }, ) @@ -369,7 +380,7 @@ def test_active_user_with_device(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]), + "scope": " ".join([self.api_scope, self.device_scope]), "username": USERNAME, }, ) @@ -399,7 +410,7 @@ def test_active_user_with_device_explicit_device_id(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_USER_SCOPE]), + "scope": " ".join([self.api_scope]), "device_id": DEVICE, "username": USERNAME, }, @@ -439,9 +450,9 @@ def test_multiple_devices(self) -> None: "sub": SUBJECT, "scope": " ".join( [ - MATRIX_USER_SCOPE, - f"{MATRIX_DEVICE_SCOPE_PREFIX}AABBCC", - f"{MATRIX_DEVICE_SCOPE_PREFIX}DDEEFF", + self.api_scope, + f"{self.device_scope_prefix}AABBCC", + f"{self.device_scope_prefix}DDEEFF", ] ), "username": USERNAME, @@ -453,68 +464,6 @@ def test_multiple_devices(self) -> None: request.requestHeaders.getRawHeaders = mock_getRawHeaders() self.get_failure(self.auth.get_user_by_req(request), AuthError) - def test_active_guest_not_allowed(self) -> None: - """The handler should return an insufficient scope error.""" - - self.http_client.request = AsyncMock( - return_value=FakeResponse.json( - code=200, - payload={ - "active": True, - "sub": SUBJECT, - "scope": " ".join([MATRIX_GUEST_SCOPE, MATRIX_DEVICE_SCOPE]), - "username": USERNAME, - }, - ) - ) - request = Mock(args={}) - request.args[b"access_token"] = [b"mockAccessToken"] - request.requestHeaders.getRawHeaders = mock_getRawHeaders() - error = self.get_failure( - self.auth.get_user_by_req(request), OAuthInsufficientScopeError - ) - 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 - ) - self._assertParams() - self.assertEqual( - getattr(error.value, "headers", {})["WWW-Authenticate"], - 'Bearer error="insufficient_scope", scope="urn:matrix:org.matrix.msc2967.client:api:*"', - ) - - def test_active_guest_allowed(self) -> None: - """The handler should return a requester with guest user rights and a device ID.""" - - self.http_client.request = AsyncMock( - return_value=FakeResponse.json( - code=200, - payload={ - "active": True, - "sub": SUBJECT, - "scope": " ".join([MATRIX_GUEST_SCOPE, MATRIX_DEVICE_SCOPE]), - "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, allow_guest=True) - ) - 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 - ) - self._assertParams() - self.assertEqual(requester.user.to_string(), "@%s:%s" % (USERNAME, SERVER_NAME)) - self.assertEqual(requester.is_guest, True) - self.assertEqual( - get_awaitable_result(self.auth.is_server_admin(requester)), False - ) - self.assertEqual(requester.device_id, DEVICE) - def test_unavailable_introspection_endpoint(self) -> None: """The handler should return an internal server error.""" request = Mock(args={}) @@ -562,8 +511,8 @@ def test_cached_expired_introspection(self) -> None: "sub": SUBJECT, "scope": " ".join( [ - MATRIX_USER_SCOPE, - f"{MATRIX_DEVICE_SCOPE_PREFIX}AABBCC", + self.api_scope, + f"{self.device_scope_prefix}AABBCC", ] ), "username": USERNAME, @@ -613,7 +562,7 @@ def test_cross_signing(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]), + "scope": " ".join([self.api_scope, self.device_scope]), "username": USERNAME, }, ) @@ -784,7 +733,7 @@ def test_device_management_endpoints_removed(self) -> None: payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]), + "scope": " ".join([self.api_scope, self.device_scope]), "username": USERNAME, }, ) @@ -865,7 +814,7 @@ async def mock_http_client_request( code=200, payload={ "active": True, - "scope": MATRIX_USER_SCOPE, + "scope": self.api_scope, "sub": SUBJECT, "username": USERNAME, }, From cb9fe38407da4aa1e9391baa21a2f58c9a3ad2a0 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 13 Jun 2025 11:43:01 +0200 Subject: [PATCH 3/4] Newsfile --- changelog.d/18549.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/18549.feature diff --git a/changelog.d/18549.feature b/changelog.d/18549.feature new file mode 100644 index 00000000000..4d78ae57ab4 --- /dev/null +++ b/changelog.d/18549.feature @@ -0,0 +1 @@ +Support for the stable endpoint and scopes of [MSC3861](https://github.com/matrix-org/matrix-spec-proposals/pull/3861) & co. From 0e5034186b71f8bd8da283915828a1aecc16e671 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Mon, 4 Aug 2025 16:10:22 +0200 Subject: [PATCH 4/4] Support for stable scopes in the stable MAS integration --- synapse/api/auth/mas.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/synapse/api/auth/mas.py b/synapse/api/auth/mas.py index 00bad768564..40b4a5bd34b 100644 --- a/synapse/api/auth/mas.py +++ b/synapse/api/auth/mas.py @@ -13,7 +13,7 @@ # # import logging -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Optional, Set from urllib.parse import urlencode from synapse._pydantic_compat import ( @@ -57,8 +57,10 @@ # Scope as defined by MSC2967 # https://github.com/matrix-org/matrix-spec-proposals/pull/2967 -SCOPE_MATRIX_API = "urn:matrix:org.matrix.msc2967.client:api:*" -SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:org.matrix.msc2967.client:device:" +UNSTABLE_SCOPE_MATRIX_API = "urn:matrix:org.matrix.msc2967.client:api:*" +UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:org.matrix.msc2967.client:device:" +STABLE_SCOPE_MATRIX_API = "urn:matrix:client:api:*" +STABLE_SCOPE_MATRIX_DEVICE_PREFIX = "urn:matrix:client:device:" class ServerMetadata(BaseModel): @@ -334,7 +336,10 @@ async def get_user_by_access_token( scope = introspection_result.get_scope_set() # Determine type of user based on presence of particular scopes - if SCOPE_MATRIX_API not in scope: + if ( + UNSTABLE_SCOPE_MATRIX_API not in scope + and STABLE_SCOPE_MATRIX_API not in scope + ): raise InvalidClientTokenError( "Token doesn't grant access to the Matrix C-S API" ) @@ -366,11 +371,12 @@ async def get_user_by_access_token( # 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) - ] + device_ids: Set[str] = set() + for tok in scope: + if tok.startswith(UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX): + device_ids.add(tok[len(UNSTABLE_SCOPE_MATRIX_DEVICE_PREFIX) :]) + elif tok.startswith(STABLE_SCOPE_MATRIX_DEVICE_PREFIX): + device_ids.add(tok[len(STABLE_SCOPE_MATRIX_DEVICE_PREFIX) :]) if len(device_ids) > 1: raise AuthError( @@ -378,7 +384,7 @@ async def get_user_by_access_token( "Multiple device IDs in scope", ) - device_id = device_ids[0] if device_ids else None + device_id = next(iter(device_ids), None) if device_id is not None: # Sanity check the device_id