Merge pull request #135 from AustralianBioCommons/email-username-check

feat: handle existing usernames and emails during registration (AAI-518)

Author: minh-biocommons

routers/biocommons_register.py

@@ -13,9 +13,14 @@
 from db.models import BiocommonsGroup, BiocommonsUser
 from db.setup import get_db_session
 from routers.errors import RegistrationRoute
+from routers.utils import check_existing_user
 from schemas.biocommons import Auth0UserData, BiocommonsRegisterData
 from schemas.biocommons_register import BiocommonsRegistrationRequest
-from schemas.responses import RegistrationErrorResponse, RegistrationResponse
+from schemas.responses import (
+    FieldError,
+    RegistrationErrorResponse,
+    RegistrationResponse,
+)
 from services.email_queue import enqueue_email
 
 logger = logging.getLogger(__name__)
@@ -140,7 +145,29 @@ async def register_biocommons_user(
         logger.error(f"Auth0 registration failed: {e}")
         # Catch specific errors where possible and return a useful error message
         if e.response.status_code == 409:
-            response = RegistrationErrorResponse(message="Username or email already in use")
+            existing_field = check_existing_user(registration.username, registration.email, auth0_client)
+            field_errors = []
+            if existing_field == "username":
+                field_errors.append(FieldError(field="username", message="Username is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Username is already taken",
+                    field_errors=field_errors
+                )
+            elif existing_field == "email":
+                field_errors.append(FieldError(field="email", message="Email is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Email is already taken",
+                    field_errors=field_errors
+                )
+            elif existing_field == "both":
+                field_errors.append(FieldError(field="username", message="Username is already taken"))
+                field_errors.append(FieldError(field="email", message="Email is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Username and email are already taken",
+                    field_errors=field_errors
+                )
+            else:
+                response = RegistrationErrorResponse(message="Username or email is already taken")
         else:
             response = RegistrationErrorResponse(message=f"Auth0 error: {str(e.response.text)}")
         return JSONResponse(status_code=400, content=response.model_dump(mode="json"))

routers/sbp_register.py

@@ -11,8 +11,13 @@
 from db.models import BiocommonsUser, PlatformEnum
 from db.setup import get_db_session
 from routers.errors import RegistrationRoute
+from routers.utils import check_existing_user
 from schemas.biocommons import Auth0UserData, BiocommonsRegisterData
-from schemas.responses import RegistrationErrorResponse, RegistrationResponse
+from schemas.responses import (
+    FieldError,
+    RegistrationErrorResponse,
+    RegistrationResponse,
+)
 from schemas.sbp import SBPRegistrationRequest
 from services.email_queue import enqueue_email
 
@@ -106,7 +111,29 @@ async def register_sbp_user(
     except HTTPStatusError as e:
         # Catch specific errors where possible and return a useful error message
         if e.response.status_code == 409:
-            response = RegistrationErrorResponse(message="Username or email already in use")
+            existing_field = check_existing_user(registration.username, registration.email, auth0_client)
+            field_errors = []
+            if existing_field == "username":
+                field_errors.append(FieldError(field="username", message="Username is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Username is already taken",
+                    field_errors=field_errors
+                )
+            elif existing_field == "email":
+                field_errors.append(FieldError(field="email", message="Email is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Email is already taken",
+                    field_errors=field_errors
+                )
+            elif existing_field == "both":
+                field_errors.append(FieldError(field="username", message="Username is already taken"))
+                field_errors.append(FieldError(field="email", message="Email is already taken"))
+                response = RegistrationErrorResponse(
+                    message="Username and email are already taken",
+                    field_errors=field_errors
+                )
+            else:
+                response = RegistrationErrorResponse(message="Username or email is already taken")
         else:
             response = RegistrationErrorResponse(message=f"Auth0 error: {str(e.response.text)}")
         return JSONResponse(status_code=400, content=response.model_dump(mode="json"))

routers/utils.py

@@ -1,20 +1,118 @@
-from typing import Annotated
+import logging
+from typing import Annotated, Literal
 
-from fastapi import APIRouter
+from fastapi import APIRouter, Query
 from fastapi.params import Depends
 from pydantic import BaseModel
 
 from auth0.client import Auth0Client, get_auth0_client
 from schemas.biocommons import AppId
+from schemas.responses import FieldError
+
+logger = logging.getLogger(__name__)
 
 router = APIRouter(prefix="/utils", tags=["utils"])
 
 
+def _check_username_exists(username: str, auth0_client: Auth0Client) -> bool:
+    """Helper function to check if a username exists in Auth0."""
+    try:
+        q = f'username:"{username}"'
+        res = auth0_client.get_users(q=q)
+
+        users = res if isinstance(res, list) else getattr(res, "users", [])
+        target = username.lower()
+
+        return any(
+            getattr(u, "username", "").lower() == target
+            for u in users
+            if getattr(u, "username", None)
+        )
+    except Exception as e:
+        logger.warning(f"Error checking username existence: {e}")
+        return False
+
+
+def _check_email_exists(email: str, auth0_client: Auth0Client) -> bool:
+    """Helper function to check if an email exists in Auth0."""
+    try:
+        email_results = auth0_client.search_users_by_email(email)
+        return len(email_results) > 0
+    except Exception as e:
+        logger.warning(f"Error checking email existence: {e}")
+        return False
+
+
+def check_existing_user(username: str, email: str, auth0_client: Auth0Client) -> Literal["both", "email", "username"] | None:
+    """Check if username or email already exists in Auth0.
+
+    Returns:
+        - "username" if username exists
+        - "email" if email exists
+        - "both" if both exist
+        - None if neither exists
+    """
+    username_exists = _check_username_exists(username, auth0_client)
+    email_exists = _check_email_exists(email, auth0_client)
+
+    if username_exists and email_exists:
+        return "both"
+    elif username_exists:
+        return "username"
+    elif email_exists:
+        return "email"
+    return None
+
+
 class RegistrationInfo(BaseModel):
     app: AppId = "biocommons"
 
 
-@router.get("/registration_info")
+class AvailabilityResponse(BaseModel):
+    """Response for checking username/email availability"""
+    available: bool
+    field_errors: list[FieldError] = []
+
+
+@router.get("/register/check-username-availability", response_model=AvailabilityResponse)
+async def check_username_availability(
+    username: Annotated[str, Query(min_length=3, max_length=128)],
+    auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)],
+):
+    """
+    Check if a username is available for registration.
+
+    Returns availability status with field errors if already taken.
+    """
+    exists = _check_username_exists(username, auth0_client)
+    if exists:
+        return AvailabilityResponse(
+            available=False,
+            field_errors=[FieldError(field="username", message="Username is already taken")]
+        )
+    return AvailabilityResponse(available=True)
+
+
+@router.get("/register/check-email-availability", response_model=AvailabilityResponse)
+async def check_email_availability(
+    email: Annotated[str, Query()],
+    auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)],
+):
+    """
+    Check if an email is available for registration.
+
+    Returns availability status with field errors if already registered.
+    """
+    exists = _check_email_exists(email, auth0_client)
+    if exists:
+        return AvailabilityResponse(
+            available=False,
+            field_errors=[FieldError(field="email", message="Email is already taken")]
+        )
+    return AvailabilityResponse(available=True)
+
+
+@router.get("/register/registration-info")
 async def get_registration_info(
         user_email: str,
         client: Annotated[Auth0Client, Depends(get_auth0_client)]):

tests/test_biocommons_register.py

@@ -314,6 +314,69 @@ def test_biocommons_registration_auth0_conflict_error(
         "User already exists", request=request, response=response
     )
 
+    mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build(username="existinguser")]
+    mock_auth0_client.search_users_by_email.return_value = []
+
+    registration_data = {
+        "first_name": "Test",
+        "last_name": "User",
+        "email": "existing@example.com",
+        "username": "existinguser",
+        "password": "StrongPass1!",
+        "bundle": "tsi",
+    }
+
+    response = test_client.post("/biocommons/register", json=registration_data)
+
+    assert response.status_code == 400
+    assert response.json()["message"] == "Username is already taken"
+
+
+def test_biocommons_registration_email_conflict_error(
+    test_client, tsi_group, mock_auth0_client, test_db_session
+):
+    """Test handling of Auth0 conflict error when email exists"""
+    from httpx import HTTPStatusError, Request, Response
+
+    response = Response(409, json={"error": "user_exists"})
+    request = Request("POST", "https://example.com")
+    mock_auth0_client.create_user.side_effect = HTTPStatusError(
+        "User already exists", request=request, response=response
+    )
+
+    mock_auth0_client.get_users.return_value = []
+    mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
+
+    registration_data = {
+        "first_name": "Test",
+        "last_name": "User",
+        "email": "existing@example.com",
+        "username": "newuser",
+        "password": "StrongPass1!",
+        "bundle": "tsi",
+    }
+
+    response = test_client.post("/biocommons/register", json=registration_data)
+
+    assert response.status_code == 400
+    assert response.json()["message"] == "Email is already taken"
+
+
+def test_biocommons_registration_both_conflict_error(
+    test_client, tsi_group, mock_auth0_client, test_db_session
+):
+    """Test handling of Auth0 conflict error when both username and email exist"""
+    from httpx import HTTPStatusError, Request, Response
+
+    response = Response(409, json={"error": "user_exists"})
+    request = Request("POST", "https://example.com")
+    mock_auth0_client.create_user.side_effect = HTTPStatusError(
+        "User already exists", request=request, response=response
+    )
+
+    mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build(username="existinguser")]
+    mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build(email="existing@example.com")]
+
     registration_data = {
         "first_name": "Test",
         "last_name": "User",
@@ -326,7 +389,7 @@ def test_biocommons_registration_auth0_conflict_error(
     response = test_client.post("/biocommons/register", json=registration_data)
 
     assert response.status_code == 400
-    assert response.json()["message"] == "Username or email already in use"
+    assert response.json()["message"] == "Username and email are already taken"
 
 
 def test_biocommons_registration_missing_group_error(test_client, mock_auth0_client):

tests/test_sbp_register.py

@@ -139,10 +139,53 @@ def test_registration_duplicate_user(
     )
     mock_auth0_client.create_user.side_effect = error
 
+    mock_auth0_client.get_users.return_value = []
+    mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
+
+    response = test_client.post("/sbp/register", json=valid_registration_data)
+
+    assert response.status_code == 400
+    assert response.json()["message"] == "Email is already taken"
+
+
+def test_registration_duplicate_username(
+    test_client, valid_registration_data, mock_auth0_client
+):
+    """Test that duplicate username returns specific error message"""
+    error = httpx.HTTPStatusError(
+        "User already exists",
+        request=httpx.Request("POST", "https://api.example.com/data"),
+        response=httpx.Response(409, text="User already exists"),
+    )
+    mock_auth0_client.create_user.side_effect = error
+
+    mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build(username=valid_registration_data["username"])]
+    mock_auth0_client.search_users_by_email.return_value = []
+
+    response = test_client.post("/sbp/register", json=valid_registration_data)
+
+    assert response.status_code == 400
+    assert response.json()["message"] == "Username is already taken"
+
+
+def test_registration_duplicate_both(
+    test_client, valid_registration_data, mock_auth0_client
+):
+    """Test that duplicate username and email returns specific error message"""
+    error = httpx.HTTPStatusError(
+        "User already exists",
+        request=httpx.Request("POST", "https://api.example.com/data"),
+        response=httpx.Response(409, text="User already exists"),
+    )
+    mock_auth0_client.create_user.side_effect = error
+
+    mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build(username=valid_registration_data["username"])]
+    mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build(email=valid_registration_data["email"])]
+
     response = test_client.post("/sbp/register", json=valid_registration_data)
 
     assert response.status_code == 400
-    assert response.json()["message"] == "Username or email already in use"
+    assert response.json()["message"] == "Username and email are already taken"
 
 
 def test_registration_auth0_error(

tests/test_utils.py

@@ -25,7 +25,7 @@ def test_get_registration_info(override_auth0_client, test_client):
     user = Auth0UserDataFactory.build(email="user@example.com",
                                       app_metadata=app_metadata)
     override_auth0_client.search_users_by_email.return_value = [user]
-    resp = test_client.get("/utils/registration_info", params={"user_email": user.email})
+    resp = test_client.get("/utils/register/registration-info", params={"user_email": user.email})
     assert resp.status_code == 200
     data = resp.json()
     assert data["app"] == "galaxy"
@@ -39,7 +39,7 @@ def test_get_registration_info_no_registration_from(override_auth0_client, test_
     user = Auth0UserDataFactory.build(email="user@example.com",
                                       app_metadata=app_metadata)
     override_auth0_client.search_users_by_email.return_value = [user]
-    resp = test_client.get("/utils/registration_info", params={"user_email": user.email})
+    resp = test_client.get("/utils/register/registration-info", params={"user_email": user.email})
     assert resp.status_code == 200
     data = resp.json()
     assert data["app"] == "biocommons"
@@ -52,7 +52,26 @@ def test_get_registration_info_no_user(override_auth0_client, test_client):
     exists or not)
     """
     override_auth0_client.search_users_by_email.return_value = []
-    resp = test_client.get("/utils/registration_info", params={"user_email": "notfound@example.com"})
+    resp = test_client.get("/utils/register/registration-info", params={"user_email": "notfound@example.com"})
     assert resp.status_code == 200
     data = resp.json()
     assert data["app"] == "biocommons"
+
+
+def test_check_username_exists_exact_match_only(override_auth0_client, test_client):
+    """Test that username check only matches exact usernames, not partial matches"""
+    user1 = Auth0UserDataFactory.build(username="testuser")
+    user2 = Auth0UserDataFactory.build(username="testuser123")
+    override_auth0_client.get_users.return_value = [user1, user2]
+
+    resp = test_client.get("/utils/register/check-username-availability", params={"username": "testuser"})
+    assert resp.status_code == 200
+    data = resp.json()
+    assert data["available"] is False
+    assert data["field_errors"][0]["message"] == "Username is already taken"
+
+    resp = test_client.get("/utils/register/check-username-availability", params={"username": "testuser99"})
+    assert resp.status_code == 200
+    data = resp.json()
+    assert data["available"] is True
+    assert data["field_errors"] == []