From fe1420aa87be93e1db29a182d642faab465d6533 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Thu, 30 Jan 2025 17:49:29 -0500 Subject: [PATCH 1/4] validate usernames --- auth/auth/auth.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/auth/auth/auth.py b/auth/auth/auth.py index afae0d552c9..bfc995bd0f0 100644 --- a/auth/auth/auth.py +++ b/auth/auth/auth.py @@ -151,6 +151,12 @@ async def check_valid_new_user(tx: Transaction, username, login_id, is_developer return None +def validate_username(username): + regex = re.compile(r'^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$') + if not regex.match(username): + raise InvalidUsername(username) + + async def insert_new_user( db: Database, username: str, @@ -183,6 +189,7 @@ async def _insert(tx): ), ) + validate_username(username) await _insert() return True From 7cd31cdf298a39708531d164cfd032a910d9de9d Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Fri, 31 Jan 2025 10:24:49 -0500 Subject: [PATCH 2/4] log decision making --- auth/auth/auth.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/auth/auth/auth.py b/auth/auth/auth.py index bfc995bd0f0..e78e5a07923 100644 --- a/auth/auth/auth.py +++ b/auth/auth/auth.py @@ -152,9 +152,13 @@ async def check_valid_new_user(tx: Transaction, username, login_id, is_developer def validate_username(username): + log.info(f'validating username {username}') regex = re.compile(r'^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$') if not regex.match(username): + log.error(f'invalid username {username}') raise InvalidUsername(username) + else: + log.info(f'valid username {username}') async def insert_new_user( From f0cc17a0b8f7c10bba5813619cf7fba0be6c5a2c Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Fri, 31 Jan 2025 16:04:20 -0500 Subject: [PATCH 3/4] more instructive error message --- auth/auth/auth.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/auth/auth.py b/auth/auth/auth.py index e78e5a07923..a8233c47bc0 100644 --- a/auth/auth/auth.py +++ b/auth/auth/auth.py @@ -155,10 +155,10 @@ def validate_username(username): log.info(f'validating username {username}') regex = re.compile(r'^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$') if not regex.match(username): - log.error(f'invalid username {username}') - raise InvalidUsername(username) - else: - log.info(f'valid username {username}') + raise AuthUserError( + f'invalid username {username}, must match RFC1123 (lowercase alphanumeric plus "." and "-", start and end with alphanumeric)', + 'error', + ) async def insert_new_user( From 8353aadd85ca0d966b9a3393a7acd52d268ff5d8 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Fri, 31 Jan 2025 17:32:40 -0500 Subject: [PATCH 4/4] review feedback --- auth/auth/auth.py | 14 ++++++++------ auth/auth/exceptions.py | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/auth/auth/auth.py b/auth/auth/auth.py index a8233c47bc0..4413b923b3e 100644 --- a/auth/auth/auth.py +++ b/auth/auth/auth.py @@ -128,7 +128,7 @@ async def check_valid_new_user(tx: Transaction, username, login_id, is_developer raise MultipleUserTypes(username) if not is_service_account and not login_id: raise EmptyLoginID(username) - if not username or not all(c for c in username if c.isalnum()): + if not username or not (username.isalnum() and username.islower()): raise InvalidUsername(username) existing_users = await users_with_username_or_login_id(tx, username, login_id) @@ -151,12 +151,14 @@ async def check_valid_new_user(tx: Transaction, username, login_id, is_developer return None -def validate_username(username): - log.info(f'validating username {username}') +def validate_credentials_secret_name_input(secret_name: Optional[str]): + if secret_name is None: + return + regex = re.compile(r'^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$') - if not regex.match(username): + if not regex.match(secret_name): raise AuthUserError( - f'invalid username {username}, must match RFC1123 (lowercase alphanumeric plus "." and "-", start and end with alphanumeric)', + f'invalid credentials_secret_name {secret_name}. Must match RFC1123 (lowercase alphanumeric plus "." and "-", start and end with alphanumeric)', 'error', ) @@ -193,7 +195,7 @@ async def _insert(tx): ), ) - validate_username(username) + validate_credentials_secret_name_input(hail_credentials_secret_name) await _insert() return True diff --git a/auth/auth/exceptions.py b/auth/auth/exceptions.py index 769404ae2e9..7ae2c2d94bf 100644 --- a/auth/auth/exceptions.py +++ b/auth/auth/exceptions.py @@ -28,7 +28,7 @@ def __init__(self, username, login_id): class InvalidUsername(AuthUserError): def __init__(self, username): - super().__init__(f"Invalid username '{username}'. Must be a non-empty alphanumeric string.", 'error') + super().__init__(f"Invalid username '{username}'. Must be a non-empty lowercase alphanumeric string.", 'error') class InvalidType(AuthUserError):