Skip to content

Commit

Permalink
(PC-29049)[API] fix: empty password user login
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dnguyen1-pass authored and cepehang committed Apr 8, 2024
1 parent 6b3641a commit f10ac49
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
6 changes: 3 additions & 3 deletions api/src/pcapi/core/users/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)):
Expand Down
11 changes: 11 additions & 0 deletions api/tests/routes/native/v1/authentication_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f10ac49

Please sign in to comment.