From 05372dca57989ee7c96f427a50c365d638d4c25e Mon Sep 17 00:00:00 2001 From: Koen van der Veen Date: Wed, 9 Oct 2024 12:56:05 +0200 Subject: [PATCH 1/2] add tests for not being able to update root users --- ...tart-and-configure-server-and-admins.ipynb | 50 +++++++++++++++---- .../src/syft/service/user/user_service.py | 13 +++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/notebooks/scenarios/bigquery/000-start-and-configure-server-and-admins.ipynb b/notebooks/scenarios/bigquery/000-start-and-configure-server-and-admins.ipynb index 3aabe924a4c..61211d1506e 100644 --- a/notebooks/scenarios/bigquery/000-start-and-configure-server-and-admins.ipynb +++ b/notebooks/scenarios/bigquery/000-start-and-configure-server-and-admins.ipynb @@ -25,6 +25,7 @@ "\n", "# syft absolute\n", "import syft as sy\n", + "from syft.server.credentials import SyftVerifyKey\n", "from syft.util.test_helpers.checkpoint import create_checkpoint\n", "from syft.util.test_helpers.email_helpers import get_email_server" ] @@ -83,9 +84,7 @@ "metadata": {}, "outputs": [], "source": [ - "root_client = sy.login(\n", - " url=\"http://localhost:8080\", email=ROOT_EMAIL, password=ROOT_PASSWORD\n", - ")" + "root_client = server.login(email=ROOT_EMAIL, password=ROOT_PASSWORD)" ] }, { @@ -97,6 +96,44 @@ "root_client.users" ] }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Verify we cannot update the root role or verify key" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "root_admin_id = root_client.users.search(email=ROOT_EMAIL)[0].id" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "with sy.raises(sy.SyftException):\n", + " root_client.users.update(uid=root_admin_id, role=\"guest\")" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "with sy.raises(sy.SyftException):\n", + " root_client.users.update(\n", + " uid=root_admin_id, verify_key=SyftVerifyKey.from_string(\"0\" * 64)\n", + " )" + ] + }, { "cell_type": "markdown", "metadata": {}, @@ -234,11 +271,6 @@ } ], "metadata": { - "kernelspec": { - "display_name": "Python 3 (ipykernel)", - "language": "python", - "name": "python3" - }, "language_info": { "codemirror_mode": { "name": "ipython", @@ -249,7 +281,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.11.10" + "version": "3.12.5" } }, "nbformat": 4, diff --git a/packages/syft/src/syft/service/user/user_service.py b/packages/syft/src/syft/service/user/user_service.py index a0d27a00786..c5aac94b853 100644 --- a/packages/syft/src/syft/service/user/user_service.py +++ b/packages/syft/src/syft/service/user/user_service.py @@ -461,6 +461,19 @@ def update( public_message=f"You are not allowed to modify '{field_name}'." ) + # important to prevent root admins from shooting themselves in the foot + if ( + user_update.role is not Empty + and user.verify_key == context.server.verify_key + ): + raise SyftException(public_message="Cannot update root role") + + if ( + user_update.verify_key is not Empty + and user.verify_key == context.server.verify_key + ): + raise SyftException(public_message="Cannot update root verify key") + if user_update.name is not Empty and user_update.name.strip() == "": # type: ignore[comparison-overlap] raise SyftException(public_message="Name can't be an empty string.") From eadb45e85e43a62291ef513ec335f0b0d2a36353 Mon Sep 17 00:00:00 2001 From: Koen van der Veen Date: Wed, 9 Oct 2024 13:10:31 +0200 Subject: [PATCH 2/2] fix lint --- packages/syft/src/syft/service/user/user_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/syft/src/syft/service/user/user_service.py b/packages/syft/src/syft/service/user/user_service.py index c5aac94b853..47c41d1ab84 100644 --- a/packages/syft/src/syft/service/user/user_service.py +++ b/packages/syft/src/syft/service/user/user_service.py @@ -463,7 +463,7 @@ def update( # important to prevent root admins from shooting themselves in the foot if ( - user_update.role is not Empty + user_update.role is not Empty # type: ignore and user.verify_key == context.server.verify_key ): raise SyftException(public_message="Cannot update root role")