From 618e500b4180840a957c550593ba03a5f2523206 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Mon, 25 Oct 2021 14:06:46 +0300 Subject: [PATCH 1/3] Enable changing user type via users admin API Users admin API can now also modify user type in addition to allowing it to be set on user creation. Signed-off-by: Jason Robinson --- changelog.d/11174.bugfix | 1 + docs/admin_api/user_admin_api.md | 9 +++- synapse/rest/admin/users.py | 3 ++ .../storage/databases/main/registration.py | 18 +++++++ tests/rest/admin/test_user.py | 51 +++++++++++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 changelog.d/11174.bugfix diff --git a/changelog.d/11174.bugfix b/changelog.d/11174.bugfix new file mode 100644 index 000000000000..8eecd9268149 --- /dev/null +++ b/changelog.d/11174.bugfix @@ -0,0 +1 @@ +Users admin API can now also modify user type in addition to allowing it to be set on user creation. diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 534f8400ba45..73a90e7fd561 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -50,7 +50,8 @@ It returns a JSON body like the following: "auth_provider": "", "external_id": "" } - ] + ], + "user_type": null } ``` @@ -97,7 +98,8 @@ with a body of: ], "avatar_url": "", "admin": false, - "deactivated": false + "deactivated": false, + "user_type": null } ``` @@ -135,6 +137,9 @@ Body parameters: unchanged on existing accounts and set to `false` for new accounts. A user cannot be erased by deactivating with this API. For details on deactivating users see [Deactivate Account](#deactivate-account). +- `user_type` - string or null, optional. If provided, the user type will be + adjusted. If `null` given, the user type will be cleared. Other + possible options are: `bot` and `support`. If the user already exists then optional parameters default to the current value. diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index c0bebc3cf0f5..d14fafbbc965 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -326,6 +326,9 @@ async def on_PUT( target_user.to_string() ) + if "user_type" in body: + await self.store.set_user_type(target_user, user_type) + user = await self.admin_handler.get_user(target_user) assert user is not None diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 37d47aa8230e..6c7d6ba50848 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -499,6 +499,24 @@ def set_shadow_banned_txn(txn): await self.db_pool.runInteraction("set_shadow_banned", set_shadow_banned_txn) + async def set_user_type(self, user: UserID, user_type: Optional[UserTypes]) -> None: + """Sets the user type. + + Args: + user: user ID of the user. + user_type: type of the user or None for a user without a type. + """ + + def set_user_type_txn(txn): + self.db_pool.simple_update_one_txn( + txn, "users", {"name": user.to_string()}, {"user_type": user_type} + ) + self._invalidate_cache_and_stream( + txn, self.get_user_by_id, (user.to_string(),) + ) + + await self.db_pool.runInteraction("set_user_type", set_user_type_txn) + def _query_for_auth(self, txn, token: str) -> Optional[TokenLookupResult]: sql = """ SELECT users.name as user_id, diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 839442ddba9e..2f61774500ce 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -2270,6 +2270,57 @@ def test_set_user_as_admin(self): self.assertEqual("@user:test", channel.json_body["name"]) self.assertTrue(channel.json_body["admin"]) + def test_set_user_type(self): + """ + Test changing user type. + """ + + # Set to support type + channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content={"user_type": UserTypes.SUPPORT}, + ) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertEqual("support", channel.json_body["user_type"]) + + # Get user + channel = self.make_request( + "GET", + self.url_other_user, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertEqual("support", channel.json_body["user_type"]) + + # Change back to a regular user + channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content={"user_type": None}, + ) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertIsNone(channel.json_body["user_type"]) + + # Get user + channel = self.make_request( + "GET", + self.url_other_user, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertIsNone(channel.json_body["user_type"]) + def test_accidental_deactivation_prevention(self): """ Ensure an account can't accidentally be deactivated by using a str value From f784a92644c3f30b07b1fe248f9a2efc892f6cb4 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 26 Oct 2021 11:32:37 +0300 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Brendan Abolivier --- docs/admin_api/user_admin_api.md | 2 +- tests/rest/admin/test_user.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/admin_api/user_admin_api.md b/docs/admin_api/user_admin_api.md index 73a90e7fd561..f03539c9f0e0 100644 --- a/docs/admin_api/user_admin_api.md +++ b/docs/admin_api/user_admin_api.md @@ -139,7 +139,7 @@ Body parameters: deactivating users see [Deactivate Account](#deactivate-account). - `user_type` - string or null, optional. If provided, the user type will be adjusted. If `null` given, the user type will be cleared. Other - possible options are: `bot` and `support`. + allowed options are: `bot` and `support`. If the user already exists then optional parameters default to the current value. diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 2f61774500ce..25e8d6cf278c 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -2285,7 +2285,7 @@ def test_set_user_type(self): self.assertEqual(200, channel.code, msg=channel.json_body) self.assertEqual("@user:test", channel.json_body["name"]) - self.assertEqual("support", channel.json_body["user_type"]) + self.assertEqual(UserTypes.SUPPORT, channel.json_body["user_type"]) # Get user channel = self.make_request( @@ -2296,7 +2296,7 @@ def test_set_user_type(self): self.assertEqual(200, channel.code, msg=channel.json_body) self.assertEqual("@user:test", channel.json_body["name"]) - self.assertEqual("support", channel.json_body["user_type"]) + self.assertEqual(UserTypes.SUPPORT, channel.json_body["user_type"]) # Change back to a regular user channel = self.make_request( From 774ff1aa36e1d77a7b879f327c73c6dcd0510249 Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Tue, 26 Oct 2021 11:34:47 +0300 Subject: [PATCH 3/3] changelog: bugfix -> feature Signed-off-by: Jason Robinson --- changelog.d/{11174.bugfix => 11174.feature} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{11174.bugfix => 11174.feature} (100%) diff --git a/changelog.d/11174.bugfix b/changelog.d/11174.feature similarity index 100% rename from changelog.d/11174.bugfix rename to changelog.d/11174.feature