diff options
| -rw-r--r-- | changelog.d/6990.bugfix | 1 | ||||
| -rw-r--r-- | synapse/rest/admin/users.py | 11 | ||||
| -rw-r--r-- | synapse/rest/client/v1/login.py | 1 | ||||
| -rw-r--r-- | tests/rest/admin/test_user.py | 59 |
4 files changed, 68 insertions, 4 deletions
diff --git a/changelog.d/6990.bugfix b/changelog.d/6990.bugfix new file mode 100644 index 0000000000..8c1c48f4d4 --- /dev/null +++ b/changelog.d/6990.bugfix @@ -0,0 +1 @@ +Prevent user from setting 'deactivated' to anything other than a bool on the v2 PUT /users Admin API. \ No newline at end of file diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 2107b5dc56..c5b461a236 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -228,13 +228,16 @@ class UserRestServletV2(RestServlet): ) if "deactivated" in body: - deactivate = bool(body["deactivated"]) + deactivate = body["deactivated"] + if not isinstance(deactivate, bool): + raise SynapseError( + 400, "'deactivated' parameter is not of type boolean" + ) + if deactivate and not user["deactivated"]: - result = await self.deactivate_account_handler.deactivate_account( + await self.deactivate_account_handler.deactivate_account( target_user.to_string(), False ) - if not result: - raise SynapseError(500, "Could not deactivate user") user = await self.admin_handler.get_user(target_user) return 200, user diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 1294e080dc..2c99536678 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -599,6 +599,7 @@ class SSOAuthHandler(object): redirect_url = self._add_login_token_to_redirect_url( client_redirect_url, login_token ) + # Load page request.redirect(redirect_url) finish_request(request) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 490ce8f55d..cbe4a6a51f 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -507,3 +507,62 @@ class UserRestTestCase(unittest.HomeserverTestCase): self.assertEqual(1, channel.json_body["admin"]) self.assertEqual(0, channel.json_body["is_guest"]) self.assertEqual(1, channel.json_body["deactivated"]) + + def test_accidental_deactivation_prevention(self): + """ + Ensure an account can't accidentally be deactivated by using a str value + for the deactivated body parameter + """ + self.hs.config.registration_shared_secret = None + + # Create user + body = json.dumps({"password": "abc123"}) + + request, channel = self.make_request( + "PUT", + self.url, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(201, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["name"]) + self.assertEqual("bob", channel.json_body["displayname"]) + + # Get user + request, channel = self.make_request( + "GET", self.url, access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["name"]) + self.assertEqual("bob", channel.json_body["displayname"]) + self.assertEqual(0, channel.json_body["deactivated"]) + + # Change password (and use a str for deactivate instead of a bool) + body = json.dumps({"password": "abc123", "deactivated": "false"}) # oops! + + request, channel = self.make_request( + "PUT", + self.url, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + + # Check user is not deactivated + request, channel = self.make_request( + "GET", self.url, access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["name"]) + self.assertEqual("bob", channel.json_body["displayname"]) + + # Ensure they're still alive + self.assertEqual(0, channel.json_body["deactivated"]) |