Support stable endpoint and scopes from the MSC3861 family

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/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,19 +553,20 @@ 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(
                     500,
                     "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),
         )

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__()

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,
                     },