🔒️ Ensure authentication takes constant time, to avoid enumeration attacks (#2105)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
7107f7e83a
commit
689d7105e1
@@ -57,11 +57,9 @@ 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
|
||||
@@ -71,7 +69,9 @@ def recover_password(email: str, session: SessionDep) -> Message:
|
||||
subject=email_data.subject,
|
||||
html_content=email_data.html_content,
|
||||
)
|
||||
return Message(message="Password recovery email sent")
|
||||
return Message(
|
||||
message="If that email is registered, we sent a password recovery link"
|
||||
)
|
||||
|
||||
|
||||
@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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user