From eca0cfbcfa9c9ec2f7a09f78a5b9a5589b174798 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 15 Aug 2024 10:48:22 +0200 Subject: [PATCH] Allow access to invocation via shared or published history --- lib/galaxy/managers/base.py | 2 + lib/galaxy/managers/markdown_util.py | 4 +- lib/galaxy/managers/workflows.py | 39 +++++++++++++------ .../webapps/galaxy/services/invocations.py | 17 ++++++-- lib/galaxy_test/api/test_workflows.py | 10 +++++ 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/lib/galaxy/managers/base.py b/lib/galaxy/managers/base.py index 3d4c39e1cd4b..9866fb91ed15 100644 --- a/lib/galaxy/managers/base.py +++ b/lib/galaxy/managers/base.py @@ -110,6 +110,8 @@ def security_check(trans, item, check_ownership=False, check_accessible=False): raise exceptions.ItemOwnershipException( f"{item.__class__.__name__} is not owned by the current user", type="error" ) + # no need to check accessibility if we're the owner + return item # Verify accessible: # if it's part of a lib - can they access via security diff --git a/lib/galaxy/managers/markdown_util.py b/lib/galaxy/managers/markdown_util.py index 009aae3ea762..02545399e997 100644 --- a/lib/galaxy/managers/markdown_util.py +++ b/lib/galaxy/managers/markdown_util.py @@ -215,7 +215,9 @@ def _remap(container, line): url = trans.app.config.organization_url rval = self.handle_instance_organization_link(line, title, url) elif container == "invocation_time": - invocation = workflow_manager.get_invocation(trans, object_id) + invocation = workflow_manager.get_invocation( + trans, object_id, check_ownership=False, check_accessible=True + ) rval = self.handle_invocation_time(line, invocation) elif container == "visualization": rval = None diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 1259dd7719ad..a1534d6440cc 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -53,7 +53,10 @@ deletable, sharable, ) -from galaxy.managers.base import decode_id +from galaxy.managers.base import ( + decode_id, + security_check, +) from galaxy.managers.context import ProvidesUserContext from galaxy.managers.executables import artifact_class from galaxy.model import ( @@ -347,10 +350,10 @@ def check_security(self, trans, has_workflow, check_ownership=True, check_access if isinstance(has_workflow, model.WorkflowInvocation): # We use the owner of the history that is associated to the invocation as a proxy # for the owner of the invocation. - if trans.user != has_workflow.history.user and not trans.user_is_admin: - raise exceptions.ItemOwnershipException() - else: - return True + security_check( + trans, has_workflow.history, check_ownership=check_ownership, check_accessible=check_accessible + ) + return True # stored workflow contains security stuff - follow that workflow to # that unless given a stored workflow. @@ -404,20 +407,26 @@ def _workflow_to_svg_canvas(self, trans, workflow, for_embed=False): workflow_canvas.add_steps() return workflow_canvas.finish(for_embed=for_embed) - def get_invocation(self, trans, decoded_invocation_id: int, eager=False) -> WorkflowInvocation: + def get_invocation( + self, trans, decoded_invocation_id: int, eager=False, check_ownership=True, check_accessible=True + ) -> WorkflowInvocation: workflow_invocation = _get_invocation(trans.sa_session, eager, decoded_invocation_id) if not workflow_invocation: encoded_wfi_id = trans.security.encode_id(decoded_invocation_id) message = f"'{encoded_wfi_id}' is not a valid workflow invocation id" raise exceptions.ObjectNotFound(message) - self.check_security(trans, workflow_invocation, check_ownership=True, check_accessible=False) + self.check_security( + trans, workflow_invocation, check_ownership=check_accessible, check_accessible=check_accessible + ) return workflow_invocation def get_invocation_report(self, trans, invocation_id, **kwd): decoded_workflow_invocation_id = ( trans.security.decode_id(invocation_id) if isinstance(invocation_id, str) else invocation_id ) - workflow_invocation = self.get_invocation(trans, decoded_workflow_invocation_id) + workflow_invocation = self.get_invocation( + trans, decoded_workflow_invocation_id, check_ownership=False, check_accessible=True + ) generator_plugin_type = kwd.get("generator_plugin_type") runtime_report_config_json = kwd.get("runtime_report_config_json") invocation_markdown = kwd.get("invocation_markdown", None) @@ -433,7 +442,7 @@ def get_invocation_report(self, trans, invocation_id, **kwd): ) def request_invocation_cancellation(self, trans, decoded_invocation_id: int): - workflow_invocation = self.get_invocation(trans, decoded_invocation_id) + workflow_invocation = self.get_invocation(trans, decoded_invocation_id, check_ownership=True) cancelled = workflow_invocation.cancel() if cancelled: @@ -445,13 +454,18 @@ def request_invocation_cancellation(self, trans, decoded_invocation_id: int): return workflow_invocation - def get_invocation_step(self, trans, decoded_workflow_invocation_step_id): + def get_invocation_step( + self, trans, decoded_workflow_invocation_step_id, check_ownership=True, check_accessible=True + ): try: workflow_invocation_step = trans.sa_session.get(WorkflowInvocationStep, decoded_workflow_invocation_step_id) except Exception: raise exceptions.ObjectNotFound() self.check_security( - trans, workflow_invocation_step.workflow_invocation, check_ownership=True, check_accessible=False + trans, + workflow_invocation_step.workflow_invocation, + check_ownership=check_ownership, + check_accessible=check_accessible, ) return workflow_invocation_step @@ -490,6 +504,7 @@ def build_invocations_query( sort_by=None, sort_desc=None, include_nested_invocations=True, + check_ownership=True, ) -> Tuple[List, int]: """Get invocations owned by the current user.""" @@ -536,7 +551,7 @@ def build_invocations_query( invocations = [ inv for inv in trans.sa_session.scalars(stmt) - if self.check_security(trans, inv, check_ownership=True, check_accessible=False) + if self.check_security(trans, inv, check_ownership=check_ownership, check_accessible=True) ] return invocations, total_matches diff --git a/lib/galaxy/webapps/galaxy/services/invocations.py b/lib/galaxy/webapps/galaxy/services/invocations.py index 9b18db8f1b2d..2195c7202dd7 100644 --- a/lib/galaxy/webapps/galaxy/services/invocations.py +++ b/lib/galaxy/webapps/galaxy/services/invocations.py @@ -122,12 +122,15 @@ def index( sort_by=invocation_payload.sort_by, sort_desc=invocation_payload.sort_desc, include_nested_invocations=invocation_payload.include_nested_invocations, + check_ownership=False, ) invocation_dict = self.serialize_workflow_invocations(invocations, serialization_params) return invocation_dict, total_matches def show(self, trans, invocation_id, serialization_params, eager=False): - wfi = self._workflows_manager.get_invocation(trans, invocation_id, eager) + wfi = self._workflows_manager.get_invocation( + trans, invocation_id, eager, check_ownership=False, check_accessible=True + ) return self.serialize_workflow_invocation(wfi, serialization_params) def cancel(self, trans, invocation_id, serialization_params): @@ -139,7 +142,9 @@ def show_invocation_report(self, trans, invocation_id, format="json"): return wfi_report def show_invocation_step(self, trans, step_id) -> InvocationStep: - wfi_step = self._workflows_manager.get_invocation_step(trans, step_id) + wfi_step = self._workflows_manager.get_invocation_step( + trans, step_id, check_ownership=False, check_accessible=True + ) return self.serialize_workflow_invocation_step(wfi_step) def update_invocation_step(self, trans, step_id, action): @@ -164,7 +169,9 @@ def prepare_store_download( ) -> AsyncFile: ensure_celery_tasks_enabled(trans.app.config) model_store_format = payload.model_store_format - workflow_invocation = self._workflows_manager.get_invocation(trans, invocation_id, eager=True) + workflow_invocation = self._workflows_manager.get_invocation( + trans, invocation_id, eager=True, check_ownership=False, check_accessible=True + ) if not workflow_invocation: raise ObjectNotFound() try: @@ -190,7 +197,9 @@ def write_store( self, trans, invocation_id: DecodedDatabaseIdField, payload: WriteInvocationStoreToPayload ) -> AsyncTaskResultSummary: ensure_celery_tasks_enabled(trans.app.config) - workflow_invocation = self._workflows_manager.get_invocation(trans, invocation_id, eager=True) + workflow_invocation = self._workflows_manager.get_invocation( + trans, invocation_id, eager=True, check_ownership=False, check_accessible=True + ) if not workflow_invocation: raise ObjectNotFound() request = WriteInvocationTo( diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 88d4112966fd..bfa448edc8bc 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -2835,6 +2835,16 @@ def test_workflow_invocation_report_1(self): assert "## Workflow Outputs" in markdown_content assert "## Workflow Inputs" in markdown_content assert "## About This Report" not in markdown_content + with self._different_user(): + exception_raised = False + try: + self.workflow_populator.workflow_report_json(workflow_id, invocation_id) + except AssertionError as e: + if "Request status code (403)" in str(e): + exception_raised = True + assert exception_raised, "Expected workflow report request to fail, but it didn't" + self.dataset_populator.make_public(history_id) + self.workflow_populator.workflow_report_json(workflow_id, invocation_id) @skip_without_tool("cat") def test_workflow_invocation_report_custom(self):