Update edits of UserGroups

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 * Update task-collection commands, to align with [fractal-server 2.9.0](https://github.com/fractal-analytics-platform/fractal-server/blob/main/CHANGELOG.md#290) (\#738).
 * Remove (internal) obsolete `do_not_separate_logs` argument (\#738).
+* Add `group {add|remove}-user` commands, and deprecate `--new-user-ids` argument from `group update` (\#748).
+* Update `user whoami --viewer-paths` to call the new dedicated [server endpoint](https://github.com/fractal-analytics-platform/fractal-server/pull/2096) (\#748).
 * Testing:
     * Align with fractal-server 2.9.0 removal of `DB_ENGINE` variable (\#743).
 

fractal_client/cmd/__init__.py

@@ -8,9 +8,11 @@
 from ._dataset import get_dataset
 from ._dataset import patch_dataset
 from ._dataset import post_dataset
+from ._group import group_add_user
 from ._group import group_get
 from ._group import group_list
 from ._group import group_new
+from ._group import group_remove_user
 from ._group import group_update
 from ._job import get_job
 from ._job import get_job_list
@@ -385,9 +387,17 @@ def group(
         function_kwargs = get_kwargs(parameters, kwargs)
         iface = group_new(client, batch=batch, **function_kwargs)
     elif subcmd == "update":
-        parameters = ["group_id", "new_user_ids", "new_viewer_paths"]
+        parameters = ["group_id", "new_viewer_paths"]
         function_kwargs = get_kwargs(parameters, kwargs)
         iface = group_update(client, **function_kwargs)
+    elif subcmd == "add-user":
+        parameters = ["group_id", "user_id"]
+        function_kwargs = get_kwargs(parameters, kwargs)
+        iface = group_add_user(client, **function_kwargs)
+    elif subcmd == "remove-user":
+        parameters = ["group_id", "user_id"]
+        function_kwargs = get_kwargs(parameters, kwargs)
+        iface = group_remove_user(client, **function_kwargs)
     else:
         raise NoCommandError(f"Command 'group {subcmd}' not found")
 

fractal_client/cmd/_group.py

@@ -50,19 +50,41 @@ def group_update(
     client: AuthClient,
     *,
     group_id: int,
-    new_user_ids: list[int] | None = None,
-    new_viewer_paths: list[str] | None = None,
+    new_viewer_paths: list[str],
 ):
 
-    request_body = dict()
-    if new_viewer_paths is not None:
-        request_body["viewer_paths"] = new_viewer_paths
-    if new_user_ids is not None:
-        request_body["new_user_ids"] = new_user_ids
-
     res = client.patch(
         f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/",
-        json=request_body,
+        json=dict(viewer_paths=new_viewer_paths),
+    )
+    data = check_response(res, expected_status_code=200)
+    return Interface(retcode=0, data=data)
+
+
+def group_add_user(
+    client: AuthClient,
+    *,
+    group_id: int,
+    user_id: int,
+):
+
+    res = client.post(
+        f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/add-user/{user_id}/"
+    )
+    data = check_response(res, expected_status_code=200)
+    return Interface(retcode=0, data=data)
+
+
+def group_remove_user(
+    client: AuthClient,
+    *,
+    group_id: int,
+    user_id: int,
+):
+
+    res = client.post(
+        f"{settings.FRACTAL_SERVER}/auth/group/{group_id}/remove-user/"
+        f"{user_id}/"
     )
     data = check_response(res, expected_status_code=200)
     return Interface(retcode=0, data=data)

fractal_client/cmd/_user.py

@@ -220,7 +220,8 @@ def user_whoami(
 
     if viewer_paths:
         res = client.get(
-            f"{settings.FRACTAL_SERVER}/auth/current-user/viewer-paths/"
+            f"{settings.FRACTAL_SERVER}/auth/current-user/"
+            "allowed-viewer-paths/"
         )
         returned_viewer_paths = check_response(res, expected_status_code=200)
         return Interface(

fractal_client/parser.py

@@ -967,27 +967,39 @@
 group_update_parser.add_argument(
     "group_id", help="ID of the group to update.", type=int
 )
-group_update_parser.add_argument(
-    "--new-user-ids",
-    help=(
-        "New list of users in the group "
-        "(e.g. `--new-user-ids 4 8 15 16 23 42`); "
-        "note that this replaces the existing one.",
-    ),
-    required=False,
-    type=int,
-    nargs="+",
-    default=None,
-)
 group_update_parser.add_argument(
     "--new-viewer-paths",
     help=(
         "New list of group `viewer_paths` (e.g "
-        "`--new-viewer-paths /something /else`);"
-        "note that this replaces the existing one.",
+        "`--new-viewer-paths /something /else`); "
+        "note that this replaces the existing one."
     ),
-    required=False,
+    required=True,
     type=str,
     nargs="+",
     default=None,
 )
+
+# group add-user
+group_add_user_parser = group_subparsers.add_parser(
+    "add-user", description="Add a single user to group.", allow_abbrev=False
+)
+group_add_user_parser.add_argument(
+    "group_id", help="ID of the group to which to add the user.", type=int
+)
+group_add_user_parser.add_argument(
+    "user_id", help="ID of the user to add.", type=int
+)
+
+# group remove-user
+group_remove_user_parser = group_subparsers.add_parser(
+    "remove-user",
+    description="Remove a single user from group.",
+    allow_abbrev=False,
+)
+group_remove_user_parser.add_argument(
+    "group_id", help="ID of the group to which to remove the user.", type=int
+)
+group_remove_user_parser.add_argument(
+    "user_id", help="ID of the user to remove.", type=int
+)

tests/fixtures_testserver.py

@@ -63,6 +63,7 @@ def testserver(tester, tmpdir_factory, request):
             "FRACTAL_RUNNER_WORKING_BASE_DIR="
             f"{FRACTAL_RUNNER_WORKING_BASE_DIR}\n"
             "FRACTAL_LOGGING_LEVEL=0\n"
+            "FRACTAL_VIEWER_AUTHORIZATION_SCHEME=viewer-paths\n"
         )
     _run_command(
         f"dropdb --username=postgres --host localhost --if-exists {DB_NAME}"

tests/test_group.py

@@ -16,7 +16,7 @@ def _assert_403(cmd):
     _assert_403(cmd="group list")
     _assert_403(cmd="group get 1")
     _assert_403(cmd="group new foo")
-    _assert_403(cmd="group update 1 --new-user-ids 1")
+    _assert_403(cmd="group update 1 --new-viewer-paths foo")
 
 
 def test_group_commands(
@@ -72,50 +72,40 @@ def test_group_commands(
     group2_viewer_paths = res.data["viewer_paths"]
     assert group2_viewer_paths == []
 
-    # Add users to groups (`group update`)
-
-    # empty update
-    res = invoke_as_superuser(f"group update {default_group_id}")
-    assert res.retcode == 0
-    assert res.data["id"] == default_group_id
+    # Add users to groups (`group add-user/remove-user`)
 
     with pytest.raises(SystemExit):
-        # missing 'group_id' and 'new_user_ids'
-        invoke_as_superuser("group update")
+        # missing both arguments
+        invoke_as_superuser("group add-user")
     with pytest.raises(SystemExit):
-        # missing 'group_id'
-        invoke_as_superuser(f"group update --new-user-ids {superuser_id}")
+        # missing one argument
+        invoke_as_superuser("group add-user 1")
+
     with pytest.raises(SystemExit):
         # user already in group
         invoke_as_superuser(
-            f"group update {default_group_id} --new-user-ids {superuser_id}"
+            f"group add-user {default_group_id} {superuser_id}"
         )
     with pytest.raises(SystemExit):
         # non existing user
-        invoke_as_superuser(
-            f"group update {default_group_id} --new-user-ids 9999"
-        )
+        invoke_as_superuser(f"group add-user {default_group_id} 9999")
 
     # add `user1` and `user2` to `group1`
-    res = invoke_as_superuser(
-        f"group update {group1_id} --new-user-ids {user1_id} {user2_id}"
-    )
+    invoke_as_superuser(f"group add-user {group1_id} {user1_id}")
+    res = invoke_as_superuser(f"group add-user {group1_id} {user2_id}")
     assert res.retcode == 0
     assert res.data["id"] == group1_id
     assert res.data["user_ids"] == [user1_id, user2_id]
     assert res.data["viewer_paths"] == group1_viewer_paths
 
     # add `user3` and `user2` to `group2`
-    res = invoke_as_superuser(
-        f"group update {group2_id} --new-user-ids {user3_id} {user2_id}"
-    )
+    invoke_as_superuser(f"group add-user {group2_id} {user3_id}")
+    res = invoke_as_superuser(f"group add-user {group2_id} {user2_id}")
     assert res.retcode == 0
     assert res.data["id"] == group2_id
     assert set(res.data["user_ids"]) == set([user3_id, user2_id])
     # add also `superuser` to `group2`
-    res = invoke_as_superuser(
-        f"group update {group2_id} --new-user-ids {superuser_id}"
-    )
+    res = invoke_as_superuser(f"group add-user {group2_id} {superuser_id}")
     assert set(res.data["user_ids"]) == set([user3_id, user2_id, superuser_id])
     assert res.data["viewer_paths"] == group2_viewer_paths
 
@@ -132,6 +122,14 @@ def test_group_commands(
         [user3_id, user2_id, superuser_id]
     )
 
+    # Remove users from group
+    res = invoke_as_superuser(f"group remove-user {group2_id} {user3_id}")
+    assert set(res.data["user_ids"]) == set([user2_id, superuser_id])
+    res = invoke_as_superuser(f"group remove-user {group2_id} {user2_id}")
+    assert set(res.data["user_ids"]) == set([superuser_id])
+    res = invoke_as_superuser(f"group remove-user {group2_id} {superuser_id}")
+    assert set(res.data["user_ids"]) == set()
+
     # Test `group get` command
 
     with pytest.raises(SystemExit):
@@ -171,10 +169,7 @@ def test_group_commands(
     assert res_post_patch.data == res_pre_patch.data
 
     # Test `whoami --viewer-paths`
-
-    invoke_as_superuser(
-        f"group update {group1_id} --new-user-ids {superuser_id}"
-    )
+    invoke_as_superuser(f"group add-user {group1_id} {superuser_id}")
     assert "viewer_paths" not in superuser
     res = invoke_as_superuser("user whoami --viewer-paths")
     assert set(res.data.get("viewer_paths")) == {"/a/b", "/c/d"}