Skip to content

Commit

Permalink
Merge branch '1774-introduce-user-settings' of github.com:fractal-ana…
Browse files Browse the repository at this point in the history
…lytics-platform/fractal-server into 1774-introduce-user-settings
  • Loading branch information
tcompa committed Sep 24, 2024
2 parents 6008439 + b54b821 commit b15d004
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 48 deletions.
7 changes: 6 additions & 1 deletion fractal_server/app/routes/api/v1/_aux_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from ....models.v1 import Workflow
from ....models.v1 import WorkflowTask
from ....schemas.v1 import JobStatusTypeV1
from ...aux.validate_user_settings import verify_user_has_settings
from fractal_server.app.models import UserOAuth


Expand Down Expand Up @@ -367,7 +368,11 @@ async def _get_task_check_owner(
),
)
else:
owner = user.username or user.slurm_user
if user.username:
owner = user.username
else:
verify_user_has_settings(user)
owner = user.settings.slurm_user
if owner != task.owner:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
Expand Down
21 changes: 12 additions & 9 deletions fractal_server/app/routes/api/v1/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from ....schemas.v1 import TaskCreateV1
from ....schemas.v1 import TaskReadV1
from ....schemas.v1 import TaskUpdateV1
from ...aux.validate_user_settings import verify_user_has_settings
from ._aux_functions import _get_task_check_owner
from ._aux_functions import _raise_if_v1_is_read_only
from fractal_server.app.models import UserOAuth
Expand Down Expand Up @@ -126,16 +127,18 @@ async def create_task(
# Set task.owner attribute
if user.username:
owner = user.username
elif user.slurm_user:
owner = user.slurm_user
else:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=(
"Cannot add a new task because current user does not "
"have `username` or `slurm_user` attributes."
),
)
verify_user_has_settings(user)
if user.settings.slurm_user:
owner = user.settings.slurm_user
else:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=(
"Cannot add a new task because current user does not "
"have `username` or `slurm_user` attributes."
),
)

# Prepend owner to task.source
task.source = f"{owner}:{task.source}"
Expand Down
7 changes: 6 additions & 1 deletion fractal_server/app/routes/api/v2/_aux_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from ....models.v2 import WorkflowTaskV2
from ....models.v2 import WorkflowV2
from ....schemas.v2 import JobStatusTypeV2
from ...aux.validate_user_settings import verify_user_has_settings
from fractal_server.app.models import UserOAuth
from fractal_server.images import Filters

Expand Down Expand Up @@ -362,7 +363,11 @@ async def _get_task_check_owner(
),
)
else:
owner = user.username or user.slurm_user
if user.username:
owner = user.username
else:
verify_user_has_settings(user)
owner = user.settings.slurm_user
if owner != task.owner:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
Expand Down
21 changes: 12 additions & 9 deletions fractal_server/app/routes/api/v2/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ....schemas.v2 import TaskCreateV2
from ....schemas.v2 import TaskReadV2
from ....schemas.v2 import TaskUpdateV2
from ...aux.validate_user_settings import verify_user_has_settings
from ._aux_functions import _get_task_check_owner
from fractal_server.app.models import UserOAuth
from fractal_server.app.routes.auth import current_active_user
Expand Down Expand Up @@ -150,16 +151,18 @@ async def create_task(
# Set task.owner attribute
if user.username:
owner = user.username
elif user.slurm_user:
owner = user.slurm_user
else:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=(
"Cannot add a new task because current user does not "
"have `username` or `slurm_user` attributes."
),
)
verify_user_has_settings(user)
if user.settings.slurm_user:
owner = user.settings.slurm_user
else:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=(
"Cannot add a new task because current user does not "
"have `username` or `slurm_user` attributes."
),
)

# Prepend owner to task.source
task.source = f"{owner}:{task.source}"
Expand Down
7 changes: 6 additions & 1 deletion fractal_server/app/routes/api/v2/task_collection_custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ....schemas.v2 import TaskCollectCustomV2
from ....schemas.v2 import TaskCreateV2
from ....schemas.v2 import TaskReadV2
from ...aux.validate_user_settings import verify_user_has_settings
from fractal_server.app.models import UserOAuth
from fractal_server.app.routes.auth import current_active_verified_user
from fractal_server.string_tools import validate_cmd
Expand Down Expand Up @@ -112,7 +113,11 @@ async def collect_task_custom(
package_root = Path(task_collect.package_root)

# Set task.owner attribute
owner = user.username or user.slurm_user
if user.username:
owner = user.username
else:
verify_user_has_settings(user)
owner = user.settings.slurm_user
if owner is None:
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,14 +268,14 @@ async def __aenter__(self):
user_attributes = dict(
email=self.email,
hashed_password="fake_hashed_password",
slurm_user="test01",
)
if self.user_kwargs is not None:
user_attributes.update(self.user_kwargs)
self.user = UserOAuth(**user_attributes)

# Create new user_settings object and associate it to user
user_settings_dict = self.user_settings_dict or {}
user_settings_dict = dict(slurm_user="test01")
user_settings_dict.update(self.user_settings_dict or {})
user_settings = UserSettings(**user_settings_dict)
self.user.settings = user_settings

Expand Down
35 changes: 23 additions & 12 deletions tests/v1/04_api/test_task_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ async def test_task_get_list(db, client, task_factory, MockCurrentUser):

async def test_post_task(client, MockCurrentUser):
async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:
TASK_OWNER = user.username or user.slurm_user
TASK_OWNER = user.username or user.settings.slurm_user
TASK_SOURCE = "some_source"

# Successful task creation
Expand Down Expand Up @@ -89,35 +89,45 @@ async def test_post_task(client, MockCurrentUser):
SLURM_USER = "some_slurm_user"
USERNAME = "some_username"
# Case 1: (username, slurm_user) = (None, None)
user_kwargs = dict(username=None, slurm_user=None, is_verified=True)
user_kwargs = dict(username=None, is_verified=True)
user_settings_dict = dict(slurm_user=None)
payload["source"] = "source_1"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 422
assert res.json()["detail"] == (
"Cannot add a new task because current user does not have "
"`username` or `slurm_user` attributes."
)
# Case 2: (username, slurm_user) = (not None, not None)
user_kwargs = dict(
username=USERNAME, slurm_user=SLURM_USER, is_verified=True
)
user_kwargs = dict(username=USERNAME, is_verified=True)
user_settings_dict = dict(slurm_user=SLURM_USER)
payload["source"] = "source_2"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == USERNAME
# Case 3: (username, slurm_user) = (None, not None)
user_kwargs = dict(username=None, slurm_user=SLURM_USER, is_verified=True)
user_kwargs = dict(username=None, is_verified=True)
user_settings_dict = dict(slurm_user=SLURM_USER)
payload["source"] = "source_3"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == SLURM_USER
# Case 4: (username, slurm_user) = (not None, None)
user_kwargs = dict(username=USERNAME, slurm_user=None, is_verified=True)
user_kwargs = dict(username=USERNAME, is_verified=True)
user_settings_dict = dict(slurm_user=None)
payload["source"] = "source_4"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == USERNAME
Expand Down Expand Up @@ -167,7 +177,8 @@ async def test_patch_task_auth(
assert res.json()["name"] == "new_name_1"

async with MockCurrentUser(
user_kwargs={"slurm_user": USER_2, "is_verified": True}
user_kwargs={"is_verified": True},
user_settings_dict={"slurm_user": USER_2},
):
update = TaskUpdateV1(name="new_name_2")

Expand Down
2 changes: 1 addition & 1 deletion tests/v2/03_api/test_api_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async def test_project_apply_missing_user_attributes(
assert res.status_code == 422
assert "User settings are not valid" in res.json()["detail"]
assert (
"validation errors for SlurmSudoUserSettings"
"validation error for SlurmSudoUserSettings"
in res.json()["detail"]
)

Expand Down
35 changes: 23 additions & 12 deletions tests/v2/03_api/test_api_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async def test_task_get_list(db, client, task_factory_v2, MockCurrentUser):
async def test_post_task(client, MockCurrentUser):
async with MockCurrentUser(user_kwargs=dict(is_verified=True)) as user:

TASK_OWNER = user.username or user.slurm_user
TASK_OWNER = user.username or user.settings.slurm_user
TASK_SOURCE = "some_source"

# Successful task creations
Expand Down Expand Up @@ -136,36 +136,46 @@ async def test_post_task(client, MockCurrentUser):
SLURM_USER = "some_slurm_user"
USERNAME = "some_username"
# Case 1: (username, slurm_user) = (None, None)
user_kwargs = dict(username=None, slurm_user=None, is_verified=True)
user_kwargs = dict(username=None, is_verified=True)
user_settings_dict = dict(slurm_user=None)
payload = dict(name="task", command_parallel="cmd")
payload["source"] = "source_x"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 422
assert res.json()["detail"] == (
"Cannot add a new task because current user does not have "
"`username` or `slurm_user` attributes."
)
# Case 2: (username, slurm_user) = (not None, not None)
user_kwargs = dict(
username=USERNAME, slurm_user=SLURM_USER, is_verified=True
)
user_kwargs = dict(username=USERNAME, is_verified=True)
user_settings_dict = dict(slurm_user=SLURM_USER)
payload["source"] = "source_y"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == USERNAME
# Case 3: (username, slurm_user) = (None, not None)
user_kwargs = dict(username=None, slurm_user=SLURM_USER, is_verified=True)
user_kwargs = dict(username=None, is_verified=True)
user_settings_dict = dict(slurm_user=SLURM_USER)
payload["source"] = "source_z"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == SLURM_USER
# Case 4: (username, slurm_user) = (not None, None)
user_kwargs = dict(username=USERNAME, slurm_user=None, is_verified=True)
user_kwargs = dict(username=USERNAME, is_verified=True)
user_settings_dict = dict(slurm_user=None)
payload["source"] = "source_xyz"
async with MockCurrentUser(user_kwargs=user_kwargs):
async with MockCurrentUser(
user_kwargs=user_kwargs, user_settings_dict=user_settings_dict
):
res = await client.post(f"{PREFIX}/", json=payload)
assert res.status_code == 201
assert res.json()["owner"] == USERNAME
Expand Down Expand Up @@ -260,7 +270,8 @@ async def test_patch_task_auth(
assert res.json()["name"] == "new_name_1"

async with MockCurrentUser(
user_kwargs={"slurm_user": USER_2, "is_verified": True}
user_kwargs={"is_verified": True},
user_settings_dict={"slurm_user": USER_2},
):
update = TaskUpdateV2(name="new_name_2")

Expand Down
19 changes: 19 additions & 0 deletions tests/v2/03_api/test_api_task_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,25 @@ async def test_task_collection_custom(
assert res.status_code == 422
assert "doesn't exist or is not a directory" in res.json()["detail"]

# Test owner=user.username
USERNAME = "username"
async with MockCurrentUser(
user_kwargs=dict(username=USERNAME, is_verified=True)
):

payload_name = TaskCollectCustomV2(
manifest=manifest,
python_interpreter=python_bin,
source="xyz",
package_name=package_name,
)
res = await client.post(
f"{PREFIX}/collect/custom/", json=payload_name.dict()
)
assert res.status_code == 201
for task in res.json():
assert task["owner"] == USERNAME


async def test_task_collection_custom_fail_with_ssh(
client,
Expand Down

0 comments on commit b15d004

Please sign in to comment.