From 2fc34cbdafe5d264a0bd0f846df0ebf8f5ef6cbe Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 8 Dec 2023 09:38:58 +0100 Subject: [PATCH 1/5] Allow reactivate a user without password --- synapse/rest/admin/users.py | 9 --------- tests/rest/admin/test_user.py | 31 +++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 77446970cb83..cb24ffdd1bae 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -406,15 +406,6 @@ async def on_PUT( target_user.to_string(), False, requester, by_admin=True ) elif not deactivate and user["deactivated"]: - if ( - "password" not in body - and self.auth_handler.can_change_password() - ): - raise SynapseError( - HTTPStatus.BAD_REQUEST, - "Must provide a password to re-activate an account.", - ) - await self.deactivate_account_handler.activate_account( target_user.to_string() ) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index cf71bbb46113..fe23e77fd509 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -2741,7 +2741,7 @@ def test_change_name_deactivate_user_user_directory(self) -> None: profile = self.get_success(self.store._get_user_in_directory(self.other_user)) self.assertIsNone(profile) - def test_reactivate_user(self) -> None: + def test_reactivate_user_with_password(self) -> None: """ Test reactivating another user. """ @@ -2749,21 +2749,36 @@ def test_reactivate_user(self) -> None: # Deactivate the user. self._deactivate_user("@user:test") - # Attempt to reactivate the user (without a password). + # Reactivate the user with password. channel = self.make_request( "PUT", self.url_other_user, access_token=self.admin_user_tok, - content={"deactivated": False}, + content={"deactivated": False, "password": "foo"}, ) - self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertFalse(channel.json_body["deactivated"]) + self._is_erased("@user:test", False) + + # This key was removed intentionally. Ensure it is not accidentally re-included. + self.assertNotIn("password_hash", channel.json_body) - # Reactivate the user. + def test_reactivate_user_without_password(self) -> None: + """ + Test reactivating another user without a password. + This can be using some local users and some user with SSO (password = `null`). + """ + + # Deactivate the user. + self._deactivate_user("@user:test") + + # Reactivate the user without a password. channel = self.make_request( "PUT", self.url_other_user, access_token=self.admin_user_tok, - content={"deactivated": False, "password": "foo"}, + content={"deactivated": False}, ) self.assertEqual(200, channel.code, msg=channel.json_body) self.assertEqual("@user:test", channel.json_body["name"]) @@ -2782,7 +2797,7 @@ def test_reactivate_user_localdb_disabled(self) -> None: # Deactivate the user. self._deactivate_user("@user:test") - # Reactivate the user with a password + # Reactivate the user with a password. channel = self.make_request( "PUT", self.url_other_user, @@ -2816,7 +2831,7 @@ def test_reactivate_user_password_disabled(self) -> None: # Deactivate the user. self._deactivate_user("@user:test") - # Reactivate the user with a password + # Reactivate the user with a password. channel = self.make_request( "PUT", self.url_other_user, From 42428c110991b4ee2499c5c4fb6b901b3abf2f04 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 8 Dec 2023 09:46:30 +0100 Subject: [PATCH 2/5] newsfile --- changelog.d/16739.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16739.bugfix diff --git a/changelog.d/16739.bugfix b/changelog.d/16739.bugfix new file mode 100644 index 000000000000..fcfcfb0fb165 --- /dev/null +++ b/changelog.d/16739.bugfix @@ -0,0 +1 @@ +Allow reactivate user without password with Admin API in some edge cases. \ No newline at end of file From 0fd2d62d3bd5a8ce4352d3955c06b742b4637d63 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 12 Dec 2023 11:39:36 +0100 Subject: [PATCH 3/5] update doc --- docs/admin_api/user_admin_api.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index e8e492d0951c..8279e7f45ee3 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -149,10 +149,11 @@ Body parameters: granting them access to the Admin API, among other things. - `deactivated` - **bool**, optional. If unspecified, deactivation state will be left unchanged. - Note: the `password` field must also be set if both of the following are true: - - `deactivated` is set to `false` and the user was previously deactivated (you are reactivating this user) - - Users are allowed to set their password on this homeserver (both `password_config.enabled` and - `password_config.localdb_enabled` config options are set to `true`). + Note: + - For the password field there is no strict check of the necessity for its presence. + It is possible to have active users without a password, e.g. when authenticating with OIDC is configured. + You must check yourself whether a password is required when reactivating a user or not. + - It is not possible to set a password if the config option `password_config.localdb_enabled` is set `true`. Users' passwords are wiped upon account deactivation, hence the need to set a new one here. Note: a user cannot be erased with this API. For more details on @@ -223,7 +224,7 @@ The following parameters should be set in the URL: **or** displaynames that contain this value. - `guests` - string representing a bool - Is optional and if `false` will **exclude** guest users. Defaults to `true` to include guest users. This parameter is not supported when MSC3861 is enabled. [See #15582](https://github.com/matrix-org/synapse/pull/15582) -- `admins` - Optional flag to filter admins. If `true`, only admins are queried. If `false`, admins are excluded from +- `admins` - Optional flag to filter admins. If `true`, only admins are queried. If `false`, admins are excluded from the query. When the flag is absent (the default), **both** admins and non-admins are included in the search results. - `deactivated` - string representing a bool - Is optional and if `true` will **include** deactivated users. Defaults to `false` to exclude deactivated users. @@ -272,7 +273,7 @@ The following fields are returned in the JSON response body: - `is_guest` - bool - Status if that user is a guest account. - `admin` - bool - Status if that user is a server administrator. - `user_type` - string - Type of the user. Normal users are type `None`. - This allows user type specific behaviour. There are also types `support` and `bot`. + This allows user type specific behaviour. There are also types `support` and `bot`. - `deactivated` - bool - Status if that user has been marked as deactivated. - `erased` - bool - Status if that user has been marked as erased. - `shadow_banned` - bool - Status if that user has been marked as shadow banned. @@ -887,7 +888,7 @@ The following fields are returned in the JSON response body: ### Create a device -Creates a new device for a specific `user_id` and `device_id`. Does nothing if the `device_id` +Creates a new device for a specific `user_id` and `device_id`. Does nothing if the `device_id` exists already. The API is: @@ -1254,11 +1255,11 @@ The following parameters should be set in the URL: ## Check username availability -Checks to see if a username is available, and valid, for the server. See [the client-server +Checks to see if a username is available, and valid, for the server. See [the client-server API](https://matrix.org/docs/spec/client_server/r0.6.0#get-matrix-client-r0-register-available) for more information. -This endpoint will work even if registration is disabled on the server, unlike +This endpoint will work even if registration is disabled on the server, unlike `/_matrix/client/r0/register/available`. The API is: From 5108cdac9bbc2eea2a9f83d2c8b19ca0e34ea456 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 12 Dec 2023 11:50:42 +0100 Subject: [PATCH 4/5] fix wrong flag value --- docs/admin_api/user_admin_api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 8279e7f45ee3..9dc600b87537 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -153,7 +153,7 @@ Body parameters: - For the password field there is no strict check of the necessity for its presence. It is possible to have active users without a password, e.g. when authenticating with OIDC is configured. You must check yourself whether a password is required when reactivating a user or not. - - It is not possible to set a password if the config option `password_config.localdb_enabled` is set `true`. + - It is not possible to set a password if the config option `password_config.localdb_enabled` is set `false`. Users' passwords are wiped upon account deactivation, hence the need to set a new one here. Note: a user cannot be erased with this API. For more details on From d6e9b715e6ee53a375380bed144aa5a00a75ee0f Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:12:04 +0100 Subject: [PATCH 5/5] Rename 16739.bugfix to 4.bugfix --- changelog.d/{16739.bugfix => 4.bugfix} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename changelog.d/{16739.bugfix => 4.bugfix} (86%) diff --git a/changelog.d/16739.bugfix b/changelog.d/4.bugfix similarity index 86% rename from changelog.d/16739.bugfix rename to changelog.d/4.bugfix index fcfcfb0fb165..c02bd8510d91 100644 --- a/changelog.d/16739.bugfix +++ b/changelog.d/4.bugfix @@ -1 +1 @@ -Allow reactivate user without password with Admin API in some edge cases. \ No newline at end of file +Allow reactivate user without password with Admin API in some edge cases.