From af923d6b8f2db445cf431382144d428de8081534 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 10 Jun 2024 12:29:46 +0200 Subject: [PATCH 1/5] Fix anonymous user job retrieval logic For anonymous users, if a history_id is provided, do not override with the current session history and instead rely on the history accessibility --- lib/galaxy/managers/jobs.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index 2b7b3d7a94e3..cb6e88eadd75 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -127,12 +127,15 @@ def index_query( search = payload.search order_by = payload.order_by - if trans.user is None: - # If the user is anonymous we can only return jobs for the current session history - if trans.galaxy_session and trans.galaxy_session.current_history_id: - history_id = trans.galaxy_session.current_history_id - else: - return None + if ( + trans.user is None + and history_id is None + and trans.galaxy_session + and trans.galaxy_session.current_history_id + ): + # If the user is anonymous and no specific history_id was provided + # we can only return jobs from the history in the current session + history_id = trans.galaxy_session.current_history_id def build_and_apply_filters(stmt, objects, filter_func): if objects is not None: From a6b45e561cc2c42368922deac5ddaa5d4e144fa3 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 10 Jun 2024 13:31:36 +0200 Subject: [PATCH 2/5] Filter jobs by session instead of using the session current history for anonymous Co-authored-by: mvdbeek --- lib/galaxy/managers/jobs.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index cb6e88eadd75..14640e240ecf 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -127,16 +127,6 @@ def index_query( search = payload.search order_by = payload.order_by - if ( - trans.user is None - and history_id is None - and trans.galaxy_session - and trans.galaxy_session.current_history_id - ): - # If the user is anonymous and no specific history_id was provided - # we can only return jobs from the history in the current session - history_id = trans.galaxy_session.current_history_id - def build_and_apply_filters(stmt, objects, filter_func): if objects is not None: if isinstance(objects, (str, date, datetime)): @@ -223,9 +213,14 @@ def add_search_criteria(stmt): if user_details: stmt = stmt.outerjoin(Job.user) else: - if history_id is None and invocation_id is None and implicit_collection_jobs_id is None and trans.user: - stmt = stmt.where(Job.user_id == trans.user.id) - # caller better check security + if history_id is None and invocation_id is None and implicit_collection_jobs_id is None: + # If we're not filtering on history, invocation or collection we filter the jobs owned by the current user + if trans.user: + stmt = stmt.where(Job.user_id == trans.user.id) + elif trans.galaxy_session: + stmt = stmt.where(Job.session_id == trans.galaxy_session.id) + else: + return None stmt = build_and_apply_filters(stmt, payload.states, lambda s: model.Job.state == s) stmt = build_and_apply_filters(stmt, payload.tool_ids, lambda t: model.Job.tool_id == t) From 9df23a64d3941114235fdd1bf61f3ab9676c5d86 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 10 Jun 2024 16:21:44 +0200 Subject: [PATCH 3/5] Require session to list jobs instead of returning empty job list for anonymous Co-authored-by: Nicola Soranzo --- lib/galaxy/managers/jobs.py | 12 ++++-------- lib/galaxy/webapps/galaxy/services/jobs.py | 2 -- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index 14640e240ecf..4e245388a3e2 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -4,10 +4,7 @@ date, datetime, ) -from typing import ( - Dict, - Optional, -) +from typing import Dict import sqlalchemy from boltons.iterutils import remap @@ -34,6 +31,7 @@ ItemAccessibilityException, ObjectNotFound, RequestParameterInvalidException, + RequestParameterMissingException, ) from galaxy.job_metrics import ( RawMetric, @@ -109,9 +107,7 @@ def __init__(self, app: StructuredApp): self.app = app self.dataset_manager = DatasetManager(app) - def index_query( - self, trans: ProvidesUserContext, payload: JobIndexQueryPayload - ) -> Optional[sqlalchemy.engine.Result]: + def index_query(self, trans: ProvidesUserContext, payload: JobIndexQueryPayload) -> sqlalchemy.engine.Result: """The caller is responsible for security checks on the resulting job if history_id, invocation_id, or implicit_collection_jobs_id is set. Otherwise this will only return the user's jobs or all jobs if the requesting @@ -220,7 +216,7 @@ def add_search_criteria(stmt): elif trans.galaxy_session: stmt = stmt.where(Job.session_id == trans.galaxy_session.id) else: - return None + raise RequestParameterMissingException("A session is required to list jobs for anonymous users") stmt = build_and_apply_filters(stmt, payload.states, lambda s: model.Job.state == s) stmt = build_and_apply_filters(stmt, payload.tool_ids, lambda t: model.Job.tool_id == t) diff --git a/lib/galaxy/webapps/galaxy/services/jobs.py b/lib/galaxy/webapps/galaxy/services/jobs.py index 727497d12af8..9ecfb718b963 100644 --- a/lib/galaxy/webapps/galaxy/services/jobs.py +++ b/lib/galaxy/webapps/galaxy/services/jobs.py @@ -82,8 +82,6 @@ def index( ) jobs = self.job_manager.index_query(trans, payload) out: List[Dict[str, Any]] = [] - if jobs is None: - return out for job in jobs.yield_per(model.YIELD_PER_ROWS): # TODO: optimize if this crucial if check_security_of_jobs and not security_check(trans, job.history, check_accessible=True): From d0414c3d9501ab87c7ea7f7ba2b38d0f5866a7c1 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 10 Jun 2024 18:45:38 -0400 Subject: [PATCH 4/5] Fix check for anonymous --- lib/galaxy/files/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/files/__init__.py b/lib/galaxy/files/__init__.py index eb3542982883..b13b2e1db8c8 100644 --- a/lib/galaxy/files/__init__.py +++ b/lib/galaxy/files/__init__.py @@ -476,4 +476,4 @@ def file_sources(self): @property def anonymous(self) -> bool: - return bool(self._kwd.get("username")) + return not bool(self._kwd.get("username")) From 83517098001f87f18ccff6c7e4a4fbf2d8e35d2b Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 11 Jun 2024 00:01:27 -0400 Subject: [PATCH 5/5] Fix mypy after merge --- lib/galaxy/managers/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/jobs.py b/lib/galaxy/managers/jobs.py index df36547134d1..c21da2883615 100644 --- a/lib/galaxy/managers/jobs.py +++ b/lib/galaxy/managers/jobs.py @@ -108,7 +108,7 @@ def __init__(self, app: StructuredApp): self.app = app self.dataset_manager = DatasetManager(app) - def index_query(self, trans: ProvidesUserContext, payload: JobIndexQueryPayload) -> sqlalchemy.engine.Result: + def index_query(self, trans: ProvidesUserContext, payload: JobIndexQueryPayload) -> sqlalchemy.engine.ScalarResult: """The caller is responsible for security checks on the resulting job if history_id, invocation_id, or implicit_collection_jobs_id is set. Otherwise this will only return the user's jobs or all jobs if the requesting