Allow reactivate a user without password

changelog.d/4.bugfix

@@ -0,0 +1 @@
+Allow reactivate user without password with Admin API in some edge cases.

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 `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
@@ -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:

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()
                     )

tests/rest/admin/test_user.py

@@ -2741,29 +2741,44 @@ 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.
         """
 
         # 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,