fix #1010

qiita_db/meta_util.py

@@ -13,7 +13,6 @@
 ..autosummary::
     :toctree: generated/
 
-    get_accessible_filepath_ids
     get_lat_longs
 """
 # -----------------------------------------------------------------------------
@@ -59,78 +58,97 @@ def _get_data_fpids(constructor, object_id):
         return {fpid for fpid, _, _ in obj.get_filepaths()}
 
 
-def get_accessible_filepath_ids(user):
-    """Gets all filepaths that this user should have access to
-
-    This gets all raw, preprocessed, and processed filepaths, for studies
-    that the user has access to, as well as all the mapping files and biom
-    tables associated with the analyses that the user has access to.
+def validate_filepath_access_by_user(user, filepath_id):
+    """Validates if the user has access to the filepath_id
 
     Parameters
     ----------
     user : User object
         The user we are interested in
-
+    filepath_id : int
+        The filepath id
 
     Returns
     -------
-    set
-        A set of filepath ids
+    bool
+        If the user has access or not to the filepath_id
 
     Notes
     -----
-    Admins have access to all files, so all filepath ids are returned for
-    admins
+    Admins have access to all files so True is always returned
     """
-    with qdb.sql_connection.TRN:
+    TRN = qdb.sql_connection.TRN
+    with TRN:
         if user.level == "admin":
             # admins have access all files
-            qdb.sql_connection.TRN.add(
-                "SELECT filepath_id FROM qiita.filepath")
-            return set(qdb.sql_connection.TRN.execute_fetchflatten())
-
-        # First, the studies
-        # There are private and shared studies
-        studies = user.user_studies | user.shared_studies
-
-        filepath_ids = set()
-        for study in studies:
-            # Add the sample template files
-            if study.sample_template:
-                filepath_ids.update(
-                    {fid for fid, _ in study.sample_template.get_filepaths()})
-
-            # Add the prep template filepaths
-            for pt in study.prep_templates():
-                filepath_ids.update({fid for fid, _ in pt.get_filepaths()})
-
-            # Add the artifact filepaths
-            for artifact in study.artifacts():
-                filepath_ids.update({fid for fid, _, _ in artifact.filepaths})
-
-        # Next, the public artifacts
-        for artifact in qdb.artifact.Artifact.iter_public():
-            # Add the filepaths of the artifact
-            filepath_ids.update({fid for fid, _, _ in artifact.filepaths})
-
-            # Then add the filepaths of the prep templates
-            for pt in artifact.prep_templates:
-                filepath_ids.update({fid for fid, _ in pt.get_filepaths()})
-
-            # Then add the filepaths of the sample template
-            filepath_ids.update(
-                {fid
-                 for fid, _ in artifact.study.sample_template.get_filepaths()})
-
-        # Next, analyses
-        # Same as before, there are public, private, and shared
-        analyses = qdb.analysis.Analysis.get_by_status('public') | \
-            user.private_analyses | user.shared_analyses
-
-        for analysis in analyses:
-            filepath_ids.update(analysis.all_associated_filepath_ids)
-
-        return filepath_ids
+            return True
+
+        sql = """SELECT
+            (SELECT array_agg(artifact_id)
+             FROM qiita.artifact_filepath
+             WHERE filepath_id = {0}) AS artifact,
+            (SELECT array_agg(study_id)
+             FROM qiita.sample_template_filepath
+             WHERE filepath_id = {0}) AS sample_info,
+            (SELECT array_agg(prep_template_id)
+             FROM qiita.prep_template_filepath
+             WHERE filepath_id = {0}) AS prep_info,
+            (SELECT array_agg(job_id)
+             FROM qiita.job_results_filepath
+             WHERE filepath_id = {0}) AS job_results,
+            (SELECT array_agg(analysis_id)
+             FROM qiita.analysis_filepath
+             WHERE filepath_id = {0}) AS analysis""".format(filepath_id)
+        TRN.add(sql)
+
+        arid, sid, pid, jid, anid = TRN.execute_fetchflatten()
+
+        # artifacts
+        if arid:
+            # [0] cause we should only have 1
+            artifact = qdb.artifact.Artifact(arid[0])
+            if artifact.visibility == 'public':
+                return True
+            else:
+                # let's take the visibility via the Study
+                return artifact.study.has_access(user)
+        # sample info files
+        elif sid:
+            # the visibility of the sample info file is given by the
+            # study visibility
+            # [0] cause we should only have 1
+            return qdb.study.Study(sid[0]).has_access(user)
+        # prep info files
+        elif pid:
+            # the prep access is given by it's artifacts, if the user has
+            # access to any artifact, it should have access to the prep
+            # [0] cause we should only have 1
+            a = qdb.metadata_template.prep_template.PrepTemplate(
+                pid[0]).artifact
+            if (a.visibility == 'public' or a.study.has_access(user)):
+                return True
+            else:
+                for c in a.children:
+                    if (c.visibility == 'public' or c.study.has_access(user)):
+                        return True
+            return False
+        # analyses
+        elif anid or jid:
+            if jid:
+                # [0] cause we should only have 1
+                sql = """SELECT analysis_id FROM qiita.analysis_job
+                         WHERE job_id = {0}""".format(jid[0])
+                TRN.add(sql)
+                aid = TRN.execute_fetchlast()
+            else:
+                aid = anid[0]
+            # [0] cause we should only have 1
+            analysis = qdb.analysis.Analysis(aid)
+            if analysis.status == 'public':
+                return True
+            else:
+                return analysis in (
+                    user.private_analyses | user.shared_analyses)
 
 
 def update_redis_stats():

qiita_db/support_files/patches/50.sql

@@ -0,0 +1,15 @@
+-- Feb 1, 2017
+-- adding study tag system
+
+
+CREATE TABLE qiita.study_tags (
+  study_tag_id bigserial NOT NULL,
+	study_tag varchar NOT NULL,
+  CONSTRAINT pk_study_tag UNIQUE ( study_tag_id ),
+  CONSTRAINT pk_study_tag_id PRIMARY KEY ( study_tag_id )
+) ;
+
+CREATE INDEX idx_study_tag_id ON qiita.study_tags ( study_tag_id ) ;
+
+-- inserting tags for GOLD
+INSERT INTO qiita.study_tags (study_tag) VALUES ('GOLD');

qiita_db/support_files/populate_test_db.sql

@@ -11,7 +11,7 @@ INSERT INTO qiita.qiita_user (email, user_level_id, password, name,
     '$2a$12$gnUi8Qg.0tvW243v889BhOBhWLIHyIJjjgaG6dxuRJkUM8nXG9Efe', 'Dude',
     'Nowhere University', '123 fake st, Apt 0, Faketown, CO 80302',
     '111-222-3344'),
-    ('shared@foo.bar', 3,
+    ('shared@foo.bar', 4,
     '$2a$12$gnUi8Qg.0tvW243v889BhOBhWLIHyIJjjgaG6dxuRJkUM8nXG9Efe', 'Shared',
     'Nowhere University', '123 fake st, Apt 0, Faketown, CO 80302',
     '111-222-3344'),

qiita_db/test/test_meta_util.py

@@ -33,46 +33,37 @@ def _set_artifact_public(self):
         self.conn_handler.execute(
             "UPDATE qiita.artifact SET visibility_id=2")
 
-    def _unshare_studies(self):
-        self.conn_handler.execute("DELETE FROM qiita.study_users")
-
-    def _unshare_analyses(self):
-        self.conn_handler.execute("DELETE FROM qiita.analysis_users")
-
-    def test_get_accessible_filepath_ids(self):
+    def test_validate_filepath_access_by_user(self):
         self._set_artifact_private()
 
         # shared has access to all study files and analysis files
-
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('shared@foo.bar'))
-        self.assertItemsEqual(obs, {
-            1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21})
+        user = qdb.user.User('shared@foo.bar')
+        for i in [1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # Now shared should not have access to the study files
-        self._unshare_studies()
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('shared@foo.bar'))
-        self.assertItemsEqual(obs, {16, 14, 15, 13})
+        qdb.study.Study(1).unshare(user)
+        for i in [1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21]:
+            self.assertFalse(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
+        for i in [13, 14, 15, 16]:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # Now shared should not have access to any files
-        self._unshare_analyses()
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('shared@foo.bar'))
-        self.assertEqual(obs, set())
+        qdb.analysis.Analysis(1).unshare(user)
+        for i in [1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]:
+            self.assertFalse(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # Now shared has access to public study files
         self._set_artifact_public()
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('shared@foo.bar'))
-        self.assertEqual(obs, {1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21})
+        for i in [1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21]:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # Test that it doesn't break: if the SampleTemplate hasn't been added
-        exp = {1, 2, 3, 4, 5, 9, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('test@foo.bar'))
-        self.assertEqual(obs, exp)
-
         info = {
             "timeseries_type_id": 1,
             "metadata_complete": True,
@@ -86,26 +77,31 @@ def test_get_accessible_filepath_ids(self):
             "principal_investigator_id": 1,
             "lab_person_id": 1
         }
-        qdb.study.Study.create(
+        study = qdb.study.Study.create(
             qdb.user.User('test@foo.bar'), "Test study", [1], info)
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('test@foo.bar'))
-        self.assertEqual(obs, exp)
+        for i in [1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21]:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # test in case there is a prep template that failed
         self.conn_handler.execute(
             "INSERT INTO qiita.prep_template (data_type_id) VALUES (2)")
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('test@foo.bar'))
-        self.assertEqual(obs, exp)
+        for i in [1, 2, 3, 4, 5, 9, 12, 17, 18, 19, 20, 21]:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                user, i))
 
         # admin should have access to everything
-        count = self.conn_handler.execute_fetchone("SELECT count(*) FROM "
-                                                   "qiita.filepath")[0]
-        exp = set(range(1, count + 1))
-        obs = qdb.meta_util.get_accessible_filepath_ids(
-            qdb.user.User('admin@foo.bar'))
-        self.assertEqual(obs, exp)
+        admin = qdb.user.User('admin@foo.bar')
+        fids = self.conn_handler.execute_fetchall(
+            "SELECT filepath_id FROM qiita.filepath")
+        for i in fids:
+            self.assertTrue(qdb.meta_util.validate_filepath_access_by_user(
+                admin, i[0]))
+
+        # returning to origina sharing
+        qdb.study.Study(1).share(user)
+        qdb.analysis.Analysis(1).share(user)
+        qdb.study.Study.delete(study.id)
 
     def test_get_lat_longs(self):
         exp = [
@@ -171,7 +167,7 @@ def test_get_lat_longs_EMP_portal(self):
         }
 
         md_ext = pd.DataFrame.from_dict(md, orient='index', dtype=str)
-        qdb.metadata_template.sample_template.SampleTemplate.create(
+        st = qdb.metadata_template.sample_template.SampleTemplate.create(
             md_ext, study)
 
         qiita_config.portal = 'EMP'
@@ -180,19 +176,21 @@ def test_get_lat_longs_EMP_portal(self):
         exp = [[42.42, 41.41]]
 
         self.assertItemsEqual(obs, exp)
+        qdb.metadata_template.sample_template.SampleTemplate.delete(st.id)
+        qdb.study.Study.delete(study.id)
 
     def test_update_redis_stats(self):
         qdb.meta_util.update_redis_stats()
 
         portal = qiita_config.portal
         vals = [
-            ('number_studies', {'sanbox': '2', 'public': '0',
-                                'private': '1'}, r_client.hgetall),
-            ('number_of_samples', {'sanbox': '1', 'public': '0',
-                                   'private': '27'}, r_client.hgetall),
+            ('number_studies', {'sanbox': '0', 'public': '1',
+                                'private': '0'}, r_client.hgetall),
+            ('number_of_samples', {'sanbox': '0', 'public': '27',
+                                   'private': '0'}, r_client.hgetall),
             ('num_users', '4', r_client.get),
             ('lat_longs', EXP_LAT_LONG, r_client.get),
-            ('num_studies_ebi', '3', r_client.get),
+            ('num_studies_ebi', '1', r_client.get),
             ('num_samples_ebi', '27', r_client.get),
             ('number_samples_ebi_prep', '54', r_client.get)
             # not testing img/time for simplicity
@@ -205,19 +203,19 @@ def test_update_redis_stats(self):
 
 
 EXP_LAT_LONG = (
-    '[[0.291867635913, 68.5945325743], [68.0991287718, 34.8360987059],'
-    ' [10.6655599093, 70.784770579], [40.8623799474, 6.66444220187],'
+    '[[60.1102854322, 74.7123248382], [23.1218032799, 42.838497795],'
+    ' [3.21190859967, 26.8138925876], [74.0894932572, 65.3283470202],'
+    ' [53.5050692395, 31.6056761814], [12.6245524972, 96.0693176066],'
+    ' [43.9614715197, 82.8516734159], [10.6655599093, 70.784770579],'
+    ' [78.3634273709, 74.423907894], [82.8302905615, 86.3615778099],'
+    ' [44.9725384282, 66.1920014699], [4.59216095574, 63.5115213108],'
+    ' [57.571893782, 32.5563076447], [40.8623799474, 6.66444220187],'
+    ' [95.2060749748, 27.3592668624], [38.2627021402, 3.48274264219],'
     ' [13.089194595, 92.5274472082], [84.0030227585, 66.8954849864],'
-    ' [12.7065957714, 84.9722975792], [78.3634273709, 74.423907894],'
-    ' [82.8302905615, 86.3615778099], [53.5050692395, 31.6056761814],'
-    ' [43.9614715197, 82.8516734159], [29.1499460692, 82.1270418227],'
-    ' [23.1218032799, 42.838497795], [12.6245524972, 96.0693176066],'
-    ' [38.2627021402, 3.48274264219], [74.0894932572, 65.3283470202],'
-    ' [35.2374368957, 68.5041623253], [4.59216095574, 63.5115213108],'
-    ' [95.2060749748, 27.3592668624], [68.51099627, 2.35063674718],'
-    ' [85.4121476399, 15.6526750776], [60.1102854322, 74.7123248382],'
-    ' [3.21190859967, 26.8138925876], [57.571893782, 32.5563076447],'
-    ' [44.9725384282, 66.1920014699], [42.42, 41.41]]')
+    ' [68.51099627, 2.35063674718], [29.1499460692, 82.1270418227],'
+    ' [35.2374368957, 68.5041623253], [12.7065957714, 84.9722975792],'
+    ' [0.291867635913, 68.5945325743], [85.4121476399, 15.6526750776],'
+    ' [68.0991287718, 34.8360987059]]')
 
 if __name__ == '__main__':
     main()

qiita_pet/handlers/download.py

@@ -5,23 +5,21 @@
 from .base_handlers import BaseHandler
 from qiita_pet.exceptions import QiitaPetAuthorizationError
 from qiita_db.util import filepath_id_to_rel_path
-from qiita_db.meta_util import get_accessible_filepath_ids
+from qiita_db.meta_util import validate_filepath_access_by_user
 from qiita_core.util import execute_as_transaction
 
 
 class DownloadHandler(BaseHandler):
     @authenticated
     @execute_as_transaction
     def get(self, filepath_id):
-        filepath_id = int(filepath_id)
-        # Check access to file
-        accessible_filepaths = get_accessible_filepath_ids(self.current_user)
+        fid = int(filepath_id)
 
-        if filepath_id not in accessible_filepaths:
+        if not validate_filepath_access_by_user(self.current_user, fid):
             raise QiitaPetAuthorizationError(
-                self.current_user, 'filepath id %s' % str(filepath_id))
+                self.current_user, 'filepath id %s' % str(fid))
 
-        relpath = filepath_id_to_rel_path(filepath_id)
+        relpath = filepath_id_to_rel_path(fid)
         fname = basename(relpath)
 
         # If we don't have nginx, write a file that indicates this