From 4588977a82db7a40c0f94a1b2236539af887e902 Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 14 Oct 2020 18:07:23 +0200 Subject: [PATCH 01/13] Fix handling of collection element identifiers in history panel --- client/galaxy/scripts/mvc/dataset/dataset-li-edit.js | 3 ++- client/galaxy/scripts/mvc/dataset/dataset-li.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/galaxy/scripts/mvc/dataset/dataset-li-edit.js b/client/galaxy/scripts/mvc/dataset/dataset-li-edit.js index 742b2f33170..2367ba5910b 100644 --- a/client/galaxy/scripts/mvc/dataset/dataset-li-edit.js +++ b/client/galaxy/scripts/mvc/dataset/dataset-li-edit.js @@ -66,8 +66,9 @@ var DatasetListItemEdit = _super.extend( const Galaxy = getGalaxyInstance(); if (Galaxy.router) { ev.preventDefault(); + const identifier = self.model.get("element_id") || self.model.get("id"); Galaxy.router.push("datasets/edit", { - dataset_id: self.model.attributes.id + dataset_id: identifier }); } } diff --git a/client/galaxy/scripts/mvc/dataset/dataset-li.js b/client/galaxy/scripts/mvc/dataset/dataset-li.js index 17f2701edbd..efdb5353809 100644 --- a/client/galaxy/scripts/mvc/dataset/dataset-li.js +++ b/client/galaxy/scripts/mvc/dataset/dataset-li.js @@ -184,7 +184,8 @@ export var DatasetListItemView = _super.extend( const Galaxy = getGalaxyInstance(); if (Galaxy.frame && Galaxy.frame.active) { // Add dataset to frames. - Galaxy.frame.addDataset(self.model.get("id")); + const identifier = self.model.get("element_id") || self.model.get("id"); + Galaxy.frame.addDataset(identifier); ev.preventDefault(); } }; From 69142e90f8ca5d105827b7f22492193b89abbef5 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Wed, 21 Oct 2020 22:41:27 -0400 Subject: [PATCH 02/13] Allow username or email for authenticate API identity --- lib/galaxy/webapps/galaxy/api/authenticate.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/api/authenticate.py b/lib/galaxy/webapps/galaxy/api/authenticate.py index 4b0da547661..54545269d56 100644 --- a/lib/galaxy/webapps/galaxy/api/authenticate.py +++ b/lib/galaxy/webapps/galaxy/api/authenticate.py @@ -17,6 +17,7 @@ from six.moves.urllib.parse import unquote from galaxy import exceptions from galaxy.managers import api_keys +from galaxy.security.validate_user_input import VALID_PUBLICNAME_RE from galaxy.util import ( smart_str, unicodify @@ -59,10 +60,12 @@ class AuthenticationController(BaseAPIController): :raises: ObjectNotFound, HTTPBadRequest """ - email, password = self._decode_baseauth(trans.environ.get('HTTP_AUTHORIZATION')) - - user = trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.email == email).all() - + identity, password = self._decode_baseauth(trans.environ.get('HTTP_AUTHORIZATION')) + # check if this is an email address or username + if VALID_PUBLICNAME_RE.match(identity): + user = trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.username == identity).all() + else: + user = trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.email == identity).all() if len(user) == 0: raise exceptions.ObjectNotFound('The user does not exist.') elif len(user) > 1: From b21bab7f7d20edb0238e758b55122cca6c8324a9 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Thu, 8 Oct 2020 23:19:11 -0400 Subject: [PATCH 03/13] Bugfix: include an index link in visualization base to allow the app to derive baseUrl --- .../visualizations/common/templates/visualization_base.mako | 1 + 1 file changed, 1 insertion(+) diff --git a/config/plugins/visualizations/common/templates/visualization_base.mako b/config/plugins/visualizations/common/templates/visualization_base.mako index 3fd2b5a8b8f..759689b6590 100644 --- a/config/plugins/visualizations/common/templates/visualization_base.mako +++ b/config/plugins/visualizations/common/templates/visualization_base.mako @@ -20,6 +20,7 @@ ${self.title()} + ${self.metas()} ${self.stylesheets()} From 2f27d2e82dd8c0f5731b5d9aa1c5fef1a53ceeb4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 23 Oct 2020 11:01:23 +0200 Subject: [PATCH 04/13] Unify getting user by email or username from database Note that handling the case of more than 1 record returned is different between the 2 original methods. - The regular login retrieves just the first matching record with `.first()` while `get_api_key` raises an Exception. I've chosen to not raise an Exception since we don't do this with regular logins and this targets 20.09. We could experiment with raising an exception in dev, since that situation should hopefully not be possible. - The baseauth method did require exact email capitalization (which was a problem for some of Jen's account on usegalaxy.org and for users with particular LDAP setups, xref: https://github.com/galaxyproject/galaxy/issues/8602) --- lib/galaxy/managers/users.py | 16 ++++++++++++- lib/galaxy/webapps/galaxy/api/authenticate.py | 21 +++++++--------- lib/galaxy/webapps/galaxy/controllers/user.py | 10 +------- test/unit/managers/test_UserManager.py | 24 +++++++++++++++++++ 4 files changed, 48 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 7d0058ff189..0e7f003f9f8 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -8,7 +8,7 @@ import time from datetime import datetime from markupsafe import escape -from sqlalchemy import and_, desc, exc, func, true +from sqlalchemy import and_, desc, exc, func, or_, true from galaxy import ( exceptions, @@ -278,6 +278,20 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): raise exceptions.AuthenticationFailed(msg, **kwargs) return user + def get_user_by_identity(self, identity): + """Get user by username or email.""" + session = self.session() + user = session.query(self.model_class).filter(or_( + self.model_class.table.c.email == identity, + self.model_class.table.c.username == identity, + )).first() + if not user: + # Try a case-insensitive match on the email + user = session.query(self.model_class).filter( + func.lower(self.model_class.table.c.email) == identity.lower() + ).first() + return user + # ---- current def current_user(self, trans): # define here for single point of change and make more readable diff --git a/lib/galaxy/webapps/galaxy/api/authenticate.py b/lib/galaxy/webapps/galaxy/api/authenticate.py index 54545269d56..668da0215b8 100644 --- a/lib/galaxy/webapps/galaxy/api/authenticate.py +++ b/lib/galaxy/webapps/galaxy/api/authenticate.py @@ -16,8 +16,10 @@ from base64 import b64decode from six.moves.urllib.parse import unquote from galaxy import exceptions -from galaxy.managers import api_keys -from galaxy.security.validate_user_input import VALID_PUBLICNAME_RE +from galaxy.managers import ( + api_keys, + users, +) from galaxy.util import ( smart_str, unicodify @@ -32,6 +34,7 @@ class AuthenticationController(BaseAPIController): def __init__(self, app): super().__init__(app) + self.user_manager = users.UserManager(app) self.api_keys_manager = api_keys.ApiKeyManager(app) @expose_api_anonymous_and_sessionless @@ -62,18 +65,10 @@ class AuthenticationController(BaseAPIController): """ identity, password = self._decode_baseauth(trans.environ.get('HTTP_AUTHORIZATION')) # check if this is an email address or username - if VALID_PUBLICNAME_RE.match(identity): - user = trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.username == identity).all() - else: - user = trans.sa_session.query(trans.app.model.User).filter(trans.app.model.User.table.c.email == identity).all() - if len(user) == 0: + user = self.user_manager.get_user_by_identity(identity) + if not user: raise exceptions.ObjectNotFound('The user does not exist.') - elif len(user) > 1: - # DB is inconsistent and we have more users with the same email. - raise exceptions.InconsistentDatabase('An error occurred, please contact your administrator.') - else: - user = user[0] - is_valid_user = self.app.auth_manager.check_password(user, password) + is_valid_user = self.app.auth_manager.check_password(user, password) if is_valid_user: key = self.api_keys_manager.get_or_create_api_key(user) return dict(api_key=key) diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index dbe211b7076..6691e6a2aa7 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -134,15 +134,7 @@ class User(BaseUIController, UsesFormDefinitionsMixin, CreatesApiKeysMixin): status = None if not login or not password: return self.message_exception(trans, "Please specify a username and password.") - user = trans.sa_session.query(trans.app.model.User).filter(or_( - trans.app.model.User.table.c.email == login, - trans.app.model.User.table.c.username == login - )).first() - if not user: - # Try a case-insensitive match on the email - user = trans.sa_session.query(trans.app.model.User).filter( - func.lower(trans.app.model.User.table.c.email) == login.lower() - ).first() + user = self.user_manager.get_user_by_identity(login) log.debug("trans.app.config.auth_config_file: %s" % trans.app.config.auth_config_file) if user is None: message, user = self.__autoregistration(trans, login, password) diff --git a/test/unit/managers/test_UserManager.py b/test/unit/managers/test_UserManager.py index 4aa3a377814..57e8c75961a 100644 --- a/test/unit/managers/test_UserManager.py +++ b/test/unit/managers/test_UserManager.py @@ -25,6 +25,8 @@ changed_password = '654321' user2_data = dict(email='user2@user2.user2', username='user2', password=default_password) user3_data = dict(email='user3@user3.user3', username='user3', password=default_password) user4_data = dict(email='user4@user4.user4', username='user4', password=default_password) +uppercase_email_user = dict(email='USER5@USER5.USER5', username='USER5', password=default_password) +lowercase_email_user = dict(email='user5@user5.user5', username='user5', password=default_password) # ============================================================================= @@ -194,6 +196,28 @@ class UserManagerTestCase(BaseTestCase): self.assertFalse(check_password("", user.password)) self.assertFalse(check_password(None, user.password)) + def test_get_user_by_identity(self): + # return None if username/email not found + assert self.user_manager.get_user_by_identity('xyz') is None + uppercase_user = self.user_manager.create(**uppercase_email_user) + assert uppercase_user.email == uppercase_email_user['email'] + assert uppercase_user.username == uppercase_email_user['username'] + assert self.user_manager.get_user_by_identity(uppercase_user.email) == uppercase_user + assert self.user_manager.get_user_by_identity(uppercase_user.username) == uppercase_user + lowercase_user = self.user_manager.create(**lowercase_email_user) + assert lowercase_user.email == lowercase_email_user['email'] + assert lowercase_user.username == lowercase_email_user['username'] + assert self.user_manager.get_user_by_identity(lowercase_user.email) == lowercase_user + assert self.user_manager.get_user_by_identity(lowercase_user.username) == lowercase_user + # assert uppercase user can still be retrieved + assert self.user_manager.get_user_by_identity(uppercase_user.email) == uppercase_user + assert self.user_manager.get_user_by_identity(uppercase_user.username) == uppercase_user + # username matches need to be exact + assert self.user_manager.get_user_by_identity(uppercase_user.username.capitalize()) is None + # email matches can ignore capitalization + ignore_email_capitalization_user = self.user_manager.create(email='user123@nopassword.com', username='someusername123') + assert self.user_manager.get_user_by_identity(ignore_email_capitalization_user.email.capitalize()) == ignore_email_capitalization_user + # ============================================================================= class UserSerializerTestCase(BaseTestCase): From d22ec5bb5bb3365166a846c353392201b771bbe2 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 23 Oct 2020 09:08:29 -0400 Subject: [PATCH 05/13] Drop now-unused imports --- lib/galaxy/webapps/galaxy/controllers/user.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 6691e6a2aa7..d716b27542f 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -7,10 +7,6 @@ from datetime import datetime, timedelta from markupsafe import escape from six.moves.urllib.parse import unquote -from sqlalchemy import ( - func, - or_ -) from sqlalchemy.orm.exc import NoResultFound from galaxy import ( From bd32a66cf071f293f355e5ada3cc2cb1a6d0eca1 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 23 Oct 2020 08:55:21 -0400 Subject: [PATCH 06/13] Improve get_by_identity by using regex to determine identity type Also fixes an issue with double querying if it's a username non-match. --- lib/galaxy/managers/users.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 0e7f003f9f8..146c2c13ab5 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -8,7 +8,7 @@ import time from datetime import datetime from markupsafe import escape -from sqlalchemy import and_, desc, exc, func, or_, true +from sqlalchemy import and_, desc, exc, func, true from galaxy import ( exceptions, @@ -21,6 +21,7 @@ from galaxy.managers import ( deletable ) from galaxy.security.validate_user_input import ( + VALID_PUBLICNAME_RE, validate_email, validate_password, validate_publicname @@ -280,16 +281,18 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): def get_user_by_identity(self, identity): """Get user by username or email.""" - session = self.session() - user = session.query(self.model_class).filter(or_( - self.model_class.table.c.email == identity, - self.model_class.table.c.username == identity, - )).first() - if not user: - # Try a case-insensitive match on the email - user = session.query(self.model_class).filter( - func.lower(self.model_class.table.c.email) == identity.lower() - ).first() + user = None + if VALID_PUBLICNAME_RE.match(identity): + # VALID_PUBLICNAME and VALID_EMAIL do not overlap, so 'identity' here is publicname + user = self.session().query(self.model_class).filter( + self.model_class.table.c.username == identity).first() + else: + user = self.session().query(self.model_class).filter( + self.model_class.table.c.email == identity).first() + if not user: + # Try a case-insensitive match on the email + user = self.session().query(self.model_class).filter( + func.lower(self.model_class.table.c.email) == identity.lower()).first() return user # ---- current From ddbeecf86b7850045df32218dfec9e67ed8bbd0f Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 23 Oct 2020 10:04:13 -0400 Subject: [PATCH 07/13] Add comment suggested by @nsoranzo --- test/unit/managers/test_UserManager.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/managers/test_UserManager.py b/test/unit/managers/test_UserManager.py index 57e8c75961a..9ccfbd0096c 100644 --- a/test/unit/managers/test_UserManager.py +++ b/test/unit/managers/test_UserManager.py @@ -204,6 +204,9 @@ class UserManagerTestCase(BaseTestCase): assert uppercase_user.username == uppercase_email_user['username'] assert self.user_manager.get_user_by_identity(uppercase_user.email) == uppercase_user assert self.user_manager.get_user_by_identity(uppercase_user.username) == uppercase_user + # Create another user with the same email just differently capitalized. + # This is not normally allowed now, since registration goes through user_manager.register(), + # which checks for that, but was possible in earlier releases of Galaxy lowercase_user = self.user_manager.create(**lowercase_email_user) assert lowercase_user.email == lowercase_email_user['email'] assert lowercase_user.username == lowercase_email_user['username'] From bd0a015a0265a8045c7adc022225c14ce1cfef6a Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Sat, 24 Oct 2020 09:57:08 -0400 Subject: [PATCH 08/13] Invert publicname/email regex detection logic We actually do have some capitalized publicnames historically. --- lib/galaxy/managers/users.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 146c2c13ab5..183eca0629f 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -21,7 +21,7 @@ from galaxy.managers import ( deletable ) from galaxy.security.validate_user_input import ( - VALID_PUBLICNAME_RE, + VALID_EMAIL_RE, validate_email, validate_password, validate_publicname @@ -282,17 +282,17 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): def get_user_by_identity(self, identity): """Get user by username or email.""" user = None - if VALID_PUBLICNAME_RE.match(identity): - # VALID_PUBLICNAME and VALID_EMAIL do not overlap, so 'identity' here is publicname - user = self.session().query(self.model_class).filter( - self.model_class.table.c.username == identity).first() - else: + if VALID_EMAIL_RE.match(identity): + # VALID_PUBLICNAME and VALID_EMAIL do not overlap, so 'identity' here is an email address user = self.session().query(self.model_class).filter( self.model_class.table.c.email == identity).first() if not user: # Try a case-insensitive match on the email user = self.session().query(self.model_class).filter( func.lower(self.model_class.table.c.email) == identity.lower()).first() + else: + user = self.session().query(self.model_class).filter( + self.model_class.table.c.username == identity).first() return user # ---- current From 23796903d34a84ed44dbd2ad88bba56abfe84076 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 26 Oct 2020 12:59:37 +0100 Subject: [PATCH 09/13] Fix composite dataset listing in upload modal Likely broken in 90c03fcc2622b9f783a160ceeb63ac9d0bb09d46. --- client/src/components/Upload/UploadBoxMixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/Upload/UploadBoxMixin.js b/client/src/components/Upload/UploadBoxMixin.js index 09b2eb981e9..8904c84b29b 100644 --- a/client/src/components/Upload/UploadBoxMixin.js +++ b/client/src/components/Upload/UploadBoxMixin.js @@ -259,7 +259,7 @@ export default { return $(this.$refs.uploadTable); }, extensionDetails(extension) { - return findExtension(this.effectiveExtensions, extension); + return findExtension(this.app.effectiveExtensions, extension); }, initExtensionInfo() { $(this.$refs.footerExtensionInfo) From 6500e32f371a39d2e7d6ff05109418b478f2c74a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 26 Oct 2020 14:46:27 +0100 Subject: [PATCH 10/13] Fix get_output_path when HDA identity changes @RJMW reported this on gitter: ``` galaxy.jobs.runners ERROR 2020-10-13 09:34:24,937 [p:23472,w:1,m:0] [SlurmRunner.monitor_thread] (442471) Failure preparing job Traceback (most recent call last): File "lib/galaxy/jobs/runners/__init__.py", line 236, in prepare_job job_wrapper.prepare() File "lib/galaxy/jobs/__init__.py", line 1077, in prepare tool_evaluator.set_compute_environment(compute_environment, get_special=get_special) File "lib/galaxy/tools/evaluation.py", line 110, in set_compute_environment output_collections=out_collections, File "lib/galaxy/tools/evaluation.py", line 149, in build_param_dict self.__populate_output_dataset_wrappers(param_dict, output_datasets, job_working_directory) File "lib/galaxy/tools/evaluation.py", line 341, in __populate_output_dataset_wrappers param_dict[name] = DatasetFilenameWrapper(hda, compute_environment=self.compute_environment, io_type="output") File "lib/galaxy/tools/wrappers.py", line 289, in __init__ path_rewrite = compute_environment and compute_environment.output_path_rewrite(dataset) File "lib/galaxy/jobs/__init__.py", line 2546, in output_path_rewrite dataset_path = self.job_wrapper.get_output_path(dataset) File "lib/galaxy/jobs/__init__.py", line 1912, in get_output_path raise KeyError("Couldn't find job output for [%s] in [%s]" % (dataset, self.output_hdas_and_paths.values())) KeyError: "Couldn't find job output for [] in [dict_values([(, )])]" ``` The HDA with the id 714739 is present in the outputs, but the identity is not the same. I don't know why that happened (maybe a flush?) but in this case it should be safe to compare HDAs by database id. --- lib/galaxy/jobs/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index cde61b8df24..10c3de9c91b 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1910,13 +1910,14 @@ class JobWrapper(HasResourceParameters): return self.output_paths def get_output_path(self, dataset): + if getattr(dataset, "fake_dataset_association", False): + return dataset.file_name + assert dataset.id is not None, "{} needs to be flushed to find output path".format(dataset) if self.output_paths is None: self.compute_outputs() for (hda, dataset_path) in self.output_hdas_and_paths.values(): - if hda == dataset: + if hda.id == dataset.id: return dataset_path - if getattr(dataset, "fake_dataset_association", False): - return dataset.file_name raise KeyError("Couldn't find job output for [%s] in [%s]" % (dataset, self.output_hdas_and_paths.values())) def get_mutable_output_fnames(self): From ed7e0fcd0cf52315b2902132db4ad5ebd94e3dd2 Mon Sep 17 00:00:00 2001 From: Oleg Zharkov Date: Tue, 27 Oct 2020 12:42:12 +0200 Subject: [PATCH 11/13] allow adding dataset from history --- .../components/LibraryFolder/TopToolbar/FolderTopBar.vue | 9 --------- 1 file changed, 9 deletions(-) diff --git a/client/src/components/LibraryFolder/TopToolbar/FolderTopBar.vue b/client/src/components/LibraryFolder/TopToolbar/FolderTopBar.vue index da8dacfcf9f..5ddcbbaa85c 100644 --- a/client/src/components/LibraryFolder/TopToolbar/FolderTopBar.vue +++ b/client/src/components/LibraryFolder/TopToolbar/FolderTopBar.vue @@ -29,7 +29,6 @@