From 98bbb8345fab1edf7365b121762993c521faa966 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 19 Jan 2022 13:44:27 +0100 Subject: [PATCH 1/8] enforce correct minimum username length Co-authored-by: Nicola Soranzo --- lib/galaxy/security/validate_user_input.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index 4f20444b3e9..e2744bdf401 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -111,7 +111,7 @@ def transform_publicname(publicname): if publicname not in ['None', None, '']: publicname = publicname.lower() publicname = re.sub(VALID_PUBLICNAME_SUB, FILL_CHAR, publicname) - publicname = publicname.ljust(PUBLICNAME_MIN_LEN + 1, FILL_CHAR)[:PUBLICNAME_MAX_LEN] + publicname = publicname.ljust(PUBLICNAME_MIN_LEN, FILL_CHAR)[:PUBLICNAME_MAX_LEN] return publicname From e6275030c0ec7a65504ea380858f651d3fd4e5fb Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 19 Jan 2022 13:46:23 +0100 Subject: [PATCH 2/8] lower the min username length limit to 1 Two would be sufficient to make at least one `Yu`, `Li`, ... per Galaxy instance happy :) Korea seem to allow for single letter names. Anyway, I just want to make Mr T. happy :) The serious background is that this seems to be an easy way to solve the following problem: If a user is registered some transformation of the username happens, e.g. it is filled by - to enforce the minlength requirement. On LDAP managed systems that submit jobs as real users this leads to the problem that users with to short names can't submit jobs. --- lib/galaxy/security/validate_user_input.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index e2744bdf401..caf0e4719a2 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -16,7 +16,7 @@ VALID_EMAIL_RE = re.compile(r"[^@]+@[^@]+\.[^@]+") EMAIL_MAX_LEN = 255 # Public name validity parameters -PUBLICNAME_MIN_LEN = 3 +PUBLICNAME_MIN_LEN = 1 PUBLICNAME_MAX_LEN = 255 VALID_PUBLICNAME_RE = re.compile(r"^[a-z0-9._\-]+$") VALID_PUBLICNAME_SUB = re.compile(r"[^a-z0-9._\-]") From 64576d0f7d765d255546c42eb7ede2b9da828da8 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 19 Jan 2022 14:08:16 +0100 Subject: [PATCH 3/8] adapt unit test to new minlength requirement --- test/unit/app/managers/test_UserManager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index 05f16869687..3a7d2b8f359 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -289,7 +289,7 @@ class UserDeserializerTestCase(BaseTestCase): self.log("usernames must be long enough and with no non-hyphen punctuation") exception = self._assertRaises_and_return_raised(base_manager.ModelDeserializingError, - self.deserializer.deserialize, user, {'username': 'ed'}, trans=self.trans) + self.deserializer.deserialize, user, {'username': ''}, trans=self.trans) self.assertTrue('Public name must be at least' in str(exception)) self.assertRaises(base_manager.ModelDeserializingError, self.deserializer.deserialize, user, {'username': 'f,d,r,'}, trans=self.trans) From daac8c9da18a0ad0c57c1d53b5536c806f6fbcba Mon Sep 17 00:00:00 2001 From: M Bernt Date: Thu, 20 Jan 2022 11:16:45 +0100 Subject: [PATCH 4/8] Apply suggestions from code review Co-authored-by: Nicola Soranzo --- lib/galaxy/security/validate_user_input.py | 3 +-- test/unit/app/managers/test_UserManager.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index caf0e4719a2..a6f6aee2f5b 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -16,7 +16,6 @@ VALID_EMAIL_RE = re.compile(r"[^@]+@[^@]+\.[^@]+") EMAIL_MAX_LEN = 255 # Public name validity parameters -PUBLICNAME_MIN_LEN = 1 PUBLICNAME_MAX_LEN = 255 VALID_PUBLICNAME_RE = re.compile(r"^[a-z0-9._\-]+$") VALID_PUBLICNAME_SUB = re.compile(r"[^a-z0-9._\-]") @@ -111,7 +110,7 @@ def transform_publicname(publicname): if publicname not in ['None', None, '']: publicname = publicname.lower() publicname = re.sub(VALID_PUBLICNAME_SUB, FILL_CHAR, publicname) - publicname = publicname.ljust(PUBLICNAME_MIN_LEN, FILL_CHAR)[:PUBLICNAME_MAX_LEN] + publicname = publicname[:PUBLICNAME_MAX_LEN] return publicname diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index 3a7d2b8f359..36840725b6e 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -290,7 +290,7 @@ class UserDeserializerTestCase(BaseTestCase): self.log("usernames must be long enough and with no non-hyphen punctuation") exception = self._assertRaises_and_return_raised(base_manager.ModelDeserializingError, self.deserializer.deserialize, user, {'username': ''}, trans=self.trans) - self.assertTrue('Public name must be at least' in str(exception)) + self.assertTrue('Public name cannot be empty' in str(exception)) self.assertRaises(base_manager.ModelDeserializingError, self.deserializer.deserialize, user, {'username': 'f,d,r,'}, trans=self.trans) From 28d1b8ec51dd8c0f2d1e8bb008bb75bc235a6f87 Mon Sep 17 00:00:00 2001 From: M Bernt Date: Thu, 20 Jan 2022 11:17:18 +0100 Subject: [PATCH 5/8] Update lib/galaxy/security/validate_user_input.py Co-authored-by: Nicola Soranzo --- lib/galaxy/security/validate_user_input.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index a6f6aee2f5b..9a86520baa1 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -107,9 +107,10 @@ def transform_publicname(publicname): FILL_CHAR is used to extend or replace characters. """ # TODO: Enhance to allow generation of semi-random publicnnames e.g., when valid but taken - if publicname not in ['None', None, '']: - publicname = publicname.lower() - publicname = re.sub(VALID_PUBLICNAME_SUB, FILL_CHAR, publicname) + if not publicname: + raise ValueError("Public name cannot be empty") + publicname = publicname.lower() + publicname = re.sub(VALID_PUBLICNAME_SUB, FILL_CHAR, publicname) publicname = publicname[:PUBLICNAME_MAX_LEN] return publicname From a522d1ac94d2c9e91dad35f6fbbdc86137c7e8ed Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 20 Jan 2022 11:29:31 +0100 Subject: [PATCH 6/8] also adapt validate_publicname_str using the same error message as in transform_publicname --- lib/galaxy/security/validate_user_input.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/security/validate_user_input.py b/lib/galaxy/security/validate_user_input.py index 9a86520baa1..144f28b891f 100644 --- a/lib/galaxy/security/validate_user_input.py +++ b/lib/galaxy/security/validate_user_input.py @@ -44,8 +44,8 @@ def validate_password_str(password): def validate_publicname_str(publicname): """Validates a string containing a public username.""" - if len(publicname) < PUBLICNAME_MIN_LEN: - return "Public name must be at least %d characters in length." % (PUBLICNAME_MIN_LEN) + if not publicname: + return "Public name cannot be empty" if len(publicname) > PUBLICNAME_MAX_LEN: return "Public name cannot be more than %d characters in length." % (PUBLICNAME_MAX_LEN) if not(VALID_PUBLICNAME_RE.match(publicname)): From 131be308163d792652501686535b526ed29b94e4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 21 Apr 2022 16:58:41 +0200 Subject: [PATCH 7/8] adapt test for too short user name --- lib/galaxy_test/api/test_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_users.py b/lib/galaxy_test/api/test_users.py index 386dd543207..6bf739dcc69 100644 --- a/lib/galaxy_test/api/test_users.py +++ b/lib/galaxy_test/api/test_users.py @@ -55,7 +55,7 @@ class UsersApiTestCase(ApiTestCase): self.assertEqual(update_json["username"], new_name) # too short - update_response = self.__update(user, username="mu") + update_response = self.__update(user, username="") self._assert_status_code_is(update_response, 400) # not them From cfc2043e00acc774c8c30eed69d4d8a3270d4900 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 21 Apr 2022 16:58:58 +0200 Subject: [PATCH 8/8] fix black formatting --- test/unit/app/managers/test_UserManager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index 67632a91686..3e3b42bed21 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -316,7 +316,7 @@ class UserDeserializerTestCase(BaseTestCase): {"username": ""}, trans=self.trans, ) - self.assertTrue('Public name cannot be empty' in str(exception)) + self.assertTrue("Public name cannot be empty" in str(exception)) self.assertRaises( base_manager.ModelDeserializingError, self.deserializer.deserialize,