diff --git a/CHANGELOG.md b/CHANGELOG.md index dd1c6025..78cd7e84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * Rename `source` into `label`, for `task collect-custom` command (\#712). * Do not refer to obsolete task attributes `source` or `owner` (\#712, \#717). * Add `--new-ssh-settings-json` option to `fractal user edit` (\#715). +* Add `--ssh-settings-json` option to `fractal user register` (\#722). * Add `--private` option to task-creating commands (\#717). * Drop `task delete` command (\#717). * Handle missing server in `fractal version` (\#724). diff --git a/fractal_client/cmd/__init__.py b/fractal_client/cmd/__init__.py index bed075ff..116fbeca 100644 --- a/fractal_client/cmd/__init__.py +++ b/fractal_client/cmd/__init__.py @@ -327,6 +327,7 @@ def user( "cache_dir", "slurm_user", "username", + "ssh_settings_json", "superuser", ] function_kwargs = get_kwargs(parameters, kwargs) diff --git a/fractal_client/cmd/_user.py b/fractal_client/cmd/_user.py index b53c3bb6..27cf2215 100644 --- a/fractal_client/cmd/_user.py +++ b/fractal_client/cmd/_user.py @@ -10,6 +10,35 @@ from ..response import check_response +def _read_ssh_settings_json(ssh_settings_json: str) -> dict: + """ + Read, validate and return as a dict the user's ssh-settings json file + """ + ssh_settings_json_path = Path(ssh_settings_json) + if not ssh_settings_json_path.exists(): + sys.exit(f"Invalid {ssh_settings_json=}. File does not exist.") + with ssh_settings_json_path.open("r") as f: + try: + ssh_settings = json.load(f) + except JSONDecodeError: + sys.exit(f"{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", + ) + settings = dict() + for key, value in ssh_settings.items(): + if key in __ALLOWED_KEYS__: + settings[key] = value + else: + sys.exit(f"Invalid {key=} in {ssh_settings_json=}.") + + return settings + + def user_register( client: AuthClient, *, @@ -18,6 +47,7 @@ def user_register( slurm_user: Optional[str] = None, cache_dir: Optional[str] = None, username: Optional[str] = None, + ssh_settings_json: Optional[str] = None, superuser: bool = False, verified: bool = True, # TODO: this is not currently exposed in the CLI batch: bool = False, @@ -34,6 +64,9 @@ def user_register( new_settings["slurm_user"] = slurm_user if cache_dir: new_settings["cache_dir"] = cache_dir + if ssh_settings_json is not None: + ssh_settings = _read_ssh_settings_json(ssh_settings_json) + new_settings.update(ssh_settings) res = client.post( f"{settings.FRACTAL_SERVER}/auth/register/", json=new_user @@ -134,26 +167,8 @@ def user_edit( 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=}.") + ssh_settings = _read_ssh_settings_json(new_ssh_settings_json) + settings_update.update(ssh_settings) res = client.patch( f"{settings.FRACTAL_SERVER}/auth/users/{user_id}/", json=user_update diff --git a/fractal_client/parser.py b/fractal_client/parser.py index dd153092..da034c52 100644 --- a/fractal_client/parser.py +++ b/fractal_client/parser.py @@ -800,6 +800,16 @@ help="Username associated to the user.", required=False, ) +user_register_parser.add_argument( + "--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_register_parser.add_argument( "--superuser", help="Give superuser privileges to the new user.", diff --git a/tests/test_user.py b/tests/test_user.py index b097085c..3f5769ff 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -40,6 +40,53 @@ def test_register_as_superuser( assert res.data["is_verified"] +def test_register_with_ssh_settings(invoke_as_superuser, new_name, tmp_path): + + EMAIL_USER = f"{new_name()}@example.org" + + with pytest.raises(SystemExit, match="File does not exist"): + invoke_as_superuser( + f"user register {EMAIL_USER} {PWD_USER} --ssh-settings-json xy.z" + ) + + invalid_json = tmp_path / "not-a-json.foo" + with invalid_json.open("w") as f: + f.write("hello world") + with pytest.raises(SystemExit, match="not a valid JSON"): + invoke_as_superuser( + f"user register {EMAIL_USER} {PWD_USER} " + f"--ssh-settings-json {invalid_json}" + ) + + invalid_key_json = tmp_path / "invalid-key.json" + with invalid_key_json.open("w") as f: + json.dump(dict(invalid="invalid"), f) + with pytest.raises(SystemExit, match="Invalid key"): + invoke_as_superuser( + f"user register {EMAIL_USER} {PWD_USER} " + f"--ssh-settings-json {invalid_key_json}" + ) + + valid_json = tmp_path / "ssh-config.json" + with valid_json.open("w") as f: + json.dump( + dict( + ssh_host="SSH_HOST", + ssh_private_key_path="/SSH_PRIVATE_KEY_PATH", + ), + f, + ) + res = invoke_as_superuser( + f"user register {EMAIL_USER} {PWD_USER} " + f"--ssh-settings-json {valid_json}" + ) + assert res.retcode == 0 + assert res.data["settings"]["ssh_host"] == "SSH_HOST" + assert res.data["settings"]["ssh_private_key_path"] == ( + "/SSH_PRIVATE_KEY_PATH" + ) + + def test_register_as_superuser_with_batch(invoke_as_superuser, new_name): EMAIL_USER = f"{new_name()}@example.org" # Register a user with the --batch flag