Skip to content

Commit cfe3cf1

Browse files
feat: handle existing usernames and emails during registration
1 parent 1887e6b commit cfe3cf1

File tree

6 files changed

+264
-11
lines changed

6 files changed

+264
-11
lines changed

routers/biocommons_register.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,14 @@
1313
from db.models import BiocommonsGroup, BiocommonsUser
1414
from db.setup import get_db_session
1515
from routers.errors import RegistrationRoute
16+
from routers.utils import check_existing_user
1617
from schemas.biocommons import Auth0UserData, BiocommonsRegisterData
1718
from schemas.biocommons_register import BiocommonsRegistrationRequest
18-
from schemas.responses import RegistrationErrorResponse, RegistrationResponse
19+
from schemas.responses import (
20+
FieldError,
21+
RegistrationErrorResponse,
22+
RegistrationResponse,
23+
)
1924
from services.email_queue import enqueue_email
2025

2126
logger = logging.getLogger(__name__)
@@ -140,7 +145,29 @@ async def register_biocommons_user(
140145
logger.error(f"Auth0 registration failed: {e}")
141146
# Catch specific errors where possible and return a useful error message
142147
if e.response.status_code == 409:
143-
response = RegistrationErrorResponse(message="Username or email already in use")
148+
existing_field = check_existing_user(registration.username, registration.email, auth0_client)
149+
field_errors = []
150+
if existing_field == "username":
151+
field_errors.append(FieldError(field="username", message="Username is already taken"))
152+
response = RegistrationErrorResponse(
153+
message="Username is already taken",
154+
field_errors=field_errors
155+
)
156+
elif existing_field == "email":
157+
field_errors.append(FieldError(field="email", message="Email is already taken"))
158+
response = RegistrationErrorResponse(
159+
message="Email is already taken",
160+
field_errors=field_errors
161+
)
162+
elif existing_field == "both":
163+
field_errors.append(FieldError(field="username", message="Username is already taken"))
164+
field_errors.append(FieldError(field="email", message="Email is already taken"))
165+
response = RegistrationErrorResponse(
166+
message="Username and email are already taken",
167+
field_errors=field_errors
168+
)
169+
else:
170+
response = RegistrationErrorResponse(message="Username or email is already taken")
144171
else:
145172
response = RegistrationErrorResponse(message=f"Auth0 error: {str(e.response.text)}")
146173
return JSONResponse(status_code=400, content=response.model_dump(mode="json"))

routers/sbp_register.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@
1111
from db.models import BiocommonsUser, PlatformEnum
1212
from db.setup import get_db_session
1313
from routers.errors import RegistrationRoute
14+
from routers.utils import check_existing_user
1415
from schemas.biocommons import Auth0UserData, BiocommonsRegisterData
15-
from schemas.responses import RegistrationErrorResponse, RegistrationResponse
16+
from schemas.responses import (
17+
FieldError,
18+
RegistrationErrorResponse,
19+
RegistrationResponse,
20+
)
1621
from schemas.sbp import SBPRegistrationRequest
1722
from services.email_queue import enqueue_email
1823

@@ -106,7 +111,29 @@ async def register_sbp_user(
106111
except HTTPStatusError as e:
107112
# Catch specific errors where possible and return a useful error message
108113
if e.response.status_code == 409:
109-
response = RegistrationErrorResponse(message="Username or email already in use")
114+
existing_field = check_existing_user(registration.username, registration.email, auth0_client)
115+
field_errors = []
116+
if existing_field == "username":
117+
field_errors.append(FieldError(field="username", message="Username is already taken"))
118+
response = RegistrationErrorResponse(
119+
message="Username is already taken",
120+
field_errors=field_errors
121+
)
122+
elif existing_field == "email":
123+
field_errors.append(FieldError(field="email", message="Email is already taken"))
124+
response = RegistrationErrorResponse(
125+
message="Email is already taken",
126+
field_errors=field_errors
127+
)
128+
elif existing_field == "both":
129+
field_errors.append(FieldError(field="username", message="Username is already taken"))
130+
field_errors.append(FieldError(field="email", message="Email is already taken"))
131+
response = RegistrationErrorResponse(
132+
message="Username and email are already taken",
133+
field_errors=field_errors
134+
)
135+
else:
136+
response = RegistrationErrorResponse(message="Username or email is already taken")
110137
else:
111138
response = RegistrationErrorResponse(message=f"Auth0 error: {str(e.response.text)}")
112139
return JSONResponse(status_code=400, content=response.model_dump(mode="json"))

routers/utils.py

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,113 @@
1+
import logging
12
from typing import Annotated
23

3-
from fastapi import APIRouter
4+
from fastapi import APIRouter, Query
45
from fastapi.params import Depends
56
from pydantic import BaseModel
67

78
from auth0.client import Auth0Client, get_auth0_client
89
from schemas.biocommons import AppId
10+
from schemas.responses import FieldError
11+
12+
logger = logging.getLogger(__name__)
913

1014
router = APIRouter(prefix="/utils", tags=["utils"])
1115

1216

17+
def _check_username_exists(username: str, auth0_client: Auth0Client) -> bool:
18+
"""Helper function to check if a username exists in Auth0."""
19+
try:
20+
username_query = f'username:"{username}"'
21+
username_results = auth0_client.get_users(q=username_query)
22+
if isinstance(username_results, list):
23+
return len(username_results) > 0
24+
else:
25+
return username_results.total > 0
26+
except Exception as e:
27+
logger.warning(f"Error checking username existence: {e}")
28+
return False
29+
30+
31+
def _check_email_exists(email: str, auth0_client: Auth0Client) -> bool:
32+
"""Helper function to check if an email exists in Auth0."""
33+
try:
34+
email_results = auth0_client.search_users_by_email(email)
35+
return len(email_results) > 0
36+
except Exception as e:
37+
logger.warning(f"Error checking email existence: {e}")
38+
return False
39+
40+
41+
def check_existing_user(username: str, email: str, auth0_client: Auth0Client) -> str | None:
42+
"""Check if username or email already exists in Auth0.
43+
44+
Returns:
45+
- "username" if username exists
46+
- "email" if email exists
47+
- "both" if both exist
48+
- None if neither exists
49+
"""
50+
username_exists = _check_username_exists(username, auth0_client)
51+
email_exists = _check_email_exists(email, auth0_client)
52+
53+
if username_exists and email_exists:
54+
return "both"
55+
elif username_exists:
56+
return "username"
57+
elif email_exists:
58+
return "email"
59+
return None
60+
61+
1362
class RegistrationInfo(BaseModel):
1463
app: AppId = "biocommons"
1564

1665

17-
@router.get("/registration_info")
66+
class AvailabilityResponse(BaseModel):
67+
"""Response for checking username/email availability"""
68+
available: bool
69+
field_errors: list[FieldError] = []
70+
71+
72+
@router.get("/check-username-availability", response_model=AvailabilityResponse)
73+
async def check_username_availability(
74+
username: Annotated[str, Query(min_length=3, max_length=128)],
75+
auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)],
76+
):
77+
"""
78+
Check if a username is available for registration.
79+
80+
Returns availability status with field errors if already taken.
81+
"""
82+
exists = _check_username_exists(username, auth0_client)
83+
if exists:
84+
return AvailabilityResponse(
85+
available=False,
86+
field_errors=[FieldError(field="username", message="Username is already taken")]
87+
)
88+
return AvailabilityResponse(available=True)
89+
90+
91+
@router.get("/check-email-availability", response_model=AvailabilityResponse)
92+
async def check_email_availability(
93+
email: Annotated[str, Query()],
94+
auth0_client: Annotated[Auth0Client, Depends(get_auth0_client)],
95+
):
96+
"""
97+
Check if an email is available for registration.
98+
99+
Returns availability status with field errors if already registered.
100+
"""
101+
exists = _check_email_exists(email, auth0_client)
102+
if exists:
103+
return AvailabilityResponse(
104+
available=False,
105+
field_errors=[FieldError(field="email", message="Email is already taken")]
106+
)
107+
return AvailabilityResponse(available=True)
108+
109+
110+
@router.get("/registration-info")
18111
async def get_registration_info(
19112
user_email: str,
20113
client: Annotated[Auth0Client, Depends(get_auth0_client)]):

tests/test_biocommons_register.py

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,69 @@ def test_biocommons_registration_auth0_conflict_error(
314314
"User already exists", request=request, response=response
315315
)
316316

317+
mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build(username="existinguser")]
318+
mock_auth0_client.search_users_by_email.return_value = []
319+
320+
registration_data = {
321+
"first_name": "Test",
322+
"last_name": "User",
323+
"email": "existing@example.com",
324+
"username": "existinguser",
325+
"password": "StrongPass1!",
326+
"bundle": "tsi",
327+
}
328+
329+
response = test_client.post("/biocommons/register", json=registration_data)
330+
331+
assert response.status_code == 400
332+
assert response.json()["message"] == "Username is already taken"
333+
334+
335+
def test_biocommons_registration_email_conflict_error(
336+
test_client, tsi_group, mock_auth0_client, test_db_session
337+
):
338+
"""Test handling of Auth0 conflict error when email exists"""
339+
from httpx import HTTPStatusError, Request, Response
340+
341+
response = Response(409, json={"error": "user_exists"})
342+
request = Request("POST", "https://example.com")
343+
mock_auth0_client.create_user.side_effect = HTTPStatusError(
344+
"User already exists", request=request, response=response
345+
)
346+
347+
mock_auth0_client.get_users.return_value = []
348+
mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
349+
350+
registration_data = {
351+
"first_name": "Test",
352+
"last_name": "User",
353+
"email": "existing@example.com",
354+
"username": "newuser",
355+
"password": "StrongPass1!",
356+
"bundle": "tsi",
357+
}
358+
359+
response = test_client.post("/biocommons/register", json=registration_data)
360+
361+
assert response.status_code == 400
362+
assert response.json()["message"] == "Email is already taken"
363+
364+
365+
def test_biocommons_registration_both_conflict_error(
366+
test_client, tsi_group, mock_auth0_client, test_db_session
367+
):
368+
"""Test handling of Auth0 conflict error when both username and email exist"""
369+
from httpx import HTTPStatusError, Request, Response
370+
371+
response = Response(409, json={"error": "user_exists"})
372+
request = Request("POST", "https://example.com")
373+
mock_auth0_client.create_user.side_effect = HTTPStatusError(
374+
"User already exists", request=request, response=response
375+
)
376+
377+
mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build()]
378+
mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
379+
317380
registration_data = {
318381
"first_name": "Test",
319382
"last_name": "User",
@@ -326,7 +389,7 @@ def test_biocommons_registration_auth0_conflict_error(
326389
response = test_client.post("/biocommons/register", json=registration_data)
327390

328391
assert response.status_code == 400
329-
assert response.json()["message"] == "Username or email already in use"
392+
assert response.json()["message"] == "Username and email are already taken"
330393

331394

332395
def test_biocommons_registration_missing_group_error(test_client, mock_auth0_client):

tests/test_sbp_register.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,53 @@ def test_registration_duplicate_user(
139139
)
140140
mock_auth0_client.create_user.side_effect = error
141141

142+
mock_auth0_client.get_users.return_value = []
143+
mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
144+
145+
response = test_client.post("/sbp/register", json=valid_registration_data)
146+
147+
assert response.status_code == 400
148+
assert response.json()["message"] == "Email is already taken"
149+
150+
151+
def test_registration_duplicate_username(
152+
test_client, valid_registration_data, mock_auth0_client
153+
):
154+
"""Test that duplicate username returns specific error message"""
155+
error = httpx.HTTPStatusError(
156+
"User already exists",
157+
request=httpx.Request("POST", "https://api.example.com/data"),
158+
response=httpx.Response(409, text="User already exists"),
159+
)
160+
mock_auth0_client.create_user.side_effect = error
161+
162+
mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build()]
163+
mock_auth0_client.search_users_by_email.return_value = []
164+
165+
response = test_client.post("/sbp/register", json=valid_registration_data)
166+
167+
assert response.status_code == 400
168+
assert response.json()["message"] == "Username is already taken"
169+
170+
171+
def test_registration_duplicate_both(
172+
test_client, valid_registration_data, mock_auth0_client
173+
):
174+
"""Test that duplicate username and email returns specific error message"""
175+
error = httpx.HTTPStatusError(
176+
"User already exists",
177+
request=httpx.Request("POST", "https://api.example.com/data"),
178+
response=httpx.Response(409, text="User already exists"),
179+
)
180+
mock_auth0_client.create_user.side_effect = error
181+
182+
mock_auth0_client.get_users.return_value = [Auth0UserDataFactory.build()]
183+
mock_auth0_client.search_users_by_email.return_value = [Auth0UserDataFactory.build()]
184+
142185
response = test_client.post("/sbp/register", json=valid_registration_data)
143186

144187
assert response.status_code == 400
145-
assert response.json()["message"] == "Username or email already in use"
188+
assert response.json()["message"] == "Username and email are already taken"
146189

147190

148191
def test_registration_auth0_error(

tests/test_utils.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def test_get_registration_info(override_auth0_client, test_client):
2525
user = Auth0UserDataFactory.build(email="user@example.com",
2626
app_metadata=app_metadata)
2727
override_auth0_client.search_users_by_email.return_value = [user]
28-
resp = test_client.get("/utils/registration_info", params={"user_email": user.email})
28+
resp = test_client.get("/utils/registration-info", params={"user_email": user.email})
2929
assert resp.status_code == 200
3030
data = resp.json()
3131
assert data["app"] == "galaxy"
@@ -39,7 +39,7 @@ def test_get_registration_info_no_registration_from(override_auth0_client, test_
3939
user = Auth0UserDataFactory.build(email="user@example.com",
4040
app_metadata=app_metadata)
4141
override_auth0_client.search_users_by_email.return_value = [user]
42-
resp = test_client.get("/utils/registration_info", params={"user_email": user.email})
42+
resp = test_client.get("/utils/registration-info", params={"user_email": user.email})
4343
assert resp.status_code == 200
4444
data = resp.json()
4545
assert data["app"] == "biocommons"
@@ -52,7 +52,7 @@ def test_get_registration_info_no_user(override_auth0_client, test_client):
5252
exists or not)
5353
"""
5454
override_auth0_client.search_users_by_email.return_value = []
55-
resp = test_client.get("/utils/registration_info", params={"user_email": "notfound@example.com"})
55+
resp = test_client.get("/utils/registration-info", params={"user_email": "notfound@example.com"})
5656
assert resp.status_code == 200
5757
data = resp.json()
5858
assert data["app"] == "biocommons"

0 commit comments

Comments
 (0)