From f10ac492c1a2f0e44a46845831e69232bef6a81e Mon Sep 17 00:00:00 2001 From: Dan Nguyen Date: Fri, 5 Apr 2024 17:49:43 +0200 Subject: [PATCH] (PC-29049)[API] fix: empty password user login To prevent user enumeration attacks by comparing server response time, a dummy password must be hashed when the email or the password does not exist. --- api/src/pcapi/core/users/repository.py | 6 +++--- api/tests/routes/native/v1/authentication_test.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/api/src/pcapi/core/users/repository.py b/api/src/pcapi/core/users/repository.py index 5c293289f55..90315f33adb 100644 --- a/api/src/pcapi/core/users/repository.py +++ b/api/src/pcapi/core/users/repository.py @@ -26,10 +26,10 @@ def check_user_and_credentials(user: models.User | None, password: str, allow_inactive: bool = False) -> None: # Order is important to prevent end-user to guess user emails # We need to check email and password before checking email validation - if not user: + if not user or not user.password: # Hash the given password, just like we would do if the user - # existed. This avoids user enumeration by comparing server - # response time. + # or the password existed. This avoids user enumeration by comparing + # server response time. crypto.check_password(password, HASHED_PLACEHOLDER) raise exceptions.InvalidIdentifier() if not (user.checkPassword(password) and (user.isActive or allow_inactive)): diff --git a/api/tests/routes/native/v1/authentication_test.py b/api/tests/routes/native/v1/authentication_test.py index 7bb2d686b99..aade7efcbb8 100644 --- a/api/tests/routes/native/v1/authentication_test.py +++ b/api/tests/routes/native/v1/authentication_test.py @@ -161,6 +161,17 @@ def test_unknown_user_logs_in(self, client, caplog): assert response.json == {"general": ["Identifiant ou Mot de passe incorrect"]} assert "Failed authentication attempt" in caplog.messages + def test_user_without_password_logs_in(self, client, caplog): + user = users_factories.UserFactory(password=None, isActive=True) + + response = client.post( + "/native/v1/signin", json={"identifier": user.email, "password": settings.TEST_DEFAULT_PASSWORD} + ) + + assert response.status_code == 400 + # generic message to prevent enumeration attack + assert response.json == {"general": ["Identifiant ou Mot de passe incorrect"]} + def test_user_logs_in_with_missing_fields(self, client): response = client.post("/native/v1/signin", json={}) assert response.status_code == 400