Merge pull request #715 from fractal-analytics-platform/706-include-m…

…ore-user-settings-attributes-in-fractal-user-registeredit-commands-tbd

Add `--new-ssh-settings-json` to `fractal user edit`

CHANGELOG.md

@@ -6,6 +6,7 @@
 * Remove `--new-name` and `--new-version` options from `task edit` command (\#712).
 * Rename `source` into `label` `task collect-custom` command (\#712).
 * Do not rely on tasks' `source` or `owner` attributes (\#712).
+* Add `--new-ssh-settings-json` to `fractal user edit` (\#715).
 
 # 2.2.1
 

fractal_client/cmd/__init__.py

@@ -339,9 +339,10 @@ def user(
             "user_id",
             "new_email",
             "new_password",
-            "new_cache_dir",
-            "new_slurm_user",
             "new_username",
+            "new_slurm_user",
+            "new_cache_dir",
+            "new_ssh_settings_json",
             "make_superuser",
             "remove_superuser",
             "make_verified",

fractal_client/cmd/_user.py

@@ -1,3 +1,7 @@
+import json
+import sys
+from json.decoder import JSONDecodeError
+from pathlib import Path
 from typing import Optional
 
 from ..authclient import AuthClient
@@ -89,9 +93,10 @@ def user_edit(
     user_id: str,
     new_email: Optional[str] = None,
     new_password: Optional[str] = None,
+    new_username: Optional[str] = None,
     new_slurm_user: Optional[str] = None,
     new_cache_dir: Optional[str] = None,
-    new_username: Optional[str] = None,
+    new_ssh_settings_json: Optional[str] = None,
     make_superuser: bool = False,
     remove_superuser: bool = False,
     make_verified: bool = False,
@@ -128,6 +133,27 @@ def user_edit(
         settings_update["cache_dir"] = new_cache_dir
     if new_slurm_user is not None:
         settings_update["slurm_user"] = new_slurm_user
+    if new_ssh_settings_json is not None:
+        new_ssh_settings_json_path = Path(new_ssh_settings_json)
+        if not new_ssh_settings_json_path.exists():
+            sys.exit(f"Invalid {new_ssh_settings_json=}. File does not exist.")
+        with new_ssh_settings_json_path.open("r") as f:
+            try:
+                ssh_settings = json.load(f)
+            except JSONDecodeError:
+                sys.exit(f"{new_ssh_settings_json_path} is not a valid JSON.")
+        __ALLOWED_KEYS__ = (
+            "ssh_host",
+            "ssh_username",
+            "ssh_private_key_path",
+            "ssh_tasks_dir",
+            "ssh_jobs_dir",
+        )
+        for key, value in ssh_settings.items():
+            if key in __ALLOWED_KEYS__:
+                settings_update[key] = value
+            else:
+                sys.exit(f"Invalid {key=} in {new_ssh_settings_json=}.")
 
     res = client.patch(
         f"{settings.FRACTAL_SERVER}/auth/users/{user_id}/", json=user_update

fractal_client/parser.py

@@ -845,6 +845,9 @@
 user_edit_parser.add_argument(
     "--new-password", help="New password.", required=False
 )
+user_edit_parser.add_argument(
+    "--new-username", help="New user username.", required=False
+)
 user_edit_parser.add_argument(
     "--new-cache-dir",
     help=(
@@ -856,9 +859,18 @@
 user_edit_parser.add_argument(
     "--new-slurm-user", help="New SLURM username.", required=False
 )
+
 user_edit_parser.add_argument(
-    "--new-username", help="New user username.", required=False
+    "--new-ssh-settings-json",
+    help=(
+        "Path to JSON file with (a subset of) following settings: "
+        "ssh_host, ssh_username, ssh_private_key_path, "
+        "ssh_tasks_dir, ssh_jobs_dir."
+    ),
+    type=str,
+    required=False,
 )
+
 user_edit_parser_superuser = user_edit_parser.add_mutually_exclusive_group()
 user_edit_parser_superuser.add_argument(
     "--make-superuser",

tests/test_user.py

@@ -1,3 +1,4 @@
+import json
 from os import environ
 
 import pytest
@@ -184,6 +185,111 @@ def test_edit_as_superuser(
         assert res.data["is_verified"]
 
 
+def test_edit_user_settings(invoke_as_superuser, tmp_path):
+
+    EMPTY_USER_SETTINGS = {
+        "ssh_host": None,
+        "ssh_username": None,
+        "ssh_private_key_path": None,
+        "ssh_tasks_dir": None,
+        "ssh_jobs_dir": None,
+        "slurm_user": None,
+        "slurm_accounts": [],
+        "cache_dir": None,
+    }
+    SSH_HOST = "something.somewhere"
+    SSH_PRIVATE_KEY_PATH = "/tmp/something.key"
+    NEW_USER_SETTINGS = {
+        "ssh_host": SSH_HOST,
+        "ssh_username": None,
+        "ssh_private_key_path": SSH_PRIVATE_KEY_PATH,
+        "ssh_tasks_dir": None,
+        "ssh_jobs_dir": None,
+        "slurm_user": None,
+        "slurm_accounts": [],
+        "cache_dir": None,
+    }
+
+    # Register a new user
+    res = invoke_as_superuser(f"user register {EMAIL_USER} {PWD_USER}")
+    assert res.retcode == 0
+    user_id = res.data["id"]
+
+    # Check empty user settings
+    res = invoke_as_superuser(f"user show {user_id}")
+    assert res.retcode == 0
+    user_settings = {
+        key: value
+        for key, value in res.data["settings"].items()
+        if key != "id"
+    }
+    debug(user_settings)
+    assert user_settings == EMPTY_USER_SETTINGS
+
+    # Call fractal user edit
+    ssh_settings_file = tmp_path / "ssh.json"
+    with ssh_settings_file.open("w") as f:
+        json.dump(
+            {
+                "ssh_host": SSH_HOST,
+                "ssh_private_key_path": SSH_PRIVATE_KEY_PATH,
+            },
+            f,
+        )
+    cmd = (
+        f"user edit {user_id} "
+        f"--new-ssh-settings-json {ssh_settings_file.as_posix()}"
+    )
+    res = invoke_as_superuser(cmd)
+    assert res.retcode == 0
+    debug(res.data)
+
+    # Check edited user settings
+    res = invoke_as_superuser(f"user show {user_id}")
+    assert res.retcode == 0
+    user_settings = {
+        key: value
+        for key, value in res.data["settings"].items()
+        if key != "id"
+    }
+    debug(user_settings)
+    assert user_settings == NEW_USER_SETTINGS
+
+    # Failure due to missing file
+    ssh_settings_file = tmp_path / "invalid-ssh.json"
+    cmd = (
+        f"user edit {user_id} "
+        f"--new-ssh-settings-json {ssh_settings_file.as_posix()}"
+    )
+    with pytest.raises(SystemExit, match="File does not exist."):
+        res = invoke_as_superuser(cmd)
+
+    # Failure due to file not being a valid JSON
+    invalid_json = tmp_path / "invalid-json.foo"
+    with invalid_json.open("w") as f:
+        f.write("hello world")
+    cmd = (
+        f"user edit {user_id} "
+        f"--new-ssh-settings-json {invalid_json.as_posix()}"
+    )
+    with pytest.raises(SystemExit, match="not a valid JSON"):
+        res = invoke_as_superuser(cmd)
+
+    # Failure due to invalid keys
+    ssh_settings_file = tmp_path / "invalid-ssh.json"
+    with ssh_settings_file.open("w") as f:
+        json.dump(
+            dict(invalid="invalid"),
+            f,
+        )
+    cmd = (
+        f"user edit {user_id} "
+        f"--new-ssh-settings-json {ssh_settings_file.as_posix()}"
+    )
+    with pytest.raises(SystemExit, match="Invalid key"):
+        res = invoke_as_superuser(cmd)
+
+
 def test_edit_arguments(invoke_as_superuser):
     # Test that superuser flags are mutually exclusive
     with pytest.raises(SystemExit):