From 9362f9b5397fca1a1d287c3e4185a69fae74ba23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 10 May 2016 12:35:21 +0200 Subject: [PATCH] Remove links from returned file docs when user is not subscriber/demo/admin. For unauthenticated/non-subscriber users, image file documents retain their variations. All other documents have ther variations stripped. Also the links + expiry info to the original file are removed for all file types. --- pillar/application/modules/file_storage.py | 25 ++++- pillar/config.py | 3 + tests/common_test_class.py | 2 + tests/common_test_data.py | 13 ++- tests/test_file_storage.py | 107 +++++++++++++++++++++ 5 files changed, 144 insertions(+), 6 deletions(-) diff --git a/pillar/application/modules/file_storage.py b/pillar/application/modules/file_storage.py index a2d60c16..ffa23e75 100644 --- a/pillar/application/modules/file_storage.py +++ b/pillar/application/modules/file_storage.py @@ -298,6 +298,24 @@ def generate_link(backend, file_path, project_id=None, is_public=False): def before_returning_file(response): ensure_valid_link(response) + # Check the access level of the user. + if g.current_user is None: + has_full_access = False + else: + user_roles = g.current_user['roles'] + access_roles = current_app.config['FULL_FILE_ACCESS_ROLES'] + has_full_access = bool(user_roles.intersection(access_roles)) + + # Strip all file variations (unless image) and link to the actual file. + if not has_full_access: + response.pop('link', None) + response.pop('link_expires', None) + + # Image files have public variations, other files don't. + if not response.get('content_type', '').startswith('image/'): + if 'variations' in response and response['variations'] is not None: + response['variations'] = [] + def before_returning_files(response): for item in response['_items']: @@ -337,8 +355,10 @@ def _generate_all_links(response, now): response['project']) if 'project' in response else None # TODO: add project id to all files backend = response['backend'] response['link'] = generate_link(backend, response['file_path'], project_id) - if 'variations' in response: - for variation in response['variations']: + + variations = response.get('variations') + if variations: + for variation in variations: variation['link'] = generate_link(backend, variation['file_path'], project_id) # Construct the new expiry datetime. @@ -491,7 +511,6 @@ def override_content_type(uploaded_file): @file_storage.route('/stream/', methods=['POST', 'OPTIONS']) @require_login(require_roles={u'subscriber', u'admin', u'demo'}) def stream_to_gcs(project_id): - projects = current_app.data.driver.db['projects'] try: project = projects.find_one(ObjectId(project_id), projection={'_id': 1}) diff --git a/pillar/config.py b/pillar/config.py index 7556ed00..e8e65951 100644 --- a/pillar/config.py +++ b/pillar/config.py @@ -64,6 +64,9 @@ FILE_LINK_VALIDITY = defaultdict( gcs=3600 * 23, # 23 hours for Google Cloud Storage. ) +# Roles with full GET-access to all variations of files. +FULL_FILE_ACCESS_ROLES = {u'admin', u'subscriber', u'demo'} + # Client and Subclient IDs for Blender ID BLENDER_ID_CLIENT_ID = 'SPECIAL-SNOWFLAKE-57' BLENDER_ID_SUBCLIENT_ID = 'PILLAR' diff --git a/tests/common_test_class.py b/tests/common_test_class.py index 6d87542c..0317c08d 100644 --- a/tests/common_test_class.py +++ b/tests/common_test_class.py @@ -79,6 +79,8 @@ class AbstractPillarTest(TestMinimal): file = copy.deepcopy(EXAMPLE_FILE) if file_overrides is not None: file.update(file_overrides) + if '_id' in file and file['_id'] is None: + del file['_id'] result = files_collection.insert_one(file) file_id = result.inserted_id diff --git a/tests/common_test_data.py b/tests/common_test_data.py index b661a753..5e543736 100644 --- a/tests/common_test_data.py +++ b/tests/common_test_data.py @@ -16,11 +16,18 @@ EXAMPLE_FILE = {u'_id': ObjectId('5672e2c1c379cf0007b31995'), {u'format': 'jpg', u'height': 2048, u'width': 2048, u'length': 819569, u'link': 'http://localhost:8002/file-variant-h', u'content_type': 'image/jpeg', u'md5': '--', u'file_path': 'c2a5c897769ce1ef0eb10f8fa1c472bcb8e2d5a4-h.jpg', - u'size': 'h'}, ], + u'size': 'h'}, + {u'format': 'jpg', u'height': 64, u'width': 64, u'length': 8195, + u'link': 'http://localhost:8002/file-variant-t', u'content_type': 'image/jpeg', + u'md5': '--', u'file_path': 'c2a5c897769ce1ef0eb10f8fa1c472bcb8e2d5a4-t.jpg', + u'size': 't'}, + ], u'filename': 'brick_dutch_soft_bump.png', u'project': EXAMPLE_PROJECT_ID, - u'width': 2048, u'length': 6227670, u'user': ObjectId('56264fc4fa3a250344bd10c5'), - u'content_type': 'image/png', u'_etag': '044ce3aede2e123e261c0d8bd77212f264d4f7b0', + u'width': 2048, u'length': 6227670, + u'user': ObjectId('56264fc4fa3a250344bd10c5'), + u'content_type': 'image/png', + u'_etag': '044ce3aede2e123e261c0d8bd77212f264d4f7b0', u'_created': datetime.datetime(2015, 12, 17, 16, 28, 49, tzinfo=tz_util.utc), u'md5': '', u'file_path': 'c2a5c897769ce1ef0eb10f8fa1c472bcb8e2d5a4.png', diff --git a/tests/test_file_storage.py b/tests/test_file_storage.py index b1cdfb88..379efb8d 100644 --- a/tests/test_file_storage.py +++ b/tests/test_file_storage.py @@ -1,3 +1,5 @@ +import json + import os import tempfile @@ -65,3 +67,108 @@ class TempDirTest(AbstractPillarTest): dirname = os.path.dirname(tmpfile.name) self.assertEqual(dirname, storage) + +class FileAccessTest(AbstractPillarTest): + def test_link_stripping(self): + """Subscribers should get all links, but non-subscribers only a subset.""" + + img_file_id, _ = self.ensure_file_exists() + video_file_id, _ = self.ensure_file_exists({ + u'_id': None, + u'content_type': u'video/matroska', + 'variations': [ + { + 'format': 'mp4', + 'height': 446, + 'width': 1064, + 'length': 219399183, + 'link': 'https://hosting/filename.mp4', + 'content_type': 'video/mp4', + 'duration': 44, + 'size': '446p', + 'file_path': 'c1/c1f7b71c248c03468b2bb3e7c9f0c4e5cdb9d6d0.mp4', + 'md5': 'c1f7b71c248c03468b2bb3e7c9f0c4e5cdb9d6d0' + }, + { + 'format': 'webm', + 'height': 446, + 'width': 1064, + 'length': 31219520, + 'link': 'https://hosting/filename.webm', + 'content_type': 'video/webm', + 'duration': 44, + 'md5': 'c1f7b71c248c03468b2bb3e7c9f0c4e5cdb9d6d0', + 'file_path': 'c1/c1f7b71c248c03468b2bb3e7c9f0c4e5cdb9d6d0.webm', + 'size': '446p' + } + ] + + }) + blend_file_id, _ = self.ensure_file_exists({u'_id': None, + u'content_type': u'application/x-blend', + u'variations': None}) + + nonsub_user_id = self.create_user(user_id='cafef00dcafef00d00000000', roles=()) + sub_user_id = self.create_user(user_id='cafef00dcafef00dcafef00d', roles=(u'subscriber',)) + demo_user_id = self.create_user(user_id='cafef00dcafef00ddeadbeef', roles=(u'demo',)) + admin_user_id = self.create_user(user_id='aaaaaaaaaaaaaaaaaaaaaaaa', roles=(u'admin',)) + + self.create_valid_auth_token(nonsub_user_id, 'nonsub-token') + self.create_valid_auth_token(sub_user_id, 'sub-token') + self.create_valid_auth_token(demo_user_id, 'demo-token') + self.create_valid_auth_token(admin_user_id, 'admin-token') + + def assert_variations(file_id, has_access, token=None): + if token: + headers = {'Authorization': self.make_header(token)} + else: + headers = None + resp = self.client.get('/files/%s' % file_id, headers=headers) + self.assertEqual(200, resp.status_code) + file_info = json.loads(resp.data) + + self.assertEqual(has_access, 'link' in file_info) + self.assertEqual(has_access, 'link_expires' in file_info) + return file_info + + # Unauthenticated user and non-subscriber should still get the file, but limited. + file_info = assert_variations(img_file_id, False) + self.assertEqual({'t', 'h', 'b'}, {var['size'] for var in file_info['variations']}) + file_info = assert_variations(img_file_id, False, 'nonsub-token') + self.assertEqual({'t', 'h', 'b'}, {var['size'] for var in file_info['variations']}) + + # Authenticated subscribers, demos and admins should get the full file. + file_info = assert_variations(img_file_id, True, 'sub-token') + self.assertEqual({'t', 'h', 'b'}, {var['size'] for var in file_info['variations']}) + file_info = assert_variations(img_file_id, True, 'demo-token') + self.assertEqual({'t', 'h', 'b'}, {var['size'] for var in file_info['variations']}) + file_info = assert_variations(img_file_id, True, 'admin-token') + self.assertEqual({'t', 'h', 'b'}, {var['size'] for var in file_info['variations']}) + + # Unauthenticated user and non-subscriber should get no links what so ever. + file_info = assert_variations(video_file_id, False) + self.assertEqual([], file_info['variations']) + file_info = assert_variations(video_file_id, False, 'nonsub-token') + self.assertEqual([], file_info['variations']) + + # Authenticated subscribers, demos and admins should get the full file. + file_info = assert_variations(video_file_id, True, 'sub-token') + self.assertEqual({'mp4', 'webm'}, {var['format'] for var in file_info['variations']}) + file_info = assert_variations(video_file_id, True, 'demo-token') + self.assertEqual({'mp4', 'webm'}, {var['format'] for var in file_info['variations']}) + file_info = assert_variations(video_file_id, True, 'admin-token') + self.assertEqual({'mp4', 'webm'}, {var['format'] for var in file_info['variations']}) + + # Unauthenticated user and non-subscriber should get no links what so ever. + file_info = assert_variations(blend_file_id, False) + self.assertIsNone(file_info['variations']) + file_info = assert_variations(blend_file_id, False, 'nonsub-token') + self.assertIsNone(file_info['variations']) + + # Authenticated subscribers, demos and admins should get the full file. + file_info = assert_variations(blend_file_id, True, 'sub-token') + self.assertIsNone(file_info['variations']) + file_info = assert_variations(blend_file_id, True, 'demo-token') + self.assertIsNone(file_info['variations']) + file_info = assert_variations(blend_file_id, True, 'admin-token') + self.assertIsNone(file_info['variations'])