Skip to content

Commit e32b107

Browse files
committed
fix: Improving safety
1 parent 95e19a7 commit e32b107

File tree

2 files changed

+47
-18
lines changed

2 files changed

+47
-18
lines changed

backend/src/app/api/routes/login.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,12 @@ async def test_token(self, current_user: CurrentUser) -> Any:
8484
description="Send a password recovery email.",
8585
)
8686
async def recover_password(
87-
self, email: str, user_crud: UserCrudDep, email_manager: EmailManagerDep, background_tasks: BackgroundTasks
87+
self,
88+
email: str,
89+
user_crud: UserCrudDep,
90+
email_manager: EmailManagerDep,
91+
background_tasks: BackgroundTasks,
92+
settings: SettingsDep,
8893
) -> Message:
8994
"""
9095
Endpoint to initiate password recovery for a user.
@@ -93,12 +98,19 @@ async def recover_password(
9398
:param user_crud: Dependency for user CRUD operations.
9499
:param email_manager: The email manager dependency.
95100
:param background_tasks: The background tasks service.
101+
:param settings: The application settings dependency.
96102
:return: A confirmation message.
97-
:raises HTTPException: If the user with the provided email is not found.
103+
:raises HTTPException: If the user with the provided email is not found (local env only).
98104
"""
99105
user: User | None = await user_crud.get_by_email(email=email)
106+
if settings.ENVIRONMENT != "local":
107+
if user:
108+
background_tasks.add_task(email_manager.send_reset_password_email, email_to=email)
109+
return Message(message="If an account with that email exists, a recovery email has been sent.")
100110
if not user:
101-
raise HTTPException(status_code=404, detail="The user with this email does not exist in the system.")
111+
raise HTTPException(
112+
status_code=404, detail="The user with this email does not exist in the system. (Local env only)"
113+
)
102114
background_tasks.add_task(email_manager.send_reset_password_email, email_to=email)
103115
return Message(message="Password recovery email sent")
104116

backend/tests/app/api/routes/test_login.py

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,36 @@ async def test_test_token() -> None:
107107

108108
@pytest.mark.asyncio
109109
@pytest.mark.parametrize(
110-
"user_exists,raises_exception,expected_status,expected_detail",
111-
[(True, False, None, None), (False, True, 404, "The user with this email does not exist in the system.")],
112-
ids=["success", "user_not_found"],
110+
"environment, user_exists, raises_exception, expected_detail_or_message, email_task_called",
111+
[
112+
("local", True, False, "Password recovery email sent", True),
113+
("local", False, True, "The user with this email does not exist in the system. (Local env only)", False),
114+
("production", True, False, "If an account with that email exists, a recovery email has been sent.", True),
115+
("production", False, False, "If an account with that email exists, a recovery email has been sent.", False),
116+
],
117+
ids=["local_success", "local_not_found_raises", "prod_success_safe", "prod_not_found_safe"],
113118
)
114119
async def test_recover_password(
115-
user_exists: bool, raises_exception: bool, expected_status: int | None, expected_detail: str | None
120+
environment: str,
121+
user_exists: bool,
122+
raises_exception: bool,
123+
expected_detail_or_message: str,
124+
email_task_called: bool,
116125
) -> None:
117126
"""
118-
Test the recover_password endpoint for various scenarios.
127+
Test the recover_password endpoint for various scenarios and environments.
119128
120-
:param user_exists: Whether the user exists.
121-
:param raises_exception: Whether an exception is expected.
122-
:param expected_status: Expected HTTP status code if exception is raised.
123-
:param expected_detail: Expected exception detail if exception is raised.
129+
:param environment: The simulated application environment.
130+
:param user_exists: Whether the user is mocked to exist.
131+
:param raises_exception: Whether an HTTPException is expected.
132+
:param expected_detail_or_message: The expected detail for exceptions or message for success.
133+
:param email_task_called: Whether the email sending task should be called.
124134
:return: None
125135
"""
126136
user_crud_mock: AsyncMock = AsyncMock(spec=UserCrudDep)
127137
email_manager_mock: AsyncMock = AsyncMock(spec=EmailManager)
128138
background_tasks_mock: MagicMock = MagicMock(spec=BackgroundTasks)
139+
settings_mock: MagicMock = MagicMock(spec=Settings, ENVIRONMENT=environment)
129140
email: str = "user@example.com"
130141
user: User = User(id=uuid4(), email=email, is_active=True, is_superuser=False, full_name=None)
131142
future_get: asyncio.Future = asyncio.Future()
@@ -139,21 +150,27 @@ async def test_recover_password(
139150
user_crud=user_crud_mock,
140151
email_manager=email_manager_mock,
141152
background_tasks=background_tasks_mock,
153+
settings=settings_mock,
142154
)
143-
assert exc_info.value.status_code == expected_status
144-
assert exc_info.value.detail == expected_detail
155+
assert exc_info.value.status_code == 404
156+
assert exc_info.value.detail == expected_detail_or_message
157+
background_tasks_mock.add_task.assert_not_called()
145158
else:
146159
result: Message = await login_router.recover_password(
147160
email=email,
148161
user_crud=user_crud_mock,
149162
email_manager=email_manager_mock,
150163
background_tasks=background_tasks_mock,
164+
settings=settings_mock,
151165
)
152166
user_crud_mock.get_by_email.assert_called_once_with(email=email)
153-
background_tasks_mock.add_task.assert_called_once_with(
154-
email_manager_mock.send_reset_password_email, email_to=email
155-
)
156-
assert result == Message(message="Password recovery email sent")
167+
assert result.message == expected_detail_or_message
168+
if email_task_called:
169+
background_tasks_mock.add_task.assert_called_once_with(
170+
email_manager_mock.send_reset_password_email, email_to=email
171+
)
172+
else:
173+
background_tasks_mock.add_task.assert_not_called()
157174

158175

159176
@pytest.mark.asyncio

0 commit comments

Comments
 (0)