From ceb3dd77db0d3ce992d40175c3f53f6b6ddfa168 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 26 May 2023 15:16:34 +0200 Subject: [PATCH] Enforce that an admin token also has the basic Matrix API scope --- synapse/api/auth/msc3861_delegated.py | 7 ++----- tests/handlers/test_oauth_delegation.py | 26 ++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py index e4b16c0b5c..31c1de0119 100644 --- a/synapse/api/auth/msc3861_delegated.py +++ b/synapse/api/auth/msc3861_delegated.py @@ -248,13 +248,10 @@ class MSC3861DelegatedAuth(BaseAuth): 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 @@ class MSC3861DelegatedAuth(BaseAuth): user_id=user_id, device_id=device_id, scope=scope, - is_guest=is_guest, + 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 0641535512..6309d7b36e 100644 --- a/tests/handlers/test_oauth_delegation.py +++ b/tests/handlers/test_oauth_delegation.py @@ -224,6 +224,30 @@ class MSC3861OAuthDelegation(HomeserverTestCase): ) 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 @@ class MSC3861OAuthDelegation(HomeserverTestCase): payload={ "active": True, "sub": SUBJECT, - "scope": " ".join([SYNAPSE_ADMIN_SCOPE]), + "scope": " ".join([SYNAPSE_ADMIN_SCOPE, MATRIX_USER_SCOPE]), "username": USERNAME, }, )