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>
This commit is contained in:
copilot-swe-agent[bot]
2026-03-08 12:07:25 +00:00
parent bd283f19fe
commit 86f9f5f9b1
4 changed files with 72 additions and 4 deletions
+3 -1
View File
@@ -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()
+3
View File
@@ -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)
+3 -3
View File
@@ -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)
+63
View File
@@ -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
# ---------------------------------------------------------------------------