Skip to content

Commit

Permalink
Merge pull request #580 from fractal-analytics-platform/579-placehold…
Browse files Browse the repository at this point in the history
…er-align-with-fractal-server-re-verified-users

Align with fractal server (re: verified users)
  • Loading branch information
tcompa authored Dec 21, 2023
2 parents 41b08a1 + b4f4ae2 commit 21697f2
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 31 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
**Note**: Numbers like (\#123) point to closed Pull Requests on the fractal repository.

# Unreleased

* Always make new users verified, within `user register` command (\#580).
* Expose verification-related features in `user edit` command (\#580).
* Testing:
* Adapt `job_factory` to new strict response-validation models in `fractal-server` (\#580).

# 1.4.0

* Align with [`fractal-server` 1.4.0](https://fractal-analytics-platform.github.io/fractal-server/changelog/#140) (\#573).
Expand Down
2 changes: 2 additions & 0 deletions fractal_client/cmd/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ async def user(
"new_username",
"make_superuser",
"remove_superuser",
"make_verified",
"remove_verified",
]
function_kwargs = get_kwargs(parameters, kwargs)
iface = await user_edit(client, **function_kwargs)
Expand Down
12 changes: 10 additions & 2 deletions fractal_client/cmd/_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ async def user_register(
cache_dir: Optional[str] = None,
username: Optional[str] = None,
superuser: bool = False,
verified: bool = True, # TODO: this is not currently exposed in the CLI
batch: bool = False,
) -> Union[RichJsonInterface, PrintInterface]:

Expand Down Expand Up @@ -46,11 +47,12 @@ async def user_register(
)
data = check_response(res, expected_status_code=201)

if superuser:
if superuser or verified:
patch_payload = dict(is_superuser=superuser, is_verified=verified)
user_id = data["id"]
res = await client.patch(
f"{settings.FRACTAL_SERVER}/auth/users/{user_id}/",
json={"is_superuser": True},
json=patch_payload,
)
data = check_response(res, expected_status_code=200)

Expand Down Expand Up @@ -83,6 +85,8 @@ async def user_edit(
new_username: Optional[str] = None,
make_superuser: bool = False,
remove_superuser: bool = False,
make_verified: bool = False,
remove_verified: bool = False,
) -> Union[RichJsonInterface, PrintInterface]:

user_update = dict()
Expand All @@ -94,6 +98,10 @@ async def user_edit(
user_update["is_superuser"] = True
if remove_superuser:
user_update["is_superuser"] = False
if make_verified:
user_update["is_verified"] = True
if remove_verified:
user_update["is_verified"] = False
if new_cache_dir is not None:
user_update["cache_dir"] = new_cache_dir
if new_slurm_user is not None:
Expand Down
13 changes: 13 additions & 0 deletions fractal_client/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -908,3 +908,16 @@
action="store_true",
required=False,
)
user_edit_parser_verified = user_edit_parser.add_mutually_exclusive_group()
user_edit_parser_verified.add_argument(
"--make-verified",
help="Make user verified.",
action="store_true",
required=False,
)
user_edit_parser_verified.add_argument(
"--remove-verified",
help="Make user unverified.",
action="store_true",
required=False,
)
46 changes: 26 additions & 20 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 22 additions & 8 deletions tests/fixtures_testserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,21 @@ async def _job_factory(**job_args_override):
first_task_index=9999,
last_task_index=9999,
workflow_dump={},
input_dataset_dump={},
output_dataset_dump={},
input_dataset_dump=dict(
id=1,
name="ds-in",
read_only=False,
project_id=1,
resource_list=[dict(path="/tmp", id=1, dataset_id=1)],
),
output_dataset_dump=dict(
id=2,
name="ds-out",
read_only=False,
project_id=1,
resource_list=[dict(path="/tmp", id=1, dataset_id=2)],
),
project_dump=dict(id=1, name="proj", read_only=True),
start_timestamp=datetime.now(tz=timezone.utc),
user_email="[email protected]",
)
Expand Down Expand Up @@ -216,20 +229,21 @@ async def __register_user(
f"http://localhost:{PORT}/auth/register/",
json=new_user,
)
if res.status_code != 201:
import logging

logging.error(res.status_code)
logging.error(res.json())
assert res.status_code == 201
user_id = res.json()["id"]
# Make user verified via API call
res = await client_superuser.patch(
f"http://localhost:{PORT}/auth/users/{user_id}/",
json=dict(is_verified=True),
)
assert res.status_code == 200
return res.json()

return __register_user


@pytest.fixture
async def register_user(user_factory, db):

from fractal_server.app.models import UserOAuth

created_user = await user_factory(
Expand Down
23 changes: 22 additions & 1 deletion tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ async def test_register_as_superuser(invoke_as_superuser, is_superuser: bool):
assert not res.data["is_superuser"]
assert res.data["email"] == EMAIL_USER

# Test that new user is verified (note: for the moment we don't expose the
# possibility of registering a non-verified user)
assert res.data["is_verified"]


async def test_register_as_superuser_with_batch(invoke_as_superuser):
# Register a user with the --batch flag
Expand Down Expand Up @@ -104,7 +108,10 @@ async def test_edit_as_user(


@pytest.mark.parametrize("new_is_superuser", [True, False])
async def test_edit_as_superuser(invoke_as_superuser, new_is_superuser):
@pytest.mark.parametrize("new_is_non_verified", [True, False])
async def test_edit_as_superuser(
invoke_as_superuser, new_is_superuser, new_is_non_verified
):
# Register a new user
res = await invoke_as_superuser(f"user register {EMAIL_USER} {PWD_USER}")
assert res.retcode == 0
Expand All @@ -124,6 +131,8 @@ async def test_edit_as_superuser(invoke_as_superuser, new_is_superuser):
)
if new_is_superuser:
cmd = f"{cmd} --make-superuser"
if new_is_non_verified:
cmd = f"{cmd} --remove-verified"
debug(cmd)
res = await invoke_as_superuser(cmd)
debug(res.data)
Expand All @@ -133,6 +142,8 @@ async def test_edit_as_superuser(invoke_as_superuser, new_is_superuser):
assert res.data["slurm_user"] == NEW_SLURM_USER
assert res.data["username"] == NEW_USERNAME
assert res.data["is_superuser"] == new_is_superuser
if new_is_non_verified:
assert not res.data["is_verified"]

BAD_CACHE_DIR = "not_absolute"
with pytest.raises(SystemExit):
Expand All @@ -149,6 +160,16 @@ async def test_edit_as_superuser(invoke_as_superuser, new_is_superuser):
assert res.retcode == 0
assert not res.data["is_superuser"]

# If the user was made verified, check that we can go back to normal
# user
if new_is_non_verified:
cmd = f"user edit {user_id} --make-verified"
debug(cmd)
res = await invoke_as_superuser(cmd)
debug(res.data)
assert res.retcode == 0
assert res.data["is_verified"]


async def test_edit_arguments(invoke_as_superuser):
# Test that superuser flags are mutually exclusive
Expand Down

0 comments on commit 21697f2

Please sign in to comment.