From 9ef17f693ea110e4b9c60762aba5346e78a9c550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 17 Apr 2019 15:57:53 +0200 Subject: [PATCH 01/22] Update db.py with new columns for access. --- backend/db.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/db.py b/backend/db.py index 1ea48aa4c..b76751004 100644 --- a/backend/db.py +++ b/backend/db.py @@ -213,6 +213,9 @@ class Meta: data_contact_link = CharField(null=True) num_variants = IntegerField(null=True) coverage_levels = ArrayField(IntegerField, null=True) + portal_avail = BooleanField(null=True) + file_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Private']) + beacon_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Private']) class DatasetFile(BaseModel): From ce9294df9ceaab52154aa59bb3cce7f1946d5925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 09:52:26 +0200 Subject: [PATCH 02/22] add file_access check to has_access() --- backend/db.py | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/backend/db.py b/backend/db.py index b76751004..3f55dbd66 100644 --- a/backend/db.py +++ b/backend/db.py @@ -214,8 +214,8 @@ class Meta: num_variants = IntegerField(null=True) coverage_levels = ArrayField(IntegerField, null=True) portal_avail = BooleanField(null=True) - file_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Private']) - beacon_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Private']) + file_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Public']) + beacon_access = EnumField(null=False, choices=['None', 'Controlled', 'Registered', 'Public']) class DatasetFile(BaseModel): @@ -337,16 +337,35 @@ def is_admin(self, dataset): DatasetAccess.is_admin ).count() - def has_access(self, dataset): - return DatasetAccessCurrent.select().where( - DatasetAccessCurrent.dataset == dataset, - DatasetAccessCurrent.user == self, - ).count() + def has_access(self, dataset, ds_version=None): + """ + Check whether user has permission to access a dataset + + Args: + dataset (str): short name of dataset + ds_version (str): the dataset version + + Returns: + bool: allowed to access + + """ + dsv = get_dataset_version(dataset, ds_version) + if not dsv: + return False + if dsv.file_access in ('Registered', 'Public'): + return True + elif dsv.file_access == 'None': + return False + + return (DatasetAccessCurrent.select() + .where(DatasetAccessCurrent.dataset == dataset, + DatasetAccessCurrent.user == self) + .count()) > 0 def has_requested_access(self, dataset): return DatasetAccessPending.select().where( DatasetAccessPending.dataset == dataset, - DatasetAccessPending.user == self + DatasetAccessPending.user == self ).count() From 6f02a5df697daff0ea91ef3526c2977cd4e4d6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 10:58:26 +0200 Subject: [PATCH 03/22] change file_access in load_dummy_data to Controlled to maintain earlier behaviour --- test/data/load_dummy_data.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/data/load_dummy_data.sql b/test/data/load_dummy_data.sql index 3751c7ffa..4bcf9e522 100644 --- a/test/data/load_dummy_data.sql +++ b/test/data/load_dummy_data.sql @@ -25,10 +25,10 @@ INSERT INTO data.sample_sets (id, dataset, "collection", sample_size, phenotype) (1000003, 1000002, 1000003, 20, 'SamplePheno2 Coll2'); INSERT INTO data.dataset_versions (id, dataset, reference_set, dataset_version, dataset_description, terms, var_call_ref, available_from, ref_doi, data_contact_name, data_contact_link, num_variants, coverage_levels, portal_avail, file_access, beacon_access) - VALUES (1000001, 1000001, 1000001, 'Version 1-1', 'Dataset 1-1, description', 'Dataset 1-1, terms', 'CallRef11', '2017-01-01', 'datset11DOI', 'Gunnar Green', 'gunnar.green@example.com', 10, ARRAY[1,5,10], TRUE, 'Registered', 'Public'), - (1000002, 1000002, 1000001, 'Version 2-1', 'Dataset 2-1, description', 'Dataset 2-1, terms', 'CallRef21', '2017-02-01', 'datset21DOI', NULL, NULL, 100, ARRAY[1,5,10], TRUE, 'Registered', 'Public'), - (1000003, 1000002, 1000002, 'Version 2-2', 'Dataset 2-2, description', 'Dataset 2-2, terms', 'CallRef22', '2017-02-02', 'datset22DOI', 'Strummer project', 'https://example.com/strummer', 1000, ARRAY[1,5,10], TRUE, 'Registered', 'Public'), - (1000004, 1000002, 1000002, 'InvVer 2-3', 'Dataset 2-3, description', 'Dataset 2-3, terms', 'CallRef23', '2030-02-03', 'datset23DOI', 'Drummer project', 'https://example.com/drummer', 10000, ARRAY[1,5,10], TRUE, 'Registered', 'Public'); + VALUES (1000001, 1000001, 1000001, 'Version 1-1', 'Dataset 1-1, description', 'Dataset 1-1, terms', 'CallRef11', '2017-01-01', 'datset11DOI', 'Gunnar Green', 'gunnar.green@example.com', 10, ARRAY[1,5,10], TRUE, 'Controlled', 'Public'), + (1000002, 1000002, 1000001, 'Version 2-1', 'Dataset 2-1, description', 'Dataset 2-1, terms', 'CallRef21', '2017-02-01', 'datset21DOI', NULL, NULL, 100, ARRAY[1,5,10], TRUE, 'Controlled', 'Public'), + (1000003, 1000002, 1000002, 'Version 2-2', 'Dataset 2-2, description', 'Dataset 2-2, terms', 'CallRef22', '2017-02-02', 'datset22DOI', 'Strummer project', 'https://example.com/strummer', 1000, ARRAY[1,5,10], TRUE, 'Controlled', 'Public'), + (1000004, 1000002, 1000002, 'InvVer 2-3', 'Dataset 2-3, description', 'Dataset 2-3, terms', 'CallRef23', '2030-02-03', 'datset23DOI', 'Drummer project', 'https://example.com/drummer', 10000, ARRAY[1,5,10], TRUE, 'Controlled', 'Public'); INSERT INTO data.dataset_files(id, dataset_version, basename, uri, file_size) VALUES (1000001, 1000001, 'File11-1', '/release/file111.txt', 100), From 4f2c6e2129537344e08053464c90f7f33b9230b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 10:59:09 +0200 Subject: [PATCH 04/22] dataset is a Dataset object, not a string --- backend/db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/db.py b/backend/db.py index 3f55dbd66..585810358 100644 --- a/backend/db.py +++ b/backend/db.py @@ -342,14 +342,14 @@ def has_access(self, dataset, ds_version=None): Check whether user has permission to access a dataset Args: - dataset (str): short name of dataset + dataset (Dataset): peewee Dataset object ds_version (str): the dataset version Returns: bool: allowed to access """ - dsv = get_dataset_version(dataset, ds_version) + dsv = get_dataset_version(dataset.short_name, ds_version) if not dsv: return False if dsv.file_access in ('Registered', 'Public'): From f7f4cceda2cd398bb4cfba181e6fbc39112efbc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:25:09 +0200 Subject: [PATCH 05/22] rename version to ds_version --- backend/application.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/backend/application.py b/backend/application.py index 8a3b7ca92..15512fd74 100644 --- a/backend/application.py +++ b/backend/application.py @@ -218,10 +218,10 @@ def get(self, dataset): class GenerateTemporaryLink(handlers.AuthorizedHandler): - def post(self, dataset, version=None): - dataset, version = utils.parse_dataset(dataset, version) + def post(self, dataset, ds_version=None): + dataset, ds_version = utils.parse_dataset(dataset, ds_version) user = self.current_user - dataset_version = db.get_dataset_version(dataset, version) + dataset_version = db.get_dataset_version(dataset, ds_version) if dataset_version is None: self.send_error(status_code=404) return @@ -248,9 +248,9 @@ def post(self, dataset, version=None): class DatasetFiles(handlers.AuthorizedHandler): - def get(self, dataset, version=None): - dataset, version = utils.parse_dataset(dataset, version) - dataset_version = db.get_dataset_version(dataset, version) + def get(self, dataset, ds_version=None): + dataset, ds_version = utils.parse_dataset(dataset, ds_version) + dataset_version = db.get_dataset_version(dataset, ds_version) if dataset_version is None: self.send_error(status_code=404) return @@ -264,6 +264,7 @@ def get(self, dataset, version=None): self.finish({'files': ret}) + def format_bytes(nbytes): postfixes = ['b', 'Kb', 'Mb', 'Gb', 'Tb', 'Pb', 'Eb'] exponent = math.floor( math.log(nbytes) / math.log(1000) ) From bf96126ba2555c41b13f4b7f0c77f528bddb655f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:25:31 +0200 Subject: [PATCH 06/22] add support for version --- backend/route.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/route.py b/backend/route.py index 4b5179f12..36c351c4d 100644 --- a/backend/route.py +++ b/backend/route.py @@ -45,7 +45,7 @@ def __init__(self, settings): {"path": "static/img/"}), (r"/release/(?P[^\/]+)/(?P[^\/]+)/(?P[^\/]+)",handlers.TemporaryStaticNginxFileHandler, {"path": "/release-files/"}), - (r"/release/(?P[^\/]+)/(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, + (r"/release/(?P[^\/]+)/(?:version/(?P[^/]+)/)?(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, {"path": "/release-files/"}), ## Authentication (r"/logout", auth.ElixirLogoutHandler), @@ -65,11 +65,11 @@ def __init__(self, settings): (r"/api/dataset/(?P[^\/]+)", application.GetDataset), (r"/api/dataset/(?P[^\/]+)/log/(?P[^\/]+)/(?P[^\/]+)", application.LogEvent), (r"/api/dataset/(?P[^\/]+)/logo", application.ServeLogo), - (r"/api/dataset/(?P[^\/]+)/files", application.DatasetFiles), + (r"/api/dataset/(?P[^\/]+)/(?:version/(?P[^/]+)/)?files", application.DatasetFiles), (r"/api/dataset/(?P[^\/]+)/collection", application.Collection), (r"/api/dataset/(?P[^\/]+)/users_current", application.DatasetUsersCurrent), (r"/api/dataset/(?P[^\/]+)/users_pending", application.DatasetUsersPending), - (r"/api/dataset/(?P[^\/]+)/temporary_link", application.GenerateTemporaryLink), + (r"/api/dataset/(?P[^\/]+)/(?:version/(?P[^/]+)/)?temporary_link", application.GenerateTemporaryLink), (r"/api/dataset/(?P[^\/]+)/users/[^\/]+/request", application.RequestAccess), (r"/api/dataset/(?P[^\/]+)/users/(?P[^\/]+)/approve", application.ApproveUser), (r"/api/dataset/(?P[^\/]+)/users/(?P[^\/]+)/revoke", application.RevokeUser), From cc2eeedc532341022b589db7953122c005e3f4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:40:47 +0200 Subject: [PATCH 07/22] version is called versions for the base routes; collection needs version support --- backend/route.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/route.py b/backend/route.py index 36c351c4d..e691d1b04 100644 --- a/backend/route.py +++ b/backend/route.py @@ -45,7 +45,7 @@ def __init__(self, settings): {"path": "static/img/"}), (r"/release/(?P[^\/]+)/(?P[^\/]+)/(?P[^\/]+)",handlers.TemporaryStaticNginxFileHandler, {"path": "/release-files/"}), - (r"/release/(?P[^\/]+)/(?:version/(?P[^/]+)/)?(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, + (r"/release/(?P[^\/]+)/(?:versions/(?P[^/]+)/)?(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, {"path": "/release-files/"}), ## Authentication (r"/logout", auth.ElixirLogoutHandler), @@ -65,11 +65,11 @@ def __init__(self, settings): (r"/api/dataset/(?P[^\/]+)", application.GetDataset), (r"/api/dataset/(?P[^\/]+)/log/(?P[^\/]+)/(?P[^\/]+)", application.LogEvent), (r"/api/dataset/(?P[^\/]+)/logo", application.ServeLogo), - (r"/api/dataset/(?P[^\/]+)/(?:version/(?P[^/]+)/)?files", application.DatasetFiles), - (r"/api/dataset/(?P[^\/]+)/collection", application.Collection), + (r"/api/dataset/(?P[^\/]+)/(?:versions/(?P[^/]+)/)?files", application.DatasetFiles), + (r"/api/dataset/(?P[^\/]+)/(?:versions/(?P[^/]+)/)?collection", application.Collection), (r"/api/dataset/(?P[^\/]+)/users_current", application.DatasetUsersCurrent), (r"/api/dataset/(?P[^\/]+)/users_pending", application.DatasetUsersPending), - (r"/api/dataset/(?P[^\/]+)/(?:version/(?P[^/]+)/)?temporary_link", application.GenerateTemporaryLink), + (r"/api/dataset/(?P[^\/]+)/(?:versions/(?P[^/]+)/)?temporary_link", application.GenerateTemporaryLink), (r"/api/dataset/(?P[^\/]+)/users/[^\/]+/request", application.RequestAccess), (r"/api/dataset/(?P[^\/]+)/users/(?P[^\/]+)/approve", application.ApproveUser), (r"/api/dataset/(?P[^\/]+)/users/(?P[^\/]+)/revoke", application.RevokeUser), From 5a0fb9ee22d722299769e84774094c2d7d40932c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:55:25 +0200 Subject: [PATCH 08/22] collection needs version --- backend/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/application.py b/backend/application.py index 15512fd74..8af88bd40 100644 --- a/backend/application.py +++ b/backend/application.py @@ -272,7 +272,7 @@ def format_bytes(nbytes): class Collection(handlers.UnsafeHandler): - def get(self, dataset): + def get(self, dataset, ds_version=None): dataset, _ = utils.parse_dataset(dataset) dataset = db.get_dataset(dataset) From 9386779ac4128bd3108891fec15b271e89387710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:55:51 +0200 Subject: [PATCH 09/22] support version check for authhandler --- backend/handlers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/handlers.py b/backend/handlers.py index 4321081b1..6d25a4f9d 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -130,10 +130,11 @@ def prepare(self): return kwargs = self.path_kwargs - if not kwargs['dataset']: + if not 'dataset' in kwargs: logging.debug("No dataset: Send error 403") self.send_error(status_code=403) - if not self.current_user.has_access( db.get_dataset(kwargs['dataset']) ): + ds_version = kwargs['ds_version'] if ds_version in kwargs else None + if not self.current_user.has_access(kwargs['dataset'], ds_version): logging.debug("No user access: Send error 403") self.send_error(status_code=403) logging.debug("User is authorized") From efd5c8b2f65c3b9c46007d760880dba43455da8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 13:59:07 +0200 Subject: [PATCH 10/22] not a variable name --- backend/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/handlers.py b/backend/handlers.py index 6d25a4f9d..35667dd42 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -133,7 +133,7 @@ def prepare(self): if not 'dataset' in kwargs: logging.debug("No dataset: Send error 403") self.send_error(status_code=403) - ds_version = kwargs['ds_version'] if ds_version in kwargs else None + ds_version = kwargs['ds_version'] if 'ds_version' in kwargs else None if not self.current_user.has_access(kwargs['dataset'], ds_version): logging.debug("No user access: Send error 403") self.send_error(status_code=403) From 8c79abc10dbcd136c50d0da3ed794e4a63f12ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 14:03:07 +0200 Subject: [PATCH 11/22] better to receive string then Database object --- backend/db.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/db.py b/backend/db.py index 585810358..bd1f36710 100644 --- a/backend/db.py +++ b/backend/db.py @@ -159,6 +159,7 @@ class Meta: class Dataset(BaseModel): """ A dataset is part of a study, and usually include a certain population. + Most studies only have a single dataset, but multiple are allowed. """ class Meta: @@ -342,14 +343,14 @@ def has_access(self, dataset, ds_version=None): Check whether user has permission to access a dataset Args: - dataset (Dataset): peewee Dataset object + dataset (str): dataset short name ds_version (str): the dataset version Returns: bool: allowed to access """ - dsv = get_dataset_version(dataset.short_name, ds_version) + dsv = get_dataset_version(dataset, ds_version) if not dsv: return False if dsv.file_access in ('Registered', 'Public'): @@ -357,6 +358,7 @@ def has_access(self, dataset, ds_version=None): elif dsv.file_access == 'None': return False + dataset = get_dataset(dataset) return (DatasetAccessCurrent.select() .where(DatasetAccessCurrent.dataset == dataset, DatasetAccessCurrent.user == self) From 4c846320471559ef92546b917a53ccdc1df84d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 14:28:51 +0200 Subject: [PATCH 12/22] change back from string to database --- backend/db.py | 5 ++--- backend/handlers.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/backend/db.py b/backend/db.py index bd1f36710..1049e0b89 100644 --- a/backend/db.py +++ b/backend/db.py @@ -343,14 +343,14 @@ def has_access(self, dataset, ds_version=None): Check whether user has permission to access a dataset Args: - dataset (str): dataset short name + dataset (Database): peewee Database object ds_version (str): the dataset version Returns: bool: allowed to access """ - dsv = get_dataset_version(dataset, ds_version) + dsv = get_dataset_version(dataset.short_name, ds_version) if not dsv: return False if dsv.file_access in ('Registered', 'Public'): @@ -358,7 +358,6 @@ def has_access(self, dataset, ds_version=None): elif dsv.file_access == 'None': return False - dataset = get_dataset(dataset) return (DatasetAccessCurrent.select() .where(DatasetAccessCurrent.dataset == dataset, DatasetAccessCurrent.user == self) diff --git a/backend/handlers.py b/backend/handlers.py index 35667dd42..71e14514a 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -134,7 +134,7 @@ def prepare(self): logging.debug("No dataset: Send error 403") self.send_error(status_code=403) ds_version = kwargs['ds_version'] if 'ds_version' in kwargs else None - if not self.current_user.has_access(kwargs['dataset'], ds_version): + if not self.current_user.has_access(db.get_dataset(kwargs['dataset']), ds_version): logging.debug("No user access: Send error 403") self.send_error(status_code=403) logging.debug("User is authorized") From 599f62e8db14d0168d2d88eea76fd615f6e9dd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 20:42:00 +0200 Subject: [PATCH 13/22] seems to fix dl permissions --- backend/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/handlers.py b/backend/handlers.py index 71e14514a..358bd8080 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -180,7 +180,7 @@ def initialize(self, path): path = "/" + path self.root = path - def get(self, dataset, file, user=None): + def get(self, dataset, file, ds_version=None, user=None): logging.debug("Want to download dataset {} ({})".format(dataset, file)) if not user: From 5e9beacab2c73dee4652fa023db811e2b48c50a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 21:08:31 +0200 Subject: [PATCH 14/22] include version in dataset has_access check --- backend/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/application.py b/backend/application.py index 8af88bd40..ed6a1eb90 100644 --- a/backend/application.py +++ b/backend/application.py @@ -35,7 +35,7 @@ def build_dataset_structure(dataset_version, user=None, dataset=None): r['is_admin'] = user.is_admin(dataset) if user.has_access(dataset): r['authorization_level'] = 'has_access' - elif user.has_requested_access(dataset): + elif user.has_requested_access(dataset, dataset_version.version): r['authorization_level'] = 'has_requested_access' else: r['authorization_level'] = 'no_access' From edb38bdb710d3180709a9e896d64040130e26560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Thu, 18 Apr 2019 21:08:45 +0200 Subject: [PATCH 15/22] return after send_error --- backend/handlers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/handlers.py b/backend/handlers.py index 358bd8080..530be8c09 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -133,10 +133,12 @@ def prepare(self): if not 'dataset' in kwargs: logging.debug("No dataset: Send error 403") self.send_error(status_code=403) + return ds_version = kwargs['ds_version'] if 'ds_version' in kwargs else None if not self.current_user.has_access(db.get_dataset(kwargs['dataset']), ds_version): logging.debug("No user access: Send error 403") self.send_error(status_code=403) + return logging.debug("User is authorized") From ff7e108a06c3df145d99775b54f967887325441e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Tue, 23 Apr 2019 14:48:44 +0200 Subject: [PATCH 16/22] add version check to file handler --- backend/handlers.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/backend/handlers.py b/backend/handlers.py index 530be8c09..108f88397 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -188,14 +188,13 @@ def get(self, dataset, file, ds_version=None, user=None): if not user: user = self.current_user - dbfile = (db.DatasetFile - .select() - .where(db.DatasetFile.name == file) + dbfile = (db.DatasetFile.select() + .join(db.DatasetVersion) + .where((db.DatasetFile.name == file) & + (db.DatasetVersion.version == ds_version)) .get()) - db.UserDownloadLog.create( - user = user, - dataset_file = dbfile - ) + + db.UserDownloadLog.create(user = user, dataset_file = dbfile) abspath = os.path.abspath(os.path.join(self.root, file)) self.set_header("X-Accel-Redirect", abspath) From 65aad54e1364bc9c95fb9e477480c0010f3b1f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Tue, 23 Apr 2019 14:53:43 +0200 Subject: [PATCH 17/22] Error handling for missing file. Also a few missing returns after send_error. --- backend/handlers.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/backend/handlers.py b/backend/handlers.py index 108f88397..2012a334a 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -153,9 +153,11 @@ def prepare(self): if not kwargs['dataset']: logging.debug("No dataset: Send error 403") self.send_error(status_code=403) + return if not self.current_user.is_admin( db.get_dataset(kwargs['dataset']) ): logging.debug("No user admin: Send error 403") self.send_error(status_code=403) + return class SafeStaticFileHandler(tornado.web.StaticFileHandler, SafeHandler): @@ -188,11 +190,15 @@ def get(self, dataset, file, ds_version=None, user=None): if not user: user = self.current_user - dbfile = (db.DatasetFile.select() - .join(db.DatasetVersion) - .where((db.DatasetFile.name == file) & - (db.DatasetVersion.version == ds_version)) - .get()) + try: + dbfile = (db.DatasetFile.select() + .join(db.DatasetVersion) + .where((db.DatasetFile.name == file) & + (db.DatasetVersion.version == ds_version)) + .get()) + except db.DatasetFile.DoesNotExist: + send_error(status_code=403) + return db.UserDownloadLog.create(user = user, dataset_file = dbfile) From fc805d753afceb61afb3217f15d6f16a13d41c11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Tue, 23 Apr 2019 14:56:00 +0200 Subject: [PATCH 18/22] send_error should be self.send_error --- backend/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/handlers.py b/backend/handlers.py index 2012a334a..2ff0e641a 100644 --- a/backend/handlers.py +++ b/backend/handlers.py @@ -197,7 +197,7 @@ def get(self, dataset, file, ds_version=None, user=None): (db.DatasetVersion.version == ds_version)) .get()) except db.DatasetFile.DoesNotExist: - send_error(status_code=403) + self.send_error(status_code=403) return db.UserDownloadLog.create(user = user, dataset_file = dbfile) From a180c46814f91d44bd32b26828a11b088fc6674c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Tue, 23 Apr 2019 15:07:06 +0200 Subject: [PATCH 19/22] dataset_version check should be made by has_access, not has_requested_access --- backend/application.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/application.py b/backend/application.py index ed6a1eb90..600d57421 100644 --- a/backend/application.py +++ b/backend/application.py @@ -33,9 +33,9 @@ def build_dataset_structure(dataset_version, user=None, dataset=None): if user: r['is_admin'] = user.is_admin(dataset) - if user.has_access(dataset): + if user.has_access(dataset, dataset_version.version): r['authorization_level'] = 'has_access' - elif user.has_requested_access(dataset, dataset_version.version): + elif user.has_requested_access(dataset): r['authorization_level'] = 'has_requested_access' else: r['authorization_level'] = 'no_access' From 4d4fbbf70e67ecae69f242ed99165a032f0db2a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Tue, 23 Apr 2019 15:22:41 +0200 Subject: [PATCH 20/22] sort returned versions by name (in reverse) --- backend/application.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/backend/application.py b/backend/application.py index 600d57421..677deb291 100644 --- a/backend/application.py +++ b/backend/application.py @@ -183,15 +183,13 @@ def get(self, dataset): user = self.current_user dataset = db.get_dataset(dataset) - versions = db.DatasetVersion.select( - db.DatasetVersion.version, db.DatasetVersion.available_from - ).where( - db.DatasetVersion.dataset == dataset - ) + versions = [ver for ver in (db.DatasetVersion.select(db.DatasetVersion.version, + db.DatasetVersion.available_from) + .where(db.DatasetVersion.dataset == dataset))] logging.info("ListDatasetVersions") - data = [] found_current = False + versions = sorted(versions, key=lambda version: version.version) for v in reversed(versions): current = False future = False From ca1b80b4bc12242c273738aa2085bf3e65072c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 24 Apr 2019 14:09:40 +0200 Subject: [PATCH 21/22] list comprehension not needed --- backend/application.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/application.py b/backend/application.py index 677deb291..ac5f8989f 100644 --- a/backend/application.py +++ b/backend/application.py @@ -183,9 +183,9 @@ def get(self, dataset): user = self.current_user dataset = db.get_dataset(dataset) - versions = [ver for ver in (db.DatasetVersion.select(db.DatasetVersion.version, - db.DatasetVersion.available_from) - .where(db.DatasetVersion.dataset == dataset))] + versions = (db.DatasetVersion.select(db.DatasetVersion.version, + db.DatasetVersion.available_from) + .where(db.DatasetVersion.dataset == dataset)) logging.info("ListDatasetVersions") data = [] found_current = False From ab6950c8fa882c3d0898a58835f6b670a2a2afa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 24 Apr 2019 14:28:44 +0200 Subject: [PATCH 22/22] version is needed to function, so no point having it optional --- backend/route.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/route.py b/backend/route.py index e691d1b04..48e8d44dc 100644 --- a/backend/route.py +++ b/backend/route.py @@ -45,7 +45,7 @@ def __init__(self, settings): {"path": "static/img/"}), (r"/release/(?P[^\/]+)/(?P[^\/]+)/(?P[^\/]+)",handlers.TemporaryStaticNginxFileHandler, {"path": "/release-files/"}), - (r"/release/(?P[^\/]+)/(?:versions/(?P[^/]+)/)?(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, + (r"/release/(?P[^\/]+)/versions/(?P[^/]+)/(?P[^\/]+)", handlers.AuthorizedStaticNginxFileHandler, {"path": "/release-files/"}), ## Authentication (r"/logout", auth.ElixirLogoutHandler),