Merge pull request #4465 from mvdbeek/workflow_invocation_security_check

Fix checking WorkflowInvocation for published workflows

lib/galaxy/managers/workflows.py

@@ -66,7 +66,7 @@ class WorkflowsManager(object):
         stored_workflow = self.get_stored_workflow(trans, workflow_id)
 
         # check to see if user has permissions to selected workflow
-        if stored_workflow.user != trans.user and not trans.user_is_admin():
+        if stored_workflow.user != trans.user and not trans.user_is_admin() and not stored_workflow.published:
             if trans.sa_session.query(trans.app.model.StoredWorkflowUserShareAssociation).filter_by(user=trans.user, stored_workflow=stored_workflow).count() == 0:
                 message = "Workflow is not owned by or shared with current user"
                 raise exceptions.ItemAccessibilityException(message)
@@ -87,12 +87,17 @@ class WorkflowsManager(object):
         workflowinvocations. Throw an exception or returns True if user has
         needed level of access.
         """
-        if not check_ownership or check_accessible:
+        if not check_ownership and not check_accessible:
             return True
 
-        # If given an invocation follow to workflow...
+        # If given an invocation verify ownership of invocation
         if isinstance(has_workflow, model.WorkflowInvocation):
-            has_workflow = has_workflow.workflow
+            # We use the 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
 
         # stored workflow contains security stuff - follow that workflow to
         # that unless given a stored workflow.
@@ -111,12 +116,13 @@ class WorkflowsManager(object):
         return True
 
     def get_invocation(self, trans, decoded_invocation_id):
-        try:
-            workflow_invocation = trans.sa_session.query(
-                self.app.model.WorkflowInvocation
-            ).get(decoded_invocation_id)
-        except Exception:
-            raise exceptions.ObjectNotFound()
+        workflow_invocation = trans.sa_session.query(
+            self.app.model.WorkflowInvocation
+        ).get(decoded_invocation_id)
+        if not workflow_invocation:
+            encoded_wfi_id = trans.security.encode_id(decoded_invocation_id)
+            message = "'%s' is not a valid workflow invocation id" % encoded_wfi_id
+            raise exceptions.ObjectNotFound(message)
         self.check_security(trans, workflow_invocation, check_ownership=True, check_accessible=False)
         return workflow_invocation
 
@@ -161,18 +167,21 @@ class WorkflowsManager(object):
         return workflow_invocation_step
 
     def build_invocations_query(self, trans, decoded_stored_workflow_id):
-        try:
-            stored_workflow = trans.sa_session.query(
-                self.app.model.StoredWorkflow
-            ).get(decoded_stored_workflow_id)
-        except Exception:
+        """Get invocations owned by the current user."""
+        stored_workflow = trans.sa_session.query(
+            self.app.model.StoredWorkflow
+        ).get(decoded_stored_workflow_id)
+        if not stored_workflow:
             raise exceptions.ObjectNotFound()
-        self.check_security(trans, stored_workflow, check_ownership=True, check_accessible=False)
-        return trans.sa_session.query(
+        invocations = trans.sa_session.query(
             model.WorkflowInvocation
         ).filter_by(
             workflow_id=stored_workflow.latest_workflow_id
         )
+        return [inv for inv in invocations if self.check_security(trans,
+                                                                  inv,
+                                                                  check_ownership=True,
+                                                                  check_accessible=False)]
 
 
 CreatedWorkflow = namedtuple("CreatedWorkflow", ["stored_workflow", "workflow", "missing_tools"])

test/api/test_workflows.py

@@ -92,22 +92,23 @@ class BaseWorkflowsApiTestCase(api.ApiTestCase):
     def _upload_yaml_workflow(self, has_yaml, **kwds):
         return self.workflow_populator.upload_yaml_workflow(has_yaml, **kwds)
 
-    def _setup_workflow_run(self, workflow, inputs_by='step_id', history_id=None):
-        uploaded_workflow_id = self.workflow_populator.create_workflow(workflow)
+    def _setup_workflow_run(self, workflow=None, inputs_by='step_id', history_id=None, workflow_id=None):
+        if not workflow_id:
+            workflow_id = self.workflow_populator.create_workflow(workflow)
         if not history_id:
             history_id = self.dataset_populator.new_history()
         hda1 = self.dataset_populator.new_dataset(history_id, content="1 2 3")
         hda2 = self.dataset_populator.new_dataset(history_id, content="4 5 6")
         workflow_request = dict(
             history="hist_id=%s" % history_id,
-            workflow_id=uploaded_workflow_id,
+            workflow_id=workflow_id,
         )
         label_map = {
             'WorkflowInput1': self._ds_entry(hda1),
             'WorkflowInput2': self._ds_entry(hda2)
         }
         if inputs_by == 'step_id':
-            ds_map = self._build_ds_map(uploaded_workflow_id, label_map)
+            ds_map = self._build_ds_map(workflow_id, label_map)
             workflow_request["ds_map"] = ds_map
         elif inputs_by == "step_index":
             index_map = {
@@ -1816,6 +1817,56 @@ steps:
         self._assert_status_code_is(step_response, 200)
         self._assert_has_keys(step_response.json(), "id", "order_index")
 
+    @skip_without_tool("cat1")
+    def test_invocations_accessible_imported_workflow(self):
+        workflow_id = self.workflow_populator.simple_workflow("test_usage", publish=True)
+        with self._different_user():
+            other_import_response = self.__import_workflow(workflow_id)
+            self._assert_status_code_is(other_import_response, 200)
+            other_id = other_import_response.json()["id"]
+            workflow_request, history_id = self._setup_workflow_run(workflow_id=other_id)
+            response = self._get("workflows/%s/usage" % other_id)
+            self._assert_status_code_is(response, 200)
+            assert len(response.json()) == 0
+            run_workflow_response = self._post("workflows", data=workflow_request)
+            self._assert_status_code_is(run_workflow_response, 200)
+            run_workflow_response = run_workflow_response.json()
+            invocation_id = run_workflow_response['id']
+            usage_details_response = self._get("workflows/%s/usage/%s" % (other_id, invocation_id))
+            self._assert_status_code_is(usage_details_response, 200)
+
+    @skip_without_tool("cat1")
+    def test_invocations_accessible_published_workflow(self):
+        workflow_id = self.workflow_populator.simple_workflow("test_usage", publish=True)
+        with self._different_user():
+            workflow_request, history_id = self._setup_workflow_run(workflow_id=workflow_id)
+            workflow_request['workflow_id'] = workflow_request.pop('workflow_id')
+            response = self._get("workflows/%s/usage" % workflow_id)
+            self._assert_status_code_is(response, 200)
+            assert len(response.json()) == 0
+            run_workflow_response = self._post("workflows", data=workflow_request)
+            self._assert_status_code_is(run_workflow_response, 200)
+            run_workflow_response = run_workflow_response.json()
+            invocation_id = run_workflow_response['id']
+            usage_details_response = self._get("workflows/%s/usage/%s" % (workflow_id, invocation_id))
+            self._assert_status_code_is(usage_details_response, 200)
+
+    @skip_without_tool("cat1")
+    def test_invocations_not_accessible_by_different_user_for_published_workflow(self):
+        workflow_id = self.workflow_populator.simple_workflow("test_usage", publish=True)
+        workflow_request, history_id = self._setup_workflow_run(workflow_id=workflow_id)
+        workflow_request['workflow_id'] = workflow_request.pop('workflow_id')
+        response = self._get("workflows/%s/usage" % workflow_id)
+        self._assert_status_code_is(response, 200)
+        assert len(response.json()) == 0
+        run_workflow_response = self._post("workflows", data=workflow_request)
+        self._assert_status_code_is(run_workflow_response, 200)
+        run_workflow_response = run_workflow_response.json()
+        invocation_id = run_workflow_response['id']
+        with self._different_user():
+            usage_details_response = self._get("workflows/%s/usage/%s" % (workflow_id, invocation_id))
+            self._assert_status_code_is(usage_details_response, 403)
+
     def _update_workflow(self, workflow_id, workflow_object):
         data = dict(
             workflow=workflow_object

test/unit/workflows/workflow_support.py

@@ -18,6 +18,7 @@ class MockTrans(object):
     def save_workflow(self, workflow):
         stored_workflow = model.StoredWorkflow()
         stored_workflow.latest_workflow = workflow
+        workflow.stored_workflow = stored_workflow
         stored_workflow.user = self.user
         self.sa_session.add(stored_workflow)
         self.sa_session.flush()