Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(launcher-api): remove orphan JobResults visibility permissions #2128

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

mabw-rte
Copy link
Contributor

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).

@mabw-rte mabw-rte force-pushed the bugfix/1970-jobs-user-permissions branch from 9e44b59 to 5a22347 Compare August 28, 2024 16:10
@mabw-rte mabw-rte self-assigned this Aug 28, 2024
@mabw-rte mabw-rte changed the title Bugfix/1970 jobs user permissions fix(launcher-api): remove orphan JobResults visibility permissions Aug 28, 2024
Copy link
Contributor

@MartinBelthle MartinBelthle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me just a minor change

antarest/launcher/service.py Show resolved Hide resolved
@MartinBelthle
Copy link
Contributor

You can also remove the import of timedelta that we don't use anymore I believe

@mabw-rte mabw-rte force-pushed the bugfix/1970-jobs-user-permissions branch from 6496d7b to a7c8994 Compare August 29, 2024 09:46
@MartinBelthle MartinBelthle self-requested a review August 29, 2024 14:16
Copy link
Contributor Author

@mabw-rte mabw-rte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sylvlecl, il faudrait priviligier de rebase en local ... Tu pourrais voir avec @skamril, on a deja eu la discussion

@mabw-rte mabw-rte merged commit 66f7b31 into dev Aug 29, 2024
7 of 8 checks passed
@mabw-rte mabw-rte deleted the bugfix/1970-jobs-user-permissions branch August 29, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants