diff --git a/CHANGELOG.md b/CHANGELOG.md index a2ad9959..605d8195 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/fractal_client/cmd/__init__.py b/fractal_client/cmd/__init__.py index e6330ee2..35d2be2a 100644 --- a/fractal_client/cmd/__init__.py +++ b/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") diff --git a/fractal_client/cmd/_group.py b/fractal_client/cmd/_group.py index b44e4986..a05dfe53 100644 --- a/fractal_client/cmd/_group.py +++ b/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) diff --git a/fractal_client/cmd/_user.py b/fractal_client/cmd/_user.py index 064a125e..2a6f57bc 100644 --- a/fractal_client/cmd/_user.py +++ b/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( diff --git a/fractal_client/parser.py b/fractal_client/parser.py index 6d366616..55501fe3 100644 --- a/fractal_client/parser.py +++ b/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 +) diff --git a/poetry.lock b/poetry.lock index 85c88991..7520f898 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.4 and should not be changed by hand. [[package]] name = "alembic" @@ -696,7 +696,7 @@ typing = ["typing-extensions (>=4.12.2)"] [[package]] name = "fractal-server" -version = "2.9.0a4" +version = "2.9.0a10" description = "Server component of the Fractal analytics platform" optional = false python-versions = "^3.10" @@ -726,7 +726,7 @@ uvicorn-worker = "^0.2.0" type = "git" url = "https://github.com/fractal-analytics-platform/fractal-server.git" reference = "main" -resolved_reference = "87c2868d73e458273beca7895d51763366ed5847" +resolved_reference = "8bb57912219c0b87997249d730c7f0413dcf8478" [[package]] name = "ghp-import" diff --git a/tests/fixtures_testserver.py b/tests/fixtures_testserver.py index b37fcdfd..0b80db4b 100644 --- a/tests/fixtures_testserver.py +++ b/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}" diff --git a/tests/test_group.py b/tests/test_group.py index d5068455..4ba36d7a 100644 --- a/tests/test_group.py +++ b/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"}