Skip to content

Commit

Permalink
Fix importing a directory from user_library_import_dir
Browse files Browse the repository at this point in the history
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 f4442ff .

Reported by @jhl667 .
  • Loading branch information
nsoranzo committed Dec 11, 2019
1 parent a6d2d51 commit 0429c4d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
4 changes: 2 additions & 2 deletions lib/galaxy/actions/library.py
Expand Up @@ -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))
Expand Down
32 changes: 31 additions & 1 deletion test/integration/test_upload_configuration_options.py
Expand Up @@ -705,7 +705,7 @@ def server_dir(cls):
return cls.temp_config_dir("server")


class ServerDirectoryRestrictedToAdminsUsageTestCase(BaseUploadContentConfigurationTestCase):
class UserServerDirectoryOffByDefaultTestCase(BaseUploadContentConfigurationTestCase):

@classmethod
def handle_galaxy_config_kwds(cls, config):
Expand All @@ -719,6 +719,36 @@ def test_library_import_dir_not_available_to_non_admins(self):
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
Expand Down

0 comments on commit 0429c4d

Please sign in to comment.