Skip to content

Commit 3f9a382

Browse files
authored
[auth] Allow (non-adjacent) hyphens in usernames (hail-is#14841)
## Change Description Addresses hail-is#14840 by allowing well-placed hyphens in usernames again. Not required for the Broad deployment but is perfectly safe to do, and was blocking external collaborators whose usernames frequently require hyphens. ## Security Assessment - This change has a medium security impact ### Impact Description Modifies the set of usernames allowed, but only by allowing well-placed individual hyphens, which will still produce "url valid" downstream resource names. (Reviewers: please confirm the security impact before approving)
1 parent 4a2f47f commit 3f9a382

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

auth/auth/auth.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
web_security_headers,
4848
)
4949

50-
from .auth_utils import validate_credentials_secret_name_input
50+
from .auth_utils import is_valid_username, validate_credentials_secret_name_input
5151
from .exceptions import (
5252
AuthUserError,
5353
DuplicateLoginID,
@@ -129,7 +129,7 @@ async def check_valid_new_user(tx: Transaction, username, login_id, is_developer
129129
raise MultipleUserTypes(username)
130130
if not is_service_account and not login_id:
131131
raise EmptyLoginID(username)
132-
if not username or not (username.isalnum() and username.islower()):
132+
if not is_valid_username(username):
133133
raise InvalidUsername(username)
134134

135135
existing_users = await users_with_username_or_login_id(tx, username, login_id)

auth/auth/auth_utils.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,32 @@ def validate_credentials_secret_name_input(secret_name: Optional[str]):
1414
f'invalid credentials_secret_name {secret_name}. Must match RFC1123 (lowercase alphanumeric plus "." and "-", start and end with alphanumeric)',
1515
'error',
1616
)
17+
18+
19+
def is_valid_username(username: str) -> bool:
20+
"""Check if a username is valid.
21+
22+
Requirements:
23+
1. Only alphanumeric characters and hyphens allowed
24+
2. Hyphens cannot be at start or end
25+
3. Hyphens cannot be adjacent to each other
26+
27+
Args:
28+
username: The username to validate
29+
30+
Returns:
31+
bool: True if username meets all requirements, False otherwise
32+
"""
33+
if not username: # Check for empty string
34+
return False
35+
36+
# Check for hyphens at start or end
37+
if username.startswith('-') or username.endswith('-'):
38+
return False
39+
40+
# Check for adjacent hyphens
41+
if '--' in username:
42+
return False
43+
44+
# Check that all characters are numeric or lowercase or hyphen:
45+
return all(c.isascii() and (c.isdigit() or c.islower() or c == '-') for c in username)

auth/test/test_auth_utils.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import pytest
22

3-
from auth.auth_utils import validate_credentials_secret_name_input
3+
from auth.auth_utils import is_valid_username, validate_credentials_secret_name_input
44
from auth.exceptions import AuthUserError
55

66

@@ -62,3 +62,34 @@ def test_validate_credentials_secret_name_input_valid(input_secret_name):
6262
def test_validate_credentials_secret_name_input_invalid(input_secret_name):
6363
with pytest.raises(AuthUserError):
6464
validate_credentials_secret_name_input(input_secret_name)
65+
66+
67+
@pytest.mark.parametrize("input_username", ['abc', 'abc-def', 'a3a', 'a3a-3a', '123'])
68+
def test_is_valid_username_valid(input_username):
69+
assert is_valid_username(input_username)
70+
71+
72+
@pytest.mark.parametrize(
73+
"input_username",
74+
[
75+
None,
76+
'',
77+
'ABC',
78+
'abc!',
79+
'a3a.3a',
80+
'abc_def',
81+
'abc!def',
82+
'abc.def.',
83+
'abc.def',
84+
'abc.def-ghi',
85+
'abc-def.ghi',
86+
'abc--def',
87+
'abc.def--ghi',
88+
'abc.def-.ghi',
89+
'.abc',
90+
'abc.',
91+
'abc..def',
92+
],
93+
)
94+
def test_is_valid_username_invalid(input_username):
95+
assert not is_valid_username(input_username)

0 commit comments

Comments
 (0)