Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions routers/biocommons_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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"))
Expand Down
31 changes: 29 additions & 2 deletions routers/sbp_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"))
Expand Down
104 changes: 101 additions & 3 deletions routers/utils.py
Original file line number Diff line number Diff line change
@@ -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)]):
Expand Down
65 changes: 64 additions & 1 deletion tests/test_biocommons_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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):
Expand Down
45 changes: 44 additions & 1 deletion tests/test_sbp_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
25 changes: 22 additions & 3 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"] == []