From 86f9f5f9b1025c9270f342cc9ea77622c27781c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:07:25 +0000 Subject: [PATCH] fix(auth): activate account on password reset and fix is_active check order - reset_password sets is_active=True so users with unverified accounts can log in after using the forgot-password flow - admin set_password also sets is_active=True for the same reason - auth() now checks is_active before verifying the password, ensuring inactive users always see the email-verification prompt regardless of password correctness (avoids leaking password validity)" Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/admin_users.py | 4 ++- app/api/local_auth.py | 3 ++ app/auth.py | 6 ++-- tests/test_local_auth.py | 63 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/app/api/admin_users.py b/app/api/admin_users.py index 67d453ef..7628709a 100644 --- a/app/api/admin_users.py +++ b/app/api/admin_users.py @@ -488,9 +488,11 @@ def admin_set_password( raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Local user not found.") user.hashed_password = hash_password(body.password) - # Clear any outstanding reset tokens + # Clear any outstanding reset tokens and activate the account so the user + # can log in immediately after an admin sets their password. user.password_reset_token = None user.password_reset_sent_at = None + user.is_active = True try: db.commit() diff --git a/app/api/local_auth.py b/app/api/local_auth.py index be6df664..8a8ce634 100644 --- a/app/api/local_auth.py +++ b/app/api/local_auth.py @@ -366,6 +366,9 @@ async def reset_password(body: PasswordResetBody, db: DbSession) -> dict[str, st user.hashed_password = hash_password(body.new_password) user.password_reset_token = None user.password_reset_sent_at = None + # Activate the account in case it was still pending email verification. + # A valid password-reset token proves control of the registered email address. + user.is_active = True db.commit() logger.info("[SECURITY] PASSWORD_RESET_SUCCESS user=%s", user.email) diff --git a/app/auth.py b/app/auth.py index 5852a6dd..ccffce3b 100644 --- a/app/auth.py +++ b/app/auth.py @@ -318,15 +318,15 @@ async def auth(request: Request, db: Session = Depends(get_db)): db.query(_LocalUser).filter((_LocalUser.username == username) | (_LocalUser.email == username)).first() ) if local_user is not None: - if not _verify_password(password or "", local_user.hashed_password): - logger.warning("[SECURITY] LOCAL_LOGIN_FAILURE user=%s", username) - return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302) if not local_user.is_active: logger.warning("[SECURITY] LOCAL_LOGIN_UNVERIFIED user=%s", username) return RedirectResponse( url="/login?error=Please+verify+your+email+address+before+logging+in", status_code=302, ) + if not _verify_password(password or "", local_user.hashed_password): + logger.warning("[SECURITY] LOCAL_LOGIN_FAILURE user=%s", username) + return RedirectResponse(url="/login?error=Invalid+username+or+password", status_code=302) user_data = _build_session_user(local_user) request.session["user"] = user_data logger.info("[SECURITY] LOCAL_LOGIN_SUCCESS user=%s", local_user.email) diff --git a/tests/test_local_auth.py b/tests/test_local_auth.py index 52cb9708..c7a23a7c 100644 --- a/tests/test_local_auth.py +++ b/tests/test_local_auth.py @@ -515,6 +515,41 @@ def test_reset_password_mismatch(la_client, la_session): assert resp.status_code == 422 +@pytest.mark.integration +def test_reset_password_activates_inactive_user(la_client, la_session): + """POST /api/auth/reset-password sets is_active=True for inactive accounts. + + A user who registered when SMTP was configured starts out with is_active=False. + Using the password-reset flow (which proves ownership of the email address) must + also activate the account so the user can log in immediately afterwards. + """ + token = "activatetoken456" + user = LocalUser( + email="inactive_reset@example.com", + username="inactivereset", + hashed_password=hash_password("oldpassword"), + is_active=False, # account not yet verified + password_reset_token=token, + password_reset_sent_at=datetime.now(tz=timezone.utc), + ) + la_session.add(user) + la_session.commit() + + resp = la_client.post( + "/api/auth/reset-password", + json={ + "token": token, + "new_password": "newpassword2", + "new_password_confirm": "newpassword2", + }, + ) + assert resp.status_code == 200 + la_session.refresh(user) + assert user.is_active is True, "reset_password must activate inactive accounts" + assert verify_password("newpassword2", user.hashed_password) + assert user.password_reset_token is None + + # --------------------------------------------------------------------------- # Integration tests: page routes # --------------------------------------------------------------------------- @@ -646,6 +681,34 @@ async def test_local_login_unverified(la_session, pending_user): assert "verify" in result.headers["location"].lower() +@pytest.mark.unit +@pytest.mark.asyncio +@patch.object(settings, "multi_user_enabled", True) +async def test_local_login_unverified_wrong_password(la_session, pending_user): + """auth() for unverified user with wrong password still shows the verify message. + + is_active is checked before password so that inactive users always receive + the email-verification prompt regardless of whether they typed the correct + password. This avoids leaking whether the password is correct for an + account that has not yet been verified. + """ + from unittest.mock import AsyncMock, MagicMock + + from fastapi import Request + + from app.auth import auth + + mock_request = MagicMock(spec=Request) + mock_request.form = AsyncMock(return_value={"username": "pendinguser", "password": "wrongpassword"}) + mock_request.session = {} + + result = await auth(mock_request, db=la_session) + assert result.status_code == 302 + # Must send to the *verify* page, not the generic "invalid credentials" page. + assert "verify" in result.headers["location"].lower() + assert "user" not in mock_request.session + + # --------------------------------------------------------------------------- # Single-user backward-compatibility: LocalUser table must NOT be queried # ---------------------------------------------------------------------------