From 0429c4d515536f9cca6b70b2abeb019de807c955 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 11 Dec 2019 01:12:53 +0000 Subject: [PATCH] Fix importing a directory from user_library_import_dir Fix the following traceback: ``` Traceback (most recent call last): File "/galaxy/lib/galaxy/web/framework/decorators.py", line 282, in decorator rval = func(self, trans, *args, **kwargs) File "/galaxy/lib/galaxy/webapps/galaxy/api/library_contents.py", line 250, in create status, output = self._upload_library_dataset(trans, library_id, real_folder_id, **payload) File "/galaxy/lib/galaxy/webapps/galaxy/api/library_contents.py", line 337, in _upload_library_dataset **kwd) File "/galaxy/lib/galaxy/actions/library.py", line 91, in _upload_dataset full_dir, import_dir_desc = validate_server_directory_upload(trans, server_dir) File "/galaxy/lib/galaxy/actions/library.py", line 48, in validate_server_directory_upload if import_dir_desc == 'user_library_import_dir' and safe_contains(import_dir, full_dir, whitelist=trans.app.config.user_library_import_symlink_whitelist, username=username): TypeError: safe_contains() got an unexpected keyword argument 'username' ``` Also add a test case to reproduce the fixed issue, which was introduced in commit f4442ffc807285a96fa049f9a7e961b5e945bbff . Reported by @jhl667 . --- lib/galaxy/actions/library.py | 4 +-- .../test_upload_configuration_options.py | 32 ++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/actions/library.py b/lib/galaxy/actions/library.py index a9c5170819d..6d0723c4d23 100644 --- a/lib/galaxy/actions/library.py +++ b/lib/galaxy/actions/library.py @@ -45,8 +45,8 @@ def validate_server_directory_upload(trans, server_dir): unsafe = None if safe_relpath(server_dir): username = trans.user.username if trans.app.config.user_library_import_check_permissions else None - if import_dir_desc == 'user_library_import_dir' and safe_contains(import_dir, full_dir, whitelist=trans.app.config.user_library_import_symlink_whitelist, username=username): - for unsafe in unsafe_walk(full_dir, whitelist=[import_dir] + trans.app.config.user_library_import_symlink_whitelist): + if import_dir_desc == 'user_library_import_dir' and safe_contains(import_dir, full_dir, whitelist=trans.app.config.user_library_import_symlink_whitelist): + for unsafe in unsafe_walk(full_dir, whitelist=[import_dir] + trans.app.config.user_library_import_symlink_whitelist, username=username): log.error('User attempted to import a path that resolves to a path outside of their import dir: %s -> %s', unsafe, os.path.realpath(unsafe)) else: log.error('User attempted to import a directory path that resolves to a path outside of their import dir: %s -> %s', server_dir, os.path.realpath(full_dir)) diff --git a/test/integration/test_upload_configuration_options.py b/test/integration/test_upload_configuration_options.py index dd076d4e060..88058195693 100644 --- a/test/integration/test_upload_configuration_options.py +++ b/test/integration/test_upload_configuration_options.py @@ -705,7 +705,7 @@ class ServerDirectoryValidUsageTestCase(BaseUploadContentConfigurationTestCase): return cls.temp_config_dir("server") -class ServerDirectoryRestrictedToAdminsUsageTestCase(BaseUploadContentConfigurationTestCase): +class UserServerDirectoryOffByDefaultTestCase(BaseUploadContentConfigurationTestCase): @classmethod def handle_galaxy_config_kwds(cls, config): @@ -719,6 +719,36 @@ class ServerDirectoryRestrictedToAdminsUsageTestCase(BaseUploadContentConfigurat assert response.status_code == 403, response.json() +class UserServerDirectoryValidUsageTestCase(BaseUploadContentConfigurationTestCase): + + @classmethod + def user_server_dir(cls): + return cls.temp_config_dir("user_library_import_dir") + + @classmethod + def handle_galaxy_config_kwds(cls, config): + user_server_dir = cls.user_server_dir() + os.makedirs(user_server_dir) + config["user_library_import_dir"] = os.path.join(user_server_dir) + + def test_valid_user_server_dir_uploads_okay(self): + dir_to_import = 'library' + full_dir_path = os.path.join(self.user_server_dir(), TEST_USER, dir_to_import) + os.makedirs(full_dir_path) + file_content = "hello world\n" + with tempfile.NamedTemporaryFile(mode='w', dir=full_dir_path, delete=False) as fh: + fh.write(file_content) + file_to_import = fh.name + + library_dataset = self.library_populator.new_library_dataset("serverdirupload", upload_option="upload_directory", server_dir=dir_to_import) + # Check the file is still there and was not modified + with open(file_to_import, 'r') as fh: + read_content = fh.read() + assert read_content == file_content + + assert library_dataset["file_size"] == 12, library_dataset + + class FetchByPathTestCase(BaseUploadContentConfigurationTestCase): require_admin_user = True