From d3717b867af9b165e12a6e687957009bb01fdd66 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:13:40 +1000 Subject: [PATCH 01/10] Add constraints on username and password format --- routers/galaxy_register.py | 4 ++-- schemas/biocommons.py | 26 +++++++++++++++++--------- schemas/bpa.py | 6 ++++-- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/routers/galaxy_register.py b/routers/galaxy_register.py index 52224be3..5bca206f 100644 --- a/routers/galaxy_register.py +++ b/routers/galaxy_register.py @@ -39,11 +39,11 @@ def register( user_data = BiocommonsRegisterData.from_galaxy_registration(registration_data) logger.debug("Checking if username exists in Galaxy") - galaxy_username = user_data.user_metadata.galaxy_username + galaxy_username = user_data.username try: existing = galaxy_client.username_exists(galaxy_username) if existing: - raise HTTPException(status_code=400, detail="Username already exists") + raise HTTPException(status_code=400, detail="Username already exists in galaxy") except httpx.HTTPError as e: logger.warning(f"Failed to check username in Galaxy: {e}") diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 17e0f164..1e18a5b3 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -4,17 +4,24 @@ These are the core schemas we use for storing/representing users and their metadata """ +import re from datetime import datetime, timezone -from typing import List, Literal, Optional, Self +from typing import Annotated, List, Literal, Optional, Self -from pydantic import BaseModel, EmailStr, Field, HttpUrl +from pydantic import BaseModel, EmailStr, Field, HttpUrl, StringConstraints +import schemas.bpa +import schemas.galaxy from schemas import Resource, Service -from schemas.bpa import BPARegistrationRequest -from schemas.galaxy import GalaxyRegistrationData from schemas.service import Group, Identity +# From Auth0 password settings +ALLOWED_SPECIAL_CHARS = "!@#$%^&*" +VALID_PASSWORD_REGEX = re.compile(f"^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[{ALLOWED_SPECIAL_CHARS}]).{{8,}}$") + AppId = Literal["biocommons", "galaxy", "bpa"] +BiocommonsUsername = Annotated[str, StringConstraints(min_length=3, max_length=100, pattern='^[-_a-z0-9]+$')] +BiocommonsPassword = Annotated[str, StringConstraints(min_length=8, pattern=VALID_PASSWORD_REGEX)] class BPAMetadata(BaseModel): @@ -105,17 +112,18 @@ class BiocommonsRegisterData(BaseModel): """ email: EmailStr email_verified: bool = False - password: str + password: BiocommonsPassword connection: str = "Username-Password-Authentication" - username: str + username: BiocommonsUsername name: Optional[str] = None username: Optional[str] = None - user_metadata: BiocommonsUserMetadata + user_metadata: Optional[BiocommonsUserMetadata] = None app_metadata: BiocommonsAppMetadata @classmethod def from_bpa_registration( - cls, registration: BPARegistrationRequest, + cls, + registration: 'schemas.bpa.BPARegistrationRequest', bpa_service: Service) -> Self: return cls( email=registration.email, @@ -135,7 +143,7 @@ def from_bpa_registration( @classmethod def from_galaxy_registration( cls, - registration: GalaxyRegistrationData): + registration: 'schemas.galaxy.GalaxyRegistrationData',): # Galaxy registration is approved automatically galaxy_service = Service( name="Galaxy Australia", diff --git a/schemas/bpa.py b/schemas/bpa.py index 5b9a5036..5942012f 100644 --- a/schemas/bpa.py +++ b/schemas/bpa.py @@ -2,11 +2,13 @@ from pydantic import BaseModel, EmailStr +from schemas.biocommons import BiocommonsPassword, BiocommonsUsername + class BPARegistrationRequest(BaseModel): - username: str + username: BiocommonsUsername fullname: str email: EmailStr reason: str - password: str + password: BiocommonsPassword organizations: Dict[str, bool] From eb6efe9392539c52478172ebe451fda80a0e3441 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:15:55 +1000 Subject: [PATCH 02/10] Remove username for user_metadata, we now use a unified username --- schemas/biocommons.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 1e18a5b3..ca15365d 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -26,7 +26,6 @@ class BPAMetadata(BaseModel): registration_reason: str - username: str class BiocommonsUserMetadata(BaseModel): @@ -35,7 +34,6 @@ class BiocommonsUserMetadata(BaseModel): like preferred usernames """ bpa: Optional[BPAMetadata] = None - galaxy_username: Optional[str] = None class BiocommonsAppMetadata(BaseModel): @@ -131,8 +129,7 @@ def from_bpa_registration( username=registration.username, name=registration.fullname, user_metadata=BiocommonsUserMetadata( - bpa=BPAMetadata(registration_reason=registration.reason, - username=registration.username,), + bpa=BPAMetadata(registration_reason=registration.reason), ), app_metadata=BiocommonsAppMetadata( services=[bpa_service], @@ -155,7 +152,7 @@ def from_galaxy_registration( ) return BiocommonsRegisterData( email=registration.email, - user_metadata=BiocommonsUserMetadata(galaxy_username=registration.public_name), + username=registration.username, password=registration.password, email_verified=False, connection="Username-Password-Authentication", From 889794301ff3ff3c84661e140b223155282c8922 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:17:44 +1000 Subject: [PATCH 03/10] Update galaxy registration data model to use username/password requirements --- schemas/galaxy.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/schemas/galaxy.py b/schemas/galaxy.py index b70f454d..3f5eb640 100644 --- a/schemas/galaxy.py +++ b/schemas/galaxy.py @@ -1,13 +1,16 @@ -from typing import Annotated, Self +from typing import Self -from pydantic import BaseModel, EmailStr, StringConstraints, model_validator +from pydantic import BaseModel, EmailStr, model_validator + +from schemas.biocommons import BiocommonsPassword, BiocommonsUsername class GalaxyRegistrationData(BaseModel): email: EmailStr - password: str + # TODO: Update name of this field in frontend from + username: BiocommonsUsername + password: BiocommonsPassword password_confirmation: str - public_name: Annotated[str, StringConstraints(min_length=3, pattern=r"^[a-z0-9._-]+$")] @model_validator(mode='after') def check_passwords_match(self) -> Self: From a011ace89ffb466eac7a79ee4325a17df2cdc4f1 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:19:02 +1000 Subject: [PATCH 04/10] Update password/username handling in tests --- tests/test_galaxy.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_galaxy.py b/tests/test_galaxy.py index 24c0435f..21006150 100644 --- a/tests/test_galaxy.py +++ b/tests/test_galaxy.py @@ -35,9 +35,9 @@ def mock_auth_token(mocker): def test_galaxy_registration_data_password_match(): with pytest.raises(ValidationError, match="Passwords do not match"): GalaxyRegistrationData(email="user@example.com", - password="securepassword", - password_confirmation="insecurepassword", - public_name="valid_username") + password="SecurePassword123!", + password_confirmation="OtherPassword123!", + username="valid_username") def test_get_registration_token(test_client, mock_settings): @@ -71,18 +71,18 @@ def test_to_biocommons_register_data(): """ data = GalaxyRegistrationData( email="user@example.com", - password="securepassword", - password_confirmation="securepassword", - public_name="valid_username" + password="SecurePassword123!", + password_confirmation="SecurePassword123!", + username="valid_username" ) auth0_data = BiocommonsRegisterData.from_galaxy_registration(data) assert auth0_data.email == "user@example.com" - assert auth0_data.password == "securepassword" + assert auth0_data.password == "SecurePassword123!" assert auth0_data.connection == "Username-Password-Authentication" assert not auth0_data.email_verified - assert auth0_data.user_metadata.galaxy_username == "valid_username" + assert auth0_data.username == "valid_username" assert auth0_data.app_metadata.registration_from == "galaxy" @@ -94,14 +94,14 @@ def test_to_biocommons_register_data_empty_fields(): """ data = GalaxyRegistrationData( email="user@example.com", - password="securepassword", - password_confirmation="securepassword", - public_name="valid_username" + password="SecurePassword123!", + password_confirmation="SecurePassword123!", + username="valid_username" ) auth0_data = BiocommonsRegisterData.from_galaxy_registration(data) dumped = auth0_data.model_dump(mode="json", exclude_none=True) - assert "username" not in dumped + assert "user_metadata" not in dumped assert "name" not in dumped From 3f13cd6bebe86f4c858e8dc5ab307a6c0cd0d6e4 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:22:59 +1000 Subject: [PATCH 05/10] remove duplicate username field in BiocommonsRegisterData --- schemas/biocommons.py | 1 - 1 file changed, 1 deletion(-) diff --git a/schemas/biocommons.py b/schemas/biocommons.py index ca15365d..e7490c52 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -114,7 +114,6 @@ class BiocommonsRegisterData(BaseModel): connection: str = "Username-Password-Authentication" username: BiocommonsUsername name: Optional[str] = None - username: Optional[str] = None user_metadata: Optional[BiocommonsUserMetadata] = None app_metadata: BiocommonsAppMetadata From d2bac2b6a013d1ad96bf0f3585b53203e2066243 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Thu, 31 Jul 2025 15:24:03 +1000 Subject: [PATCH 06/10] Fix circular import --- schemas/biocommons.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/schemas/biocommons.py b/schemas/biocommons.py index e7490c52..78d9e099 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -10,8 +10,7 @@ from pydantic import BaseModel, EmailStr, Field, HttpUrl, StringConstraints -import schemas.bpa -import schemas.galaxy +import schemas from schemas import Resource, Service from schemas.service import Group, Identity From c8c7ab5a8a632f4a215dcdb1417a6d0726e41f7a Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 1 Aug 2025 15:10:18 +1000 Subject: [PATCH 07/10] Include /register in registration token URL so frontend doesn't try to get an access token for it --- routers/galaxy_register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/galaxy_register.py b/routers/galaxy_register.py index 5bca206f..cc0eeb09 100644 --- a/routers/galaxy_register.py +++ b/routers/galaxy_register.py @@ -19,7 +19,7 @@ ) -@router.get("/get-registration-token") +@router.get("/register/get-registration-token") async def get_registration_token(settings: Settings = Depends(get_settings)): return {"token": create_registration_token(settings)} From 8588cc6345e56c437c158b6ba6ffb8244428f9a9 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 1 Aug 2025 15:11:10 +1000 Subject: [PATCH 08/10] Add max_length to password --- schemas/biocommons.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schemas/biocommons.py b/schemas/biocommons.py index 78d9e099..ba4c1ab0 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -20,7 +20,7 @@ AppId = Literal["biocommons", "galaxy", "bpa"] BiocommonsUsername = Annotated[str, StringConstraints(min_length=3, max_length=100, pattern='^[-_a-z0-9]+$')] -BiocommonsPassword = Annotated[str, StringConstraints(min_length=8, pattern=VALID_PASSWORD_REGEX)] +BiocommonsPassword = Annotated[str, StringConstraints(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX)] class BPAMetadata(BaseModel): From 7bb856115f424342c4f552c27e12d5658b3fbd29 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 1 Aug 2025 15:17:28 +1000 Subject: [PATCH 09/10] Update tests with registration token URL --- tests/test_galaxy.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_galaxy.py b/tests/test_galaxy.py index 21006150..b5087341 100644 --- a/tests/test_galaxy.py +++ b/tests/test_galaxy.py @@ -44,7 +44,7 @@ def test_get_registration_token(test_client, mock_settings): """ Test get-registration-token endpoint returns a valid JWT token. """ - response = test_client.get("/galaxy/get-registration-token") + response = test_client.get("/galaxy/register/get-registration-token") assert response.status_code == 200 jwt.decode(response.json()["token"], mock_settings.jwt_secret_key, algorithms=mock_settings.auth0_algorithms) @@ -120,7 +120,7 @@ def test_register(mocker, mock_auth_token, mock_settings, test_client): mock_resp.status_code = 201 mock_post = mocker.patch("httpx.post", return_value=mock_resp) user_data = GalaxyRegistrationDataFactory.build() - token_resp = test_client.get("/galaxy/get-registration-token") + token_resp = test_client.get("/galaxy/register/get-registration-token") headers = {"registration-token": token_resp.json()["token"]} resp = test_client.post("/galaxy/register", json=user_data.model_dump(), headers=headers) assert resp.status_code == 200 @@ -152,7 +152,7 @@ def test_register_json_types(respx_mock, mock_auth_token, mock_settings, test_cl json=user.model_dump(mode="json")) ) user_data = GalaxyRegistrationDataFactory.build() - token_resp = test_client.get("/galaxy/get-registration-token") + token_resp = test_client.get("/galaxy/register/get-registration-token") headers = {"registration-token": token_resp.json()["token"]} mock_galaxy_client.username_exists.return_value = False resp = test_client.post("/galaxy/register", json=user_data.model_dump(), headers=headers) From 367cb1d67667a3418dad22fb1f76382622dcf9a6 Mon Sep 17 00:00:00 2001 From: marius-mather Date: Fri, 1 Aug 2025 16:29:01 +1000 Subject: [PATCH 10/10] Schema tests --- schemas/biocommons.py | 2 +- tests/schemas/test_biocommons_schemas.py | 55 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/schemas/test_biocommons_schemas.py diff --git a/schemas/biocommons.py b/schemas/biocommons.py index ba4c1ab0..6a62a623 100644 --- a/schemas/biocommons.py +++ b/schemas/biocommons.py @@ -19,7 +19,7 @@ VALID_PASSWORD_REGEX = re.compile(f"^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[{ALLOWED_SPECIAL_CHARS}]).{{8,}}$") AppId = Literal["biocommons", "galaxy", "bpa"] -BiocommonsUsername = Annotated[str, StringConstraints(min_length=3, max_length=100, pattern='^[-_a-z0-9]+$')] +BiocommonsUsername = Annotated[str, StringConstraints(min_length=3, max_length=128, pattern='^[-_a-z0-9]+$')] BiocommonsPassword = Annotated[str, StringConstraints(min_length=8, max_length=128, pattern=VALID_PASSWORD_REGEX)] diff --git a/tests/schemas/test_biocommons_schemas.py b/tests/schemas/test_biocommons_schemas.py new file mode 100644 index 00000000..fcce64d9 --- /dev/null +++ b/tests/schemas/test_biocommons_schemas.py @@ -0,0 +1,55 @@ +import pytest +from pydantic import TypeAdapter + +from schemas.biocommons import BiocommonsPassword, BiocommonsUsername + + +@pytest.mark.parametrize("password", [ + "V6Zs^B8E", + "k$M2FZa@", + "6*@sZ", + "Jd9sugcfjgWXY@Dzje^83!mcfM@A$YZ8be^bUhrBZ8s$KjbbNwAHr*bdiEhmLyMPyPowFU@rX4k8h5KCh#qm9bYS5RUmtjaLmVds", +]) +def test_valid_password(password: str): + password_adapter = TypeAdapter(BiocommonsPassword) + result = password_adapter.validate_python(password) + assert result == password + + + +@pytest.mark.parametrize("password", [ + # No lowercase + "ABCD1234!", + # No capital + "abcd1234!", + # Too short + "aB1!", + # No special character + "abCD1234" +]) +def test_invalid_password(password: str): + password_adapter = TypeAdapter(BiocommonsPassword) + with pytest.raises(ValueError): + password_adapter.validate_python(password) + + +@pytest.mark.parametrize("username", [ + "abc", + "a_c", + "user_n-ame" +]) +def test_valid_username(username: str): + username_adapter = TypeAdapter(BiocommonsUsername) + result = username_adapter.validate_python(username) + assert result == username + + +@pytest.mark.parametrize("username", [ + "ab", + "a.b", + "x" * 129 # Too long +]) +def test_invalid_username(username: str): + username_adapter = TypeAdapter(BiocommonsUsername) + with pytest.raises(ValueError): + username_adapter.validate_python(username)