Skip to content

Commit

Permalink
fix(launcher-api): remove orphan JobResults visibility permissions (#…
Browse files Browse the repository at this point in the history
…2128)

Context: 
In the job listing of `AntaREST`, we call two endpoints: 
- `GET`, `v1/launcher/jobs`
- `GET`, `v1/launcher/jobs/{job_id}/progress`
It turns out that when user does not have a permission for some job,
this job, though listed (using the first endpoint of `jobs`), spits out
an error `403` when it is called for the second endpoint `progress`.

Issue:
For the endpoint `jobs`, the logic beyond the permissions includes a
second option where when an orphan job is created recently enough would
become visible to everyone. This logic was not included in the
`progress` endpoint which leads to the `Exception` above.

Solution:
After consulting the application users, they think that this `orphan`
visibility logic is not that much relevant and should be removed (at
least temporarily).

---------

Co-authored-by: Sylvain Leclerc <sylvain.leclerc@rte-france.com>
  • Loading branch information
mabw-rte and sylvlecl authored Aug 29, 2024
1 parent adf9146 commit 66f7b31
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 15 deletions.
8 changes: 2 additions & 6 deletions antarest/launcher/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
import os
import shutil
from datetime import datetime, timedelta
from datetime import datetime
from http import HTTPStatus
from pathlib import Path
from typing import Dict, List, Optional, cast
Expand Down Expand Up @@ -59,7 +59,6 @@ def __init__(self, engine: str):
)


ORPHAN_JOBS_VISIBILITY_THRESHOLD = 10 # days
LAUNCHER_PARAM_NAME_SUFFIX = "output_suffix"
EXECUTION_INFO_FILE = "execution_info.ini"

Expand Down Expand Up @@ -305,7 +304,6 @@ def _filter_from_user_permission(self, job_results: List[JobResult], user: Optio
if not user:
return []

orphan_visibility_threshold = datetime.utcnow() - timedelta(days=ORPHAN_JOBS_VISIBILITY_THRESHOLD)
allowed_job_results = []

study_ids = [job_result.study_id for job_result in job_results]
Expand All @@ -330,9 +328,7 @@ def _filter_from_user_permission(self, job_results: List[JobResult], user: Optio
raising=False,
):
allowed_job_results.append(job_result)
elif (
user and (user.is_site_admin() or user.is_admin_token())
) or job_result.creation_date >= orphan_visibility_threshold:
elif user and (user.is_site_admin() or user.is_admin_token()):
allowed_job_results.append(job_result)
return allowed_job_results

Expand Down
41 changes: 41 additions & 0 deletions tests/integration/launcher_blueprint/test_launcher_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,44 @@ def test_get_launcher_time_limit(
"description": "Unknown solver configuration: 'unknown'",
"exception": "UnknownSolverConfig",
}

def test_jobs_permissions(
self,
client: TestClient,
user_access_token: str,
admin_access_token: str,
) -> None:
# create an admin study with no permissions
res = client.post(
"/v1/studies",
headers={"Authorization": f"Bearer {admin_access_token}"},
params={"name": "study_admin"},
)
res.raise_for_status()
# get the study_id
study_id = res.json()

# launch a job with the admin user
res = client.post(
f"/v1/launcher/run/{study_id}",
headers={"Authorization": f"Bearer {admin_access_token}"},
json={"launcher": "local"},
)
res.raise_for_status()
job_id = res.json()["job_id"]

# check that the user cannot see the job
res = client.get(
"/v1/launcher/jobs",
headers={"Authorization": f"Bearer {user_access_token}"},
)
res.raise_for_status()
assert job_id not in [job.get("id") for job in res.json()]

# check that the admin can see the job
res = client.get(
"/v1/launcher/jobs",
headers={"Authorization": f"Bearer {admin_access_token}"},
)
res.raise_for_status()
assert job_id in [job.get("id") for job in res.json()]
12 changes: 3 additions & 9 deletions tests/launcher/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,7 @@
LauncherParametersDTO,
LogType,
)
from antarest.launcher.service import (
EXECUTION_INFO_FILE,
LAUNCHER_PARAM_NAME_SUFFIX,
ORPHAN_JOBS_VISIBILITY_THRESHOLD,
JobNotFound,
LauncherService,
)
from antarest.launcher.service import EXECUTION_INFO_FILE, LAUNCHER_PARAM_NAME_SUFFIX, JobNotFound, LauncherService
from antarest.login.auth import Auth
from antarest.login.model import Identity
from antarest.study.model import OwnerInfo, PublicMode, Study, StudyMetadataDTO
Expand Down Expand Up @@ -257,7 +251,7 @@ def test_service_get_jobs_from_database(self, db_session) -> None:
job_status=JobStatus.SUCCESS,
msg="Hello, World!",
exit_code=0,
creation_date=now - timedelta(days=ORPHAN_JOBS_VISIBILITY_THRESHOLD + 1),
creation_date=now - timedelta(days=11),
owner=identity_instance,
)
]
Expand Down Expand Up @@ -308,7 +302,7 @@ def test_service_get_jobs_from_database(self, db_session) -> None:
)
),
)
== returned_faked_execution_results
== []
)

with pytest.raises(UserHasNotPermissionError):
Expand Down

0 comments on commit 66f7b31

Please sign in to comment.