diff --git a/backend/app/api/routes/login.py b/backend/app/api/routes/login.py index 58441e37e9..d5fb9237c2 100644 --- a/backend/app/api/routes/login.py +++ b/backend/app/api/routes/login.py @@ -1,3 +1,4 @@ +import logging from datetime import timedelta from typing import Annotated, Any @@ -18,6 +19,7 @@ ) router = APIRouter(tags=["login"]) +logger = logging.getLogger(__name__) @router.post("/login/access-token") @@ -55,23 +57,29 @@ def recover_password(email: str, session: SessionDep) -> Message: """ Password Recovery """ - user = crud.get_user_by_email(session=session, email=email) - - # 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 - ) - send_email( - email_to=user.email, - subject=email_data.subject, - html_content=email_data.html_content, - ) - return Message( + # Always return the same response to prevent email enumeration attacks. + recovery_message = Message( message="If that email is registered, we sent a password recovery link" ) + if not settings.emails_enabled: + logger.warning("Password recovery requested, but email delivery is disabled") + return recovery_message + + user = crud.get_user_by_email(session=session, email=email) + if user: + try: + 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, + ) + except Exception: + return recovery_message + return recovery_message @router.post("/reset-password/") diff --git a/backend/tests/api/routes/test_login.py b/backend/tests/api/routes/test_login.py index 96677a25f6..16c2050fc5 100644 --- a/backend/tests/api/routes/test_login.py +++ b/backend/tests/api/routes/test_login.py @@ -1,5 +1,8 @@ +import logging +from types import SimpleNamespace from unittest.mock import patch +import pytest from fastapi.testclient import TestClient from pwdlib.hashers.bcrypt import BcryptHasher from sqlmodel import Session @@ -79,6 +82,162 @@ def test_recovery_password_user_not_exits( } +def test_recovery_password_with_disabled_email_skips_user_lookup_and_email( + client: TestClient, + caplog: pytest.LogCaptureFixture, +) -> None: + email = random_email() + + with caplog.at_level(logging.WARNING, logger="app.api.routes.login"): + with ( + patch("app.api.routes.login.crud.get_user_by_email") as get_user_mock, + patch("app.core.config.settings.SMTP_HOST", None), + patch("app.core.config.settings.EMAILS_FROM_EMAIL", None), + patch("app.api.routes.login.generate_password_reset_token") as token_mock, + patch("app.api.routes.login.generate_reset_password_email") as email_mock, + patch("app.api.routes.login.send_email") as send_email_mock, + ): + r = client.post(f"{settings.API_V1_STR}/password-recovery/{email}") + + assert r.status_code == 200 + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } + get_user_mock.assert_not_called() + token_mock.assert_not_called() + email_mock.assert_not_called() + send_email_mock.assert_not_called() + assert "email delivery is disabled" in caplog.text + assert email not in caplog.text + + +def test_recovery_password_with_existing_user_and_email_enabled( + client: TestClient, +) -> None: + email = random_email() + user = User( + email=email, + hashed_password="not-used", + full_name="Test User", + is_active=True, + is_superuser=False, + ) + + email_data = SimpleNamespace( + subject="Reset password", + html_content="

reset-password

", + ) + + with ( + patch("app.api.routes.login.crud.get_user_by_email", return_value=user), + patch("app.core.config.settings.SMTP_HOST", "smtp.example.com"), + patch("app.core.config.settings.EMAILS_FROM_EMAIL", "test@example.com"), + patch( + "app.api.routes.login.generate_password_reset_token", + return_value="reset-token", + ) as token_mock, + patch( + "app.api.routes.login.generate_reset_password_email", + return_value=email_data, + ) as email_mock, + patch("app.api.routes.login.send_email") as send_email_mock, + ): + r = client.post(f"{settings.API_V1_STR}/password-recovery/{email}") + + assert r.status_code == 200 + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } + token_mock.assert_called_once_with(email=email) + email_mock.assert_called_once_with( + email_to=user.email, + email=email, + token="reset-token", + ) + send_email_mock.assert_called_once_with( + email_to=user.email, + subject=email_data.subject, + html_content=email_data.html_content, + ) + + +def test_recovery_password_with_email_failure_returns_generic_response( + client: TestClient, + caplog: pytest.LogCaptureFixture, +) -> None: + email = random_email() + user = User( + email=email, + hashed_password="not-used", + full_name="Test User", + is_active=True, + is_superuser=False, + ) + + with caplog.at_level(logging.ERROR, logger="app.api.routes.login"): + with ( + patch("app.api.routes.login.crud.get_user_by_email", return_value=user), + patch("app.core.config.settings.SMTP_HOST", "smtp.example.com"), + patch("app.core.config.settings.EMAILS_FROM_EMAIL", "test@example.com"), + patch( + "app.api.routes.login.send_email", + side_effect=RuntimeError("SMTP unavailable"), + ) as send_email_mock, + ): + r = client.post(f"{settings.API_V1_STR}/password-recovery/{email}") + + assert r.status_code == 200 + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } + send_email_mock.assert_called_once() + assert caplog.text == "" + + +def test_recovery_password_with_email_template_failure_returns_generic_response( + client: TestClient, + caplog: pytest.LogCaptureFixture, +) -> None: + email = random_email() + user = User( + email=email, + hashed_password="not-used", + full_name="Test User", + is_active=True, + is_superuser=False, + ) + + with caplog.at_level(logging.ERROR, logger="app.api.routes.login"): + with ( + patch("app.api.routes.login.crud.get_user_by_email", return_value=user), + patch("app.core.config.settings.SMTP_HOST", "smtp.example.com"), + patch("app.core.config.settings.EMAILS_FROM_EMAIL", "test@example.com"), + patch( + "app.api.routes.login.generate_password_reset_token", + return_value="reset-token", + ) as token_mock, + patch( + "app.api.routes.login.generate_reset_password_email", + side_effect=RuntimeError("template unavailable"), + ) as email_mock, + patch("app.api.routes.login.send_email") as send_email_mock, + ): + r = client.post(f"{settings.API_V1_STR}/password-recovery/{email}") + + assert r.status_code == 200 + assert r.json() == { + "message": "If that email is registered, we sent a password recovery link" + } + token_mock.assert_called_once_with(email=email) + email_mock.assert_called_once_with( + email_to=user.email, + email=email, + token="reset-token", + ) + send_email_mock.assert_not_called() + assert caplog.text == "" + + def test_reset_password(client: TestClient, db: Session) -> None: email = random_email() password = random_lower_string()