Merge pull request galaxyproject#18707 from mvdbeek/allow_access_to_i…

…nvocation_via_shared_history

Allow access to invocation via shared or published history

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

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

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
 

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(

lib/galaxy_test/api/test_workflows.py

@@ -2847,6 +2847,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):