Enforce that an admin token also has the basic Matrix API scope

Author: sandhose

synapse/api/auth/msc3861_delegated.py

@@ -248,13 +248,10 @@ async def get_user_by_access_token(
         scope: List[str] = scope_to_list(introspection_result.get("scope", ""))
 
         # Determine type of user based on presence of particular scopes
-        has_admin_scope = SCOPE_SYNAPSE_ADMIN in scope
         has_user_scope = SCOPE_MATRIX_API in scope
         has_guest_scope = SCOPE_MATRIX_GUEST in scope
-        is_user = has_user_scope or has_admin_scope
-        is_guest = has_guest_scope and not is_user
 
-        if not is_user and not is_guest:
+        if not has_user_scope and not has_guest_scope:
             raise InvalidClientTokenError("No scope in token granting user rights")
 
         # Match via the sub claim
@@ -351,5 +348,5 @@ async def get_user_by_access_token(
             user_id=user_id,
             device_id=device_id,
             scope=scope,
-            is_guest=is_guest,
+            is_guest=(has_guest_scope and not has_user_scope),
         )

tests/handlers/test_oauth_delegation.py

@@ -224,6 +224,30 @@ def test_active_no_user_scope(self) -> None:
         )
         self._assertParams()
 
+    def test_active_admin_not_user(self) -> None:
+        """The handler should raise when the scope has admin right but not user."""
+
+        self.http_client.request = simple_async_mock(
+            return_value=FakeResponse.json(
+                code=200,
+                payload={
+                    "active": True,
+                    "sub": SUBJECT,
+                    "scope": " ".join([SYNAPSE_ADMIN_SCOPE]),
+                    "username": USERNAME,
+                },
+            )
+        )
+        request = Mock(args={})
+        request.args[b"access_token"] = [b"mockAccessToken"]
+        request.requestHeaders.getRawHeaders = mock_getRawHeaders()
+        self.get_failure(self.auth.get_user_by_req(request), InvalidClientTokenError)
+        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()
+
     def test_active_admin(self) -> None:
         """The handler should return a requester with admin rights."""
 
@@ -233,7 +257,7 @@ def test_active_admin(self) -> None:
                 payload={
                     "active": True,
                     "sub": SUBJECT,
-                    "scope": " ".join([SYNAPSE_ADMIN_SCOPE]),
+                    "scope": " ".join([SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE]),
                     "username": USERNAME,
                 },
             )