From 74be5cfdbc2208f0b34d9ab75f99994bd8ed217d Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 2 May 2025 12:13:26 +0200 Subject: [PATCH] Do not auto-provision missing users & devices when delegating auth to MAS (#18181) Since MAS 0.13.0, the provisionning of devices and users is done synchronously and reliably enough that we don't need to auto-provision on the Synapse side anymore. It's important to remove this behaviour if we want to start caching token introspection results. --- changelog.d/18181.misc | 1 + synapse/api/auth/msc3861_delegated.py | 39 +++++++------------------ tests/handlers/test_oauth_delegation.py | 10 +++++++ 3 files changed, 22 insertions(+), 28 deletions(-) create mode 100644 changelog.d/18181.misc diff --git a/changelog.d/18181.misc b/changelog.d/18181.misc new file mode 100644 index 0000000000..d9ba2f1dd1 --- /dev/null +++ b/changelog.d/18181.misc @@ -0,0 +1 @@ +Stop auto-provisionning missing users & devices when delegating auth to Matrix Authentication Service. Requires MAS 0.13.0 or later. diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py index 9ded3366e3..e500a06afe 100644 --- a/synapse/api/auth/msc3861_delegated.py +++ b/synapse/api/auth/msc3861_delegated.py @@ -39,7 +39,6 @@ from synapse.api.errors import ( HttpResponseException, InvalidClientTokenError, OAuthInsufficientScopeError, - StoreError, SynapseError, UnrecognizedRequestError, ) @@ -512,7 +511,7 @@ class MSC3861DelegatedAuth(BaseAuth): raise InvalidClientTokenError("No scope in token granting user rights") # Match via the sub claim - sub: Optional[str] = introspection_result.get_sub() + sub = introspection_result.get_sub() if sub is None: raise InvalidClientTokenError( "Invalid sub claim in the introspection result" @@ -525,29 +524,20 @@ class MSC3861DelegatedAuth(BaseAuth): # If we could not find a user via the external_id, it either does not exist, # or the external_id was never recorded - # TODO: claim mapping should be configurable - username: Optional[str] = introspection_result.get_username() - if username is None or not isinstance(username, str): + username = introspection_result.get_username() + if username is None: raise AuthError( 500, "Invalid username claim in the introspection result", ) user_id = UserID(username, self._hostname) - # First try to find a user from the username claim + # Try to find a user from the username claim user_info = await self.store.get_user_by_id(user_id=user_id.to_string()) if user_info is None: - # If the user does not exist, we should create it on the fly - # TODO: we could use SCIM to provision users ahead of time and listen - # for SCIM SET events if those ever become standard: - # https://datatracker.ietf.org/doc/html/draft-hunt-scim-notify-00 - - # TODO: claim mapping should be configurable - # If present, use the name claim as the displayname - name: Optional[str] = introspection_result.get_name() - - await self.store.register_user( - user_id=user_id.to_string(), create_profile_with_displayname=name + raise AuthError( + 500, + "User not found", ) # And record the sub as external_id @@ -587,17 +577,10 @@ class MSC3861DelegatedAuth(BaseAuth): "Invalid device ID in introspection result", ) - # Create the device on the fly if it does not exist - try: - await self.store.get_device( - user_id=user_id.to_string(), device_id=device_id - ) - except StoreError: - await self.store.store_device( - user_id=user_id.to_string(), - device_id=device_id, - initial_device_display_name="OIDC-native client", - ) + # Make sure the device exists + await self.store.get_device( + user_id=user_id.to_string(), device_id=device_id + ) # TODO: there is a few things missing in the requester here, which still need # to be figured out, like: diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py index 034a1594d9..934bfee0bc 100644 --- a/tests/handlers/test_oauth_delegation.py +++ b/tests/handlers/test_oauth_delegation.py @@ -147,6 +147,16 @@ class MSC3861OAuthDelegation(HomeserverTestCase): return hs + def prepare( + self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer + ) -> None: + # Provision the user and the device we use in the tests. + store = homeserver.get_datastores().main + self.get_success(store.register_user(USER_ID)) + self.get_success( + store.store_device(USER_ID, DEVICE, initial_device_display_name=None) + ) + def _assertParams(self) -> None: """Assert that the request parameters are correct.""" params = parse_qs(self.http_client.request.call_args[1]["data"].decode("utf-8"))