diff --git a/changelog.d/20241.bugfix b/changelog.d/20241.bugfix new file mode 100644 index 0000000000..3478b28dd3 --- /dev/null +++ b/changelog.d/20241.bugfix @@ -0,0 +1 @@ +Reject user creation via the Admin API when Synapse is delegating authentication to MAS. diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 43dab16598..ecf7fc45f4 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -237,6 +237,7 @@ class UserRestServletV2Get(RestServlet): self.pusher_pool = hs.get_pusherpool() self._msc3866_enabled = hs.config.experimental.msc3866.enabled self._all_user_types = hs.config.user_types.all_user_types + self._auth_delegation_enabled = hs.config.mas.enabled async def on_GET( self, request: SynapseRequest, user_id: str @@ -479,6 +480,15 @@ class UserRestServletV2(UserRestServletV2Get): return HTTPStatus.OK, user else: # create user + if self._auth_delegation_enabled: + raise SynapseError( + HTTPStatus.FORBIDDEN, + "User creation via the Admin API is not available when " + "Synapse is delegating authentication to MAS. Create the " + "user via MAS instead.", + errcode=Codes.FORBIDDEN, + ) + displayname = body.get("displayname", None) password_hash = None diff --git a/tests/handlers/test_oauth_delegation.py b/tests/handlers/test_oauth_delegation.py index 0fc1e17d6b..d37a58949e 100644 --- a/tests/handlers/test_oauth_delegation.py +++ b/tests/handlers/test_oauth_delegation.py @@ -662,3 +662,43 @@ class DisabledEndpointsTestCase(HomeserverTestCase): self.expect_unrecognized("GET", "/_synapse/admin/v1/users/foo/admin") self.expect_unrecognized("PUT", "/_synapse/admin/v1/users/foo/admin") self.expect_unrecognized("POST", "/_synapse/admin/v1/account_validity/validity") + + def _mock_admin_requester(self) -> None: + self.hs.get_auth().get_user_by_req = AsyncMock( # type: ignore[method-assign] + return_value=create_requester(user_id=USER_ID, device_id=DEVICE) + ) + self.hs.get_auth().is_server_admin = AsyncMock(return_value=True) # type: ignore[method-assign] + + def test_admin_api_create_user_rejected(self) -> None: + """MAS never learns about a user created through the admin API, so the + account would be unusable.""" + self._mock_admin_requester() + + channel = self.make_request( + "PUT", + "/_synapse/admin/v2/users/@newuser:test", + {"password": "hunter2"}, + access_token="token", + ) + + self.assertEqual(channel.code, 403, channel.json_body) + self.assertEqual( + channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body + ) + + def test_admin_api_modify_existing_user_allowed(self) -> None: + """Only creation is rejected: modifying a user MAS already knows about + has to keep working, since MAS has no equivalent endpoint.""" + self._mock_admin_requester() + self.get_success( + self.hs.get_datastores().main.register_user(f"@existing:{SERVER_NAME}") + ) + + channel = self.make_request( + "PUT", + "/_synapse/admin/v2/users/@existing:test", + {"displayname": "Existing User"}, + access_token="token", + ) + + self.assertEqual(channel.code, 200, channel.json_body)