private objectstores & dataset.sharable

Setup abstractions to prevent sharing transient or private per-user objects in an objectstore.

client/src/components/Dataset/DatasetStorage/DatasetStorage.test.js

@@ -10,14 +10,17 @@ const localVue = getLocalVue();
 
 const TEST_STORAGE_API_RESPONSE_WITHOUT_ID = {
     object_store_id: null,
+    private: false,
 };
 const TEST_STORAGE_API_RESPONSE_WITH_ID = {
     object_store_id: "foobar",
+    private: false,
 };
 const TEST_STORAGE_API_RESPONSE_WITH_NAME = {
     object_store_id: "foobar",
     name: "my cool storage",
     description: "My cool **markdown**",
+    private: true,
 };
 const TEST_DATASET_ID = "1";
 const TEST_STORAGE_URL = `/api/datasets/${TEST_DATASET_ID}/storage`;
@@ -46,9 +49,6 @@ describe("Dataset Storage", () => {
         wrapper = shallowMount(DatasetStorage, {
             propsData: { datasetId: TEST_DATASET_ID },
             localVue,
-            stubs: {
-                "loading-span": true,
-            },
         });
     }
 
@@ -102,6 +102,7 @@ describe("Dataset Storage", () => {
         expect(byIdSpan.length).toBe(1);
         const byNameSpan = wrapper.findAll(".display-os-by-name");
         expect(byNameSpan.length).toBe(0);
+        expect(wrapper.find("object-store-restriction-span-stub").props("isPrivate")).toBeFalsy();
     });
 
     it("test dataset storage with object store name", async () => {
@@ -116,6 +117,7 @@ describe("Dataset Storage", () => {
         expect(byIdSpan.length).toBe(0);
         const byNameSpan = wrapper.findAll(".display-os-by-name");
         expect(byNameSpan.length).toBe(1);
+        expect(wrapper.find("object-store-restriction-span-stub").props("isPrivate")).toBeTruthy();
     });
 
     afterEach(() => {

client/src/components/Dataset/DatasetStorage/DatasetStorage.vue

@@ -18,13 +18,17 @@
         <div v-else>
             <p>
                 This dataset is stored in
-                <span v-if="storageInfo.name" class="display-os-by-name">
-                    a Galaxy object store named <b>{{ storageInfo.name }}</b>
+                <span class="display-os-by-name" v-if="storageInfo.name">
+                    a Galaxy <object-store-restriction-span :is-private="isPrivate" /> object store named
+                    <b>{{ storageInfo.name }}</b>
                 </span>
-                <span v-else-if="storageInfo.object_store_id" class="display-os-by-id">
-                    a Galaxy object store with id <b>{{ storageInfo.object_store_id }}</b>
+                <span class="display-os-by-id" v-else-if="storageInfo.object_store_id">
+                    a Galaxy <object-store-restriction-span :is-private="isPrivate" /> object store with id
+                    <b>{{ storageInfo.object_store_id }}</b>
                 </span>
-                <span v-else class="display-os-default"> the default configured Galaxy object store </span>.
+                <span class="display-os-default" v-else>
+                    the default configured Galaxy <object-store-restriction-span :is-private="isPrivate" /> object store </span
+                >.
             </p>
             <div v-html="descriptionRendered"></div>
         </div>
@@ -37,10 +41,12 @@ import { getAppRoot } from "onload/loadConfig";
 import LoadingSpan from "components/LoadingSpan";
 import MarkdownIt from "markdown-it";
 import { errorMessageAsString } from "utils/simple-error";
+import ObjectStoreRestrictionSpan from "./ObjectStoreRestrictionSpan";
 
 export default {
     components: {
         LoadingSpan,
+        ObjectStoreRestrictionSpan,
     },
     props: {
         datasetId: {
@@ -80,6 +86,9 @@ export default {
             }
             return rootSources[0].source_uri;
         },
+        isPrivate() {
+            return this.storageInfo.private;
+        },
     },
     created() {
         const datasetId = this.datasetId;

client/src/components/Dataset/DatasetStorage/ObjectStoreRestrictionSpan.test.js

@@ -0,0 +1,27 @@
+import { shallowMount } from "@vue/test-utils";
+import { getLocalVue } from "jest/helpers";
+import ObjectStoreRestrictionSpan from "./ObjectStoreRestrictionSpan";
+
+const localVue = getLocalVue();
+
+describe("ObjectStoreRestrictionSpan", () => {
+    let wrapper;
+
+    it("should render info about private storage if isPrivate", () => {
+        wrapper = shallowMount(ObjectStoreRestrictionSpan, {
+            propsData: { isPrivate: true },
+            localVue,
+        });
+        expect(wrapper.find(".stored-how").text()).toBe("private");
+        expect(wrapper.find(".stored-how").attributes("title")).toBeTruthy();
+    });
+
+    it("should render info about unrestricted storage if not isPrivate", () => {
+        wrapper = shallowMount(ObjectStoreRestrictionSpan, {
+            propsData: { isPrivate: false },
+            localVue,
+        });
+        expect(wrapper.find(".stored-how").text()).toBe("unrestricted");
+        expect(wrapper.find(".stored-how").attributes("title")).toBeTruthy();
+    });
+});

client/src/components/Dataset/DatasetStorage/ObjectStoreRestrictionSpan.vue

@@ -0,0 +1,38 @@
+<template>
+    <span class="stored-how" v-b-tooltip.hover :title="title">{{ text }}</span>
+</template>
+
+<script>
+import Vue from "vue";
+import BootstrapVue from "bootstrap-vue";
+Vue.use(BootstrapVue);
+
+export default {
+    props: {
+        isPrivate: {
+            // private is reserved word
+            type: Boolean,
+        },
+    },
+    computed: {
+        text() {
+            return this.isPrivate ? "private" : "unrestricted";
+        },
+        title() {
+            if (this.isPrivate) {
+                return "This dataset is stored on storage restricted to a single user. It can not be shared, pubished, or added to Galaxy data libraries.";
+            } else {
+                return "This dataset is stored on unrestricted storage. With sufficient Galaxy permissions, this dataset can be published, shared, or added to Galaxy data libraries.";
+            }
+        },
+    },
+};
+</script>
+
+<style scoped>
+/* Give visual indication of mouseover info */
+.stored-how {
+    text-decoration-line: underline;
+    text-decoration-style: dashed;
+}
+</style>

client/src/mvc/library/library-model.js

@@ -174,7 +174,7 @@ var HistoryContents = Backbone.Collection.extend({
         this.id = options.id;
     },
     url: function () {
-        return `${this.urlRoot + this.id}/contents`;
+        return `${this.urlRoot + this.id}/contents?sharable=true`;
     },
     model: HistoryItem,
 });

lib/galaxy/job_execution/output_collect.py

@@ -341,7 +341,7 @@ def add_library_dataset_to_folder(self, library_folder, ld):
         trans.app.security_agent.copy_library_permissions(trans, ld, ldda)
         # Copy the current user's DefaultUserPermissions to the new LibraryDatasetDatasetAssociation.dataset
         trans.app.security_agent.set_all_dataset_permissions(
-            ldda.dataset, trans.app.security_agent.user_get_default_permissions(trans.user)
+            ldda.dataset, trans.app.security_agent.user_get_default_permissions(trans.user), flush=False, new=True
         )
         library_folder.add_library_dataset(ld, genome_build=ldda.dbkey)
         trans.sa_session.add(library_folder)

lib/galaxy/jobs/__init__.py

@@ -1584,6 +1584,8 @@ def _set_object_store_ids(self, job):
 
         object_store_populator = ObjectStorePopulator(self.app, job.user)
         object_store_id = self.get_destination_configuration("object_store_id", None)
+        require_sharable = job.requires_sharable_storage(self.app.security_agent)
+
         if object_store_id:
             object_store_populator.object_store_id = object_store_id
 
@@ -1595,7 +1597,7 @@ def _set_object_store_ids(self, job):
         # afterward. State below needs to happen the same way.
         for dataset_assoc in job.output_datasets + job.output_library_datasets:
             dataset = dataset_assoc.dataset
-            object_store_populator.set_object_store_id(dataset)
+            object_store_populator.set_object_store_id(dataset, require_sharable=require_sharable)
 
         job.object_store_id = object_store_populator.object_store_id
         self._setup_working_directory(job=job)

lib/galaxy/jobs/runners/__init__.py

@@ -167,6 +167,8 @@ def put(self, job_wrapper):
         """Add a job to the queue (by job identifier), indicate that the job is ready to run."""
         put_timer = ExecutionTimer()
         queue_job = job_wrapper.enqueue()
+        # TODO: @jmchilton rebasing on a branch that had above line wrapped in an exception
+        # handler that would call fail on job_wrapper. Needed?
         if queue_job:
             self.mark_as_queued(job_wrapper)
             log.debug(f"Job [{job_wrapper.job_id}] queued {put_timer}")

lib/galaxy/model/__init__.py

@@ -172,6 +172,7 @@
 # Tags that get automatically propagated from inputs to outputs when running jobs.
 AUTO_PROPAGATED_TAGS = ["name"]
 YIELD_PER_ROWS = 100
+CANNOT_SHARE_PRIVATE_DATASET_MESSAGE = "Attempting to share a non-sharable dataset."
 
 
 if TYPE_CHECKING:
@@ -1470,6 +1471,19 @@ def remap_objects(p, k, obj):
         job_attrs["params"] = params_dict
         return job_attrs
 
+    def requires_sharable_storage(self, security_agent):
+        # An easy optimization would be to calculate this in galaxy.tools.actions when the
+        # job is created and all the output permissions are already known. Having to reload
+        # these permissions in the job code shouldn't strictly be needed.
+
+        requires_sharing = False
+        for dataset_assoc in self.output_datasets + self.output_library_datasets:
+            if not security_agent.dataset_is_private_to_a_user(dataset_assoc.dataset.dataset):
+                requires_sharing = True
+                break
+
+        return requires_sharing
+
     def to_dict(self, view="collection", system_details=False):
         if view == "admin_job_list":
             rval = super().to_dict(view="collection")
@@ -2979,7 +2993,14 @@ def __filter_contents(self, content_class, **kwds):
         visible = galaxy.util.string_as_bool_or_none(kwds.get("visible", None))
         if visible is not None:
             query = query.filter(content_class.visible == visible)
+        if "object_store_ids" in kwds:
+            if content_class == HistoryDatasetAssociation:
+                query = query.join(content_class.dataset).filter(
+                    Dataset.table.c.object_store_id.in_(kwds.get("object_store_ids"))
+                )
+            # else ignoring object_store_ids on HDCAs...
         if "ids" in kwds:
+            assert "object_store_ids" not in kwds
             ids = kwds["ids"]
             max_in_filter_length = kwds.get("max_in_filter_length", MAX_IN_FILTER_LENGTH)
             if len(ids) < max_in_filter_length:
@@ -3485,14 +3506,27 @@ def is_new(self):
     def in_ready_state(self):
         return self.state in self.ready_states
 
+    @property
+    def sharable(self):
+        """Return True if placed into an objectstore not labeled as ``private``."""
+        if self.external_filename:
+            return True
+        else:
+            object_store = self._assert_object_store_set()
+            return not object_store.is_private(self)
+
+    def ensure_sharable(self):
+        if not self.sharable:
+            raise Exception(CANNOT_SHARE_PRIVATE_DATASET_MESSAGE)
+
     def get_file_name(self):
         if self.purged:
             log.warning(f"Attempt to get file name of purged dataset {self.id}")
             return ""
         if not self.external_filename:
-            assert self.object_store is not None, f"Object Store has not been initialized for dataset {self.id}"
-            if self.object_store.exists(self):
-                file_name = self.object_store.get_filename(self)
+            object_store = self._assert_object_store_set()
+            if object_store.exists(self):
+                file_name = object_store.get_filename(self)
             else:
                 file_name = ""
             if not file_name and self.state not in (self.states.NEW, self.states.QUEUED):
@@ -3513,6 +3547,10 @@ def set_file_name(self, filename):
 
     file_name = property(get_file_name, set_file_name)
 
+    def _assert_object_store_set(self):
+        assert self.object_store is not None, f"Object Store has not been initialized for dataset {self.id}"
+        return self.object_store
+
     def get_extra_files_path(self):
         # Unlike get_file_name - external_extra_files_path is not backed by an
         # actual database column so if SA instantiates this object - the
@@ -4553,6 +4591,9 @@ def to_library_dataset_dataset_association(
         """
         Copy this HDA to a library optionally replacing an existing LDDA.
         """
+        if not self.dataset.sharable:
+            raise Exception("Attempting to share a non-sharable dataset.")
+
         if replace_dataset:
             # The replace_dataset param ( when not None ) refers to a LibraryDataset that
             #   is being replaced with a new version.

lib/galaxy/model/security.py

@@ -899,16 +899,23 @@ def set_all_dataset_permissions(self, dataset, permissions=None, new=False, flus
         # Make sure that DATASET_MANAGE_PERMISSIONS is associated with at least 1 role
         has_dataset_manage_permissions = False
         permissions = permissions or {}
-        for action, roles in permissions.items():
-            if isinstance(action, Action):
-                if action == self.permitted_actions.DATASET_MANAGE_PERMISSIONS and roles:
-                    has_dataset_manage_permissions = True
-                    break
-            elif action == self.permitted_actions.DATASET_MANAGE_PERMISSIONS.action and roles:
-                has_dataset_manage_permissions = True
-                break
+        for _ in _walk_action_roles(permissions, self.permitted_actions.DATASET_MANAGE_PERMISSIONS):
+            has_dataset_manage_permissions = True
+            break
         if not has_dataset_manage_permissions:
             return "At least 1 role must be associated with manage permissions on this dataset."
+
+        # If this is new, the objectstore likely hasn't been set yet - defer check until
+        # the job handler assigns it.
+        if not new and not dataset.sharable:
+            # ensure dataset not shared.
+            dataset_access_roles = []
+            for _, roles in _walk_action_roles(permissions, self.permitted_actions.DATASET_ACCESS):
+                dataset_access_roles.extend(roles)
+
+            if len(dataset_access_roles) != 1 or dataset_access_roles[0].type != self.model.Role.types.PRIVATE:
+                return galaxy.model.CANNOT_SHARE_PRIVATE_DATASET_MESSAGE
+
         flush_needed = False
         # Delete all of the current permissions on the dataset
         if not new:
@@ -937,6 +944,12 @@ def set_dataset_permission(self, dataset, permission=None):
         Permission looks like: { Action.action : [ Role, Role ] }
         """
         permission = permission or {}
+
+        # if modifying access - ensure it is sharable.
+        for _ in _walk_action_roles(permission, self.permitted_actions.DATASET_ACCESS):
+            dataset.ensure_sharable()
+            break
+
         flush_needed = False
         for action, roles in permission.items():
             if isinstance(action, Action):
@@ -976,6 +989,7 @@ def copy_dataset_permissions(self, src, dst):
         self.set_all_dataset_permissions(dst, self.get_permissions(src))
 
     def privately_share_dataset(self, dataset, users=None):
+        dataset.ensure_sharable()
         intersect = None
         users = users or []
         for user in users:
@@ -1154,6 +1168,19 @@ def dataset_is_private_to_user(self, trans, dataset):
             else:
                 return False
 
+    def dataset_is_private_to_a_user(self, dataset):
+        """
+        If the Dataset object has exactly one access role and that is
+        the current user's private role then we consider the dataset private.
+        """
+        access_roles = dataset.get_access_roles(self)
+
+        if len(access_roles) != 1:
+            return False
+        else:
+            access_role = access_roles[0]
+            return access_role.type == self.model.Role.types.PRIVATE
+
     def datasets_are_public(self, trans, datasets):
         """
         Given a transaction object and a list of Datasets, return
@@ -1188,6 +1215,8 @@ def datasets_are_public(self, trans, datasets):
     def make_dataset_public(self, dataset):
         # A dataset is considered public if there are no "access" actions associated with it.  Any
         # other actions ( 'manage permissions', 'edit metadata' ) are irrelevant.
+        dataset.ensure_sharable()
+
         flush_needed = False
         for dp in dataset.actions:
             if dp.action == self.permitted_actions.DATASET_ACCESS.action:
@@ -1632,3 +1661,12 @@ def set_dataset_permissions(self, hda, user, site):
             hdadaa = self.model.HistoryDatasetAssociationDisplayAtAuthorization(hda=hda, user=user, site=site)
         self.sa_session.add(hdadaa)
         self.sa_session.flush()
+
+
+def _walk_action_roles(permissions, query_action):
+    for action, roles in permissions.items():
+        if isinstance(action, Action):
+            if action == query_action and roles:
+                yield action, roles
+        elif action == query_action.action and roles:
+            yield action, roles