Support for stable scopes in the stable MAS integration

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.

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 @@ logger = logging.getLogger(__name__)
 
 # 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 @@ class MasDelegatedAuth(BaseAuth):
         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 @@ class MasDelegatedAuth(BaseAuth):
             # 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 @@ class MasDelegatedAuth(BaseAuth):
                     "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

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
@@ -34,7 +34,6 @@ from synapse.api.errors import (
     AuthError,
     HttpResponseException,
     InvalidClientTokenError,
-    OAuthInsufficientScopeError,
     SynapseError,
     UnrecognizedRequestError,
 )
@@ -63,9 +62,10 @@ logger = logging.getLogger(__name__)
 
 # 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:*"
@@ -444,9 +444,6 @@ class MSC3861DelegatedAuth(BaseAuth):
         if not self._is_access_token_the_admin_token(access_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
@@ -528,10 +525,11 @@ class MSC3861DelegatedAuth(BaseAuth):
         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
@@ -579,11 +577,12 @@ class MSC3861DelegatedAuth(BaseAuth):
             # 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(
@@ -591,7 +590,7 @@ class MSC3861DelegatedAuth(BaseAuth):
                     "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
@@ -617,5 +616,4 @@ class MSC3861DelegatedAuth(BaseAuth):
             user_id=user_id,
             device_id=device_id,
             scope=scope,
-            is_guest=(has_guest_scope and not has_user_scope),
         )

synapse/rest/client/auth_metadata.py

@@ -76,11 +76,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__()

tests/handlers/test_oauth_delegation.py

@@ -25,11 +25,11 @@ import time
 from http import HTTPStatus
 from http.server import BaseHTTPRequestHandler, HTTPServer
 from io import BytesIO
-from typing import Any, Coroutine, Dict, Generator, Optional, TypeVar, Union
+from typing import Any, ClassVar, Coroutine, Dict, Generator, Optional, TypeVar, Union
 from unittest.mock import ANY, AsyncMock, Mock
 from urllib.parse import parse_qs
 
-from parameterized import parameterized_class
+from parameterized.parameterized import parameterized_class
 from signedjson.key import (
     encode_verify_key_base64,
     generate_signing_key,
@@ -46,7 +46,6 @@ from synapse.api.errors import (
     Codes,
     HttpResponseException,
     InvalidClientTokenError,
-    OAuthInsufficientScopeError,
     SynapseError,
 )
 from synapse.appservice import ApplicationService
@@ -78,11 +77,7 @@ JWKS_URI = ISSUER + ".well-known/jwks.json"
 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
@@ -112,7 +107,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,
         keys.register_servlets,
@@ -212,7 +224,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
         """The handler should return a 500 when no subject is present."""
 
         self._set_introspection_returnvalue(
-            {"active": True, "scope": " ".join([MATRIX_USER_SCOPE])}
+            {"active": True, "scope": " ".join([self.api_scope])},
         )
 
         request = Mock(args={})
@@ -235,7 +247,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([MATRIX_DEVICE_SCOPE]),
+                "scope": " ".join([self.device_scope]),
             }
         )
         request = Mock(args={})
@@ -282,7 +294,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE]),
+                "scope": " ".join([SYNAPSE_ADMIN_SCOPE, self.api_scope]),
                 "username": USERNAME,
             }
         )
@@ -312,9 +324,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "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,
             }
         )
@@ -344,7 +354,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([MATRIX_USER_SCOPE]),
+                "scope": " ".join([self.api_scope]),
                 "username": USERNAME,
             }
         )
@@ -374,7 +384,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]),
+                "scope": " ".join([self.api_scope, self.device_scope]),
                 "username": USERNAME,
             }
         )
@@ -404,7 +414,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([MATRIX_USER_SCOPE]),
+                "scope": " ".join([self.api_scope]),
                 "device_id": DEVICE,
                 "username": USERNAME,
             }
@@ -444,9 +454,9 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
                 "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,
@@ -457,68 +467,6 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
         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._set_introspection_returnvalue(
-            {
-                "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._rust_client.post.assert_called_once_with(
-            url=INTROSPECTION_ENDPOINT,
-            response_limit=ANY,
-            request_body=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._set_introspection_returnvalue(
-            {
-                "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._rust_client.post.assert_called_once_with(
-            url=INTROSPECTION_ENDPOINT,
-            response_limit=ANY,
-            request_body=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 +510,8 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
                 "sub": SUBJECT,
                 "scope": " ".join(
                     [
-                        MATRIX_USER_SCOPE,
-                        f"{MATRIX_DEVICE_SCOPE_PREFIX}AABBCC",
+                        self.api_scope,
+                        f"{self.device_scope_prefix}AABBCC",
                     ]
                 ),
                 "username": USERNAME,
@@ -611,7 +559,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
             {
                 "active": True,
                 "sub": SUBJECT,
-                "scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]),
+                "scope": " ".join([self.api_scope, self.device_scope]),
                 "username": USERNAME,
             }
         )
@@ -676,7 +624,7 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
                 return json.dumps(
                     {
                         "active": True,
-                        "scope": MATRIX_USER_SCOPE,
+                        "scope": self.api_scope,
                         "sub": SUBJECT,
                         "username": USERNAME,
                     },
@@ -842,8 +790,24 @@ class FakeMasServer(HTTPServer):
 T = TypeVar("T")
 
 
+@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 MasAuthDelegation(HomeserverTestCase):
     server: FakeMasServer
+    device_scope_prefix: ClassVar[str]
+    api_scope: ClassVar[str]
+
+    @property
+    def device_scope(self) -> str:
+        return self.device_scope_prefix + DEVICE
 
     def till_deferred_has_result(
         self,
@@ -914,12 +878,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join(
-                [
-                    MATRIX_USER_SCOPE,
-                    f"{MATRIX_DEVICE_SCOPE_PREFIX}{DEVICE}",
-                ]
-            ),
+            "scope": " ".join([self.api_scope, self.device_scope]),
             "username": USERNAME,
             "expires_in": 60,
         }
@@ -943,12 +902,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join(
-                [
-                    MATRIX_USER_SCOPE,
-                    f"{MATRIX_DEVICE_SCOPE_PREFIX}{DEVICE}",
-                ]
-            ),
+            "scope": " ".join([self.api_scope, self.device_scope]),
             "username": USERNAME,
         }
 
@@ -971,12 +925,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join(
-                [
-                    MATRIX_USER_SCOPE,
-                    f"{MATRIX_DEVICE_SCOPE_PREFIX}ABCDEF",
-                ]
-            ),
+            "scope": " ".join([self.api_scope, f"{self.device_scope_prefix}ABCDEF"]),
             "username": USERNAME,
             "expires_in": 60,
         }
@@ -993,7 +942,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join([MATRIX_USER_SCOPE]),
+            "scope": " ".join([self.api_scope]),
             "username": "inexistent_user",
             "expires_in": 60,
         }
@@ -1039,7 +988,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": MATRIX_USER_SCOPE,
+            "scope": self.api_scope,
             "username": USERNAME,
             "expires_in": 60,
             "device_id": DEVICE,
@@ -1057,7 +1006,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join([SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE]),
+            "scope": " ".join([SYNAPSE_ADMIN_SCOPE, self.api_scope]),
             "username": USERNAME,
             "expires_in": 60,
         }
@@ -1079,12 +1028,7 @@ class MasAuthDelegation(HomeserverTestCase):
         self.server.introspection_response = {
             "active": True,
             "sub": SUBJECT,
-            "scope": " ".join(
-                [
-                    MATRIX_USER_SCOPE,
-                    f"{MATRIX_DEVICE_SCOPE_PREFIX}{DEVICE}",
-                ]
-            ),
+            "scope": " ".join([self.api_scope, self.device_scope]),
             "username": USERNAME,
             "expires_in": 60,
         }

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 @@ class AuthIssuerTestCase(HomeserverTestCase):
         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 @@ class AuthMetadataTestCase(HomeserverTestCase):
         )
         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(