From 9435939613b5bcb69eecba70aed06baeebcae193 Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Thu, 26 Jun 2014 14:23:17 -0600 Subject: [PATCH 1/8] initial addition of analysis access conrol --- qiita_db/analysis.py | 15 ++++++++ qiita_db/study.py | 5 ++- qiita_pet/handlers/analysis_handlers.py | 48 +++++++++++++++++++------ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/qiita_db/analysis.py b/qiita_db/analysis.py index 310ec50bf..cbb0a33ff 100644 --- a/qiita_db/analysis.py +++ b/qiita_db/analysis.py @@ -64,6 +64,21 @@ def _status_setter_checks(self, conn_handler): """ self._lock_check(conn_handler) + @classmethod + def get_public(cls): + """Returns analysis id for all public Analyses + + Returns + ------- + list of int + All public analysses in the database + """ + conn_handler = SQLConnectionHandler() + sql = ("SELECT analysis_id FROM qiita.{0} WHERE " + "{0}_analysis_id = %s".format(cls._table)) + # MAGIC NUMBER 6: status id for a public study + return [x[0] for x in conn_handler.execute_fetchall(sql, (6,))] + @classmethod def create(cls, owner, name, description, parent=None): """Creates a new analysis on the database diff --git a/qiita_db/study.py b/qiita_db/study.py index be3282e1c..dce536bd5 100644 --- a/qiita_db/study.py +++ b/qiita_db/study.py @@ -149,7 +149,7 @@ def _status_setter_checks(self, conn_handler): @classmethod def get_public(cls): - """Returns study for all public Studies + """Returns study id for all public Studies Returns ------- @@ -160,8 +160,7 @@ def get_public(cls): sql = ("SELECT study_id FROM qiita.{0} WHERE " "{0}_status_id = %s".format(cls._table)) # MAGIC NUMBER 2: status id for a public study - return [cls(x[0]) for x in - conn_handler.execute_fetchall(sql, (2,))] + return [x[0] for x in conn_handler.execute_fetchall(sql, (2, ))] @classmethod def create(cls, owner, title, efo, info, investigation=None): diff --git a/qiita_pet/handlers/analysis_handlers.py b/qiita_pet/handlers/analysis_handlers.py index 7f450ec4c..1cf33707f 100644 --- a/qiita_pet/handlers/analysis_handlers.py +++ b/qiita_pet/handlers/analysis_handlers.py @@ -26,7 +26,26 @@ from qiita_db.metadata_template import SampleTemplate from qiita_db.job import Job from qiita_db.util import get_db_files_base_dir -# login code modified from https://gist.github.com/guillaumevincent/4771570 + + +def check_access(user, analysis_id): + """Checks whether user has access to an analysis + + Parameters + ---------- + user : User object + User to check + analysis_id : int + Analysis to check access for + + Raises + ------ + RuntimeError + Tried to access analysis that user does not have access to + """ + if analysis_id not in set(Analysis.get_public() + user.shared_analyses + + user.private_analyses): + raise RuntimeError("Analysis access denied to %s" % (analysis_id)) class CreateAnalysisHandler(BaseHandler): @@ -43,11 +62,10 @@ def post(self): name = self.get_argument('name') description = self.get_argument('description') user = self.get_current_user() - # create list of studies - study_ids = {s.id for s in Study.get_public()} userobj = User(user) - [study_ids.add(x) for x in userobj.private_studies] - [study_ids.add(x) for x in userobj.shared_studies] + # create list of studies + study_ids = set(Study.get_public() + userobj.private_studies + + userobj.shared_studies) studies = [Study(i) for i in study_ids] analysis = Analysis.create(User(user), name, description) @@ -101,22 +119,28 @@ def post(self): class AnalysisWaitHandler(BaseHandler): @authenticated def get(self, analysis_id): + user = self.get_current_user() + check_access(User(user), analysis_id) + analysis = Analysis(analysis_id) commands = [] for job in analysis.jobs: jobject = Job(job) commands.append("%s:%s" % (jobject.datatype, jobject.command[0])) - self.render("analysis_waiting.html", user=self.get_current_user(), + self.render("analysis_waiting.html", user=user, aid=analysis_id, aname=analysis.name, commands=commands) @authenticated @asynchronous - def post(self, analysis_id): + def post(self, aid): + user = self.get_current_user() + check_access(User(user), aid) + command_args = self.get_arguments("commands") split = [x.split("#") for x in command_args] - analysis = Analysis(analysis_id) + analysis = Analysis(aid) commands = [] # HARD CODED HACKY THING FOR DEMO, FIX Issue #164 @@ -141,9 +165,8 @@ def post(self, analysis_id): Job.create(data_type, command, opts, analysis) commands.append("%s: %s" % (data_type, command)) user = self.get_current_user() - self.render("analysis_waiting.html", user=user, - aid=analysis_id, aname=analysis.name, - commands=commands) + self.render("analysis_waiting.html", user=user, aid=aid, + aname=analysis.name, commands=commands) # fire off analysis run here # currently synch run so redirect done here. Will remove after demo run_analysis(user, analysis) @@ -152,6 +175,9 @@ def post(self, analysis_id): class AnalysisResultsHandler(BaseHandler): @authenticated def get(self, aid): + user = self.get_current_user() + check_access(User(user), aid) + analysis = Analysis(aid) jobres = defaultdict(list) for job in analysis.jobs: From f93b97cba14b78c3224b56e992de4c901f613f7d Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Tue, 1 Jul 2014 16:59:04 -0600 Subject: [PATCH 2/8] fix test to reflect changes --- qiita_db/test/test_study.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiita_db/test/test_study.py b/qiita_db/test/test_study.py index 0f5a3f4aa..487a7147d 100644 --- a/qiita_db/test/test_study.py +++ b/qiita_db/test/test_study.py @@ -149,7 +149,7 @@ def test_get_public(self): new = Study.create(User('test@foo.bar'), 'Identification of the ' 'Microbiomes for Cannabis Soils', [1], self.info) obs = Study.get_public() - self.assertEqual(obs, [Study(1)]) + self.assertEqual(obs, [1]) def test_create_study_min_data(self): """Insert a study into the database""" From 1831d6d0add8ebb5102a483b8faccc7dee266899 Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Tue, 1 Jul 2014 17:01:42 -0600 Subject: [PATCH 3/8] remove set creation. Closes #147 --- qiita_pet/handlers/analysis_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiita_pet/handlers/analysis_handlers.py b/qiita_pet/handlers/analysis_handlers.py index 1cf33707f..50f7a04dc 100644 --- a/qiita_pet/handlers/analysis_handlers.py +++ b/qiita_pet/handlers/analysis_handlers.py @@ -64,8 +64,8 @@ def post(self): user = self.get_current_user() userobj = User(user) # create list of studies - study_ids = set(Study.get_public() + userobj.private_studies + - userobj.shared_studies) + study_ids = Study.get_public() + userobj.private_studies + \ + userobj.shared_studies studies = [Study(i) for i in study_ids] analysis = Analysis.create(User(user), name, description) From 6817cd4f3bb9f1df8674107d00c73f82eed60abe Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Tue, 1 Jul 2014 17:04:44 -0600 Subject: [PATCH 4/8] rename check_access to check_analysis_access --- qiita_pet/handlers/analysis_handlers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qiita_pet/handlers/analysis_handlers.py b/qiita_pet/handlers/analysis_handlers.py index 50f7a04dc..2bb8627f9 100644 --- a/qiita_pet/handlers/analysis_handlers.py +++ b/qiita_pet/handlers/analysis_handlers.py @@ -28,7 +28,7 @@ from qiita_db.util import get_db_files_base_dir -def check_access(user, analysis_id): +def check_analysis_access(user, analysis_id): """Checks whether user has access to an analysis Parameters @@ -120,7 +120,7 @@ class AnalysisWaitHandler(BaseHandler): @authenticated def get(self, analysis_id): user = self.get_current_user() - check_access(User(user), analysis_id) + check_analysis_access(User(user), analysis_id) analysis = Analysis(analysis_id) commands = [] @@ -136,7 +136,7 @@ def get(self, analysis_id): @asynchronous def post(self, aid): user = self.get_current_user() - check_access(User(user), aid) + check_analysis_access(User(user), aid) command_args = self.get_arguments("commands") split = [x.split("#") for x in command_args] @@ -176,7 +176,7 @@ class AnalysisResultsHandler(BaseHandler): @authenticated def get(self, aid): user = self.get_current_user() - check_access(User(user), aid) + check_analysis_access(User(user), aid) analysis = Analysis(aid) jobres = defaultdict(list) From e1dca81832b7f98272b4096952e43ea74509ddb8 Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Tue, 1 Jul 2014 19:13:37 -0600 Subject: [PATCH 5/8] remove test.txt --- test.txt | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 test.txt diff --git a/test.txt b/test.txt deleted file mode 100644 index 5e7f172ac..000000000 --- a/test.txt +++ /dev/null @@ -1,4 +0,0 @@ -#SampleID center_name center_project_name data_type_id ebi_study_accession ebi_submission_accession emp_status_id str_column -SKB7.640196 ANL Test Project 2 None None 1 Value for sample 3 -SKB8.640193 ANL Test Project 2 None None 1 Value for sample 1 -SKD8.640184 ANL Test Project 2 None None 1 Value for sample 2 From cf5ffe317f72fe193faa5843938da64d8c4f892e Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Wed, 2 Jul 2014 08:22:58 -0600 Subject: [PATCH 6/8] add test for analysis get_public --- qiita_db/test/test_analysis.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/qiita_db/test/test_analysis.py b/qiita_db/test/test_analysis.py index bda1911ee..e76385732 100644 --- a/qiita_db/test/test_analysis.py +++ b/qiita_db/test/test_analysis.py @@ -31,6 +31,11 @@ def test_lock_check_ok(self): self.analysis.status = "queued" self.analysis._lock_check(self.conn_handler) + def test_get_public(self): + self.assertEqual(Analysis.get_public(), []) + self.analysis.status = "public" + self.assertEqual(Analysis.get_public(), [1]) + def test_create(self): new = Analysis.create(User("admin@foo.bar"), "newAnalysis", "A New Analysis") From 01a862229bf2c5c64b17fac1bc66946b58eeb570 Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Wed, 2 Jul 2014 08:32:24 -0600 Subject: [PATCH 7/8] fix SQL call --- qiita_db/analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiita_db/analysis.py b/qiita_db/analysis.py index cbb0a33ff..1f76610b6 100644 --- a/qiita_db/analysis.py +++ b/qiita_db/analysis.py @@ -75,7 +75,7 @@ def get_public(cls): """ conn_handler = SQLConnectionHandler() sql = ("SELECT analysis_id FROM qiita.{0} WHERE " - "{0}_analysis_id = %s".format(cls._table)) + "{0}_status_id = %s".format(cls._table)) # MAGIC NUMBER 6: status id for a public study return [x[0] for x in conn_handler.execute_fetchall(sql, (6,))] From 0b95cbeefb7b896caf5b1725e96301a40dfc7fe2 Mon Sep 17 00:00:00 2001 From: Joshua Shorenstein Date: Wed, 2 Jul 2014 13:11:44 -0600 Subject: [PATCH 8/8] remove set creation fir access check --- qiita_pet/handlers/analysis_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiita_pet/handlers/analysis_handlers.py b/qiita_pet/handlers/analysis_handlers.py index 2bb8627f9..64757ffc1 100644 --- a/qiita_pet/handlers/analysis_handlers.py +++ b/qiita_pet/handlers/analysis_handlers.py @@ -43,8 +43,8 @@ def check_analysis_access(user, analysis_id): RuntimeError Tried to access analysis that user does not have access to """ - if analysis_id not in set(Analysis.get_public() + user.shared_analyses + - user.private_analyses): + if analysis_id not in Analysis.get_public() + user.shared_analyses + \ + user.private_analyses: raise RuntimeError("Analysis access denied to %s" % (analysis_id))