diff --git a/backend/app/api/routes/login.py b/backend/app/api/routes/login.py index 87b04d11a7..58441e37e9 100644 --- a/backend/app/api/routes/login.py +++ b/backend/app/api/routes/login.py @@ -57,21 +57,21 @@ def recover_password(email: str, session: SessionDep) -> Message: """ user = crud.get_user_by_email(session=session, email=email) - if not user: - raise HTTPException( - status_code=404, - detail="The user with this email does not exist in the system.", + # Always return the same response to prevent email enumeration attacks + # Only send email if user actually exists + if user: + password_reset_token = generate_password_reset_token(email=email) + email_data = generate_reset_password_email( + email_to=user.email, email=email, token=password_reset_token ) - password_reset_token = generate_password_reset_token(email=email) - email_data = generate_reset_password_email( - email_to=user.email, email=email, token=password_reset_token - ) - send_email( - email_to=user.email, - subject=email_data.subject, - html_content=email_data.html_content, + send_email( + email_to=user.email, + subject=email_data.subject, + html_content=email_data.html_content, + ) + return Message( + message="If that email is registered, we sent a password recovery link" ) - return Message(message="Password recovery email sent") @router.post("/reset-password/") @@ -84,10 +84,8 @@ def reset_password(session: SessionDep, body: NewPassword) -> Message: raise HTTPException(status_code=400, detail="Invalid token") user = crud.get_user_by_email(session=session, email=email) if not user: - raise HTTPException( - status_code=404, - detail="The user with this email does not exist in the system.", - ) + # Don't reveal that the user doesn't exist - use same error as invalid token + raise HTTPException(status_code=400, detail="Invalid token") elif not user.is_active: raise HTTPException(status_code=400, detail="Inactive user") user_in_update = UserUpdate(password=body.new_password) diff --git a/backend/app/crud.py b/backend/app/crud.py index 4b026b7c41..a8ceba6444 100644 --- a/backend/app/crud.py +++ b/backend/app/crud.py @@ -37,9 +37,17 @@ def get_user_by_email(*, session: Session, email: str) -> User | None: return session_user +# Dummy hash to use for timing attack prevention when user is not found +# This is an Argon2 hash of a random password, used to ensure constant-time comparison +DUMMY_HASH = "$argon2id$v=19$m=65536,t=3,p=4$MjQyZWE1MzBjYjJlZTI0Yw$YTU4NGM5ZTZmYjE2NzZlZjY0ZWY3ZGRkY2U2OWFjNjk" + + def authenticate(*, session: Session, email: str, password: str) -> User | None: db_user = get_user_by_email(session=session, email=email) if not db_user: + # Prevent timing attacks by running password verification even when user doesn't exist + # This ensures the response time is similar whether or not the email exists + verify_password(password, DUMMY_HASH) return None verified, updated_password_hash = verify_password(password, db_user.hashed_password) if not verified: diff --git a/backend/tests/api/routes/test_login.py b/backend/tests/api/routes/test_login.py index 98d4dd8e97..96677a25f6 100644 --- a/backend/tests/api/routes/test_login.py +++ b/backend/tests/api/routes/test_login.py @@ -59,7 +59,9 @@ def test_recovery_password( headers=normal_user_token_headers, ) assert r.status_code == 200 - assert r.json() == {"message": "Password recovery email sent"} + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } def test_recovery_password_user_not_exits( @@ -70,7 +72,11 @@ def test_recovery_password_user_not_exits( f"{settings.API_V1_STR}/password-recovery/{email}", headers=normal_user_token_headers, ) - assert r.status_code == 404 + # Should return 200 with generic message to prevent email enumeration attacks + assert r.status_code == 200 + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } def test_reset_password(client: TestClient, db: Session) -> None: