diff --git a/src/govoplan_access/backend/api/v1/auth.py b/src/govoplan_access/backend/api/v1/auth.py index f456b6c..b6de81f 100644 --- a/src/govoplan_access/backend/api/v1/auth.py +++ b/src/govoplan_access/backend/api/v1/auth.py @@ -50,7 +50,7 @@ from govoplan_core.settings import settings from govoplan_access.backend.semantic import collect_function_assignment_ids, collect_function_delegation_ids, identity_id_for_account from govoplan_access.backend.auth.tenant_context import AccessTenantContextSwitcher from govoplan_access.backend.security.api_keys import authenticate_api_key -from govoplan_access.backend.security.passwords import verify_password +from govoplan_access.backend.security.passwords import DUMMY_PASSWORD_HASH, verify_password from govoplan_access.backend.security.sessions import ( authenticate_session_token, collect_user_authorization_context, @@ -253,7 +253,9 @@ def _resolve_login_user(session: Session, payload: LoginRequest) -> tuple[Accoun .filter(Account.normalized_email == normalize_email(payload.email), Account.is_active.is_(True)) .one_or_none() ) - if account is None or not verify_password(payload.password, account.password_hash): + password_hash = account.password_hash if account is not None and account.password_hash else DUMMY_PASSWORD_HASH + password_matches = verify_password(payload.password, password_hash) + if account is None or not account.password_hash or not password_matches: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid login") query = ( @@ -269,7 +271,9 @@ def _resolve_login_user(session: Session, payload: LoginRequest) -> tuple[Accoun query = query.filter(Tenant.slug == payload.tenant_slug) row = query.order_by(Tenant.name.asc()).first() if row is None: - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="No active tenant membership") + # Keep every authentication failure generic so callers cannot infer + # whether an account exists but lacks an active tenant membership. + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid login") return account, row[0], row[1] diff --git a/src/govoplan_access/backend/security/passwords.py b/src/govoplan_access/backend/security/passwords.py index 7d140f9..d8a3022 100644 --- a/src/govoplan_access/backend/security/passwords.py +++ b/src/govoplan_access/backend/security/passwords.py @@ -9,6 +9,14 @@ _ALGORITHM = "pbkdf2_sha256" _DEFAULT_ITERATIONS = 260_000 _SALT_BYTES = 16 +# A valid, fixed-cost hash used when a login identity has no local password hash. +# Its plaintext value is intentionally irrelevant; the hash only keeps failed +# login attempts on the same verification path as existing local accounts. +DUMMY_PASSWORD_HASH = ( + "pbkdf2_sha256$260000$Z292b3BsYW4tZHVtbXktdjE=" # noqa: S105 # nosec B105 - non-account timing equalizer. + "$uWgE7ht8wO6cotOqNKK2yNomPt57gstVss5ben5gTbw=" +) + def hash_password(password: str, *, iterations: int = _DEFAULT_ITERATIONS) -> str: salt = os.urandom(_SALT_BYTES) @@ -37,4 +45,3 @@ def verify_password(password: str, encoded: str | None) -> bool: return False actual = hashlib.pbkdf2_hmac("sha256", password.encode("utf-8"), salt, iterations) return hmac.compare_digest(actual, expected) - diff --git a/tests/test_login_security.py b/tests/test_login_security.py new file mode 100644 index 0000000..f3653b8 --- /dev/null +++ b/tests/test_login_security.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +import unittest +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from fastapi import HTTPException + +from govoplan_access.backend.api.v1 import auth +from govoplan_access.backend.security.passwords import ( + DUMMY_PASSWORD_HASH, + verify_password, +) +from govoplan_core.api.v1.schemas import LoginRequest + + +class LoginSecurityTests(unittest.TestCase): + @staticmethod + def _session_with_account(account: object | None) -> MagicMock: + session = MagicMock() + session.query.return_value.filter.return_value.one_or_none.return_value = ( + account + ) + return session + + def test_absent_identity_verifies_a_valid_dummy_hash_before_generic_failure( + self, + ) -> None: + self.assertTrue(verify_password("not-a-user-password", DUMMY_PASSWORD_HASH)) + payload = LoginRequest( + email="missing@example.test", password="attempted-password" + ) + + with patch.object(auth, "verify_password", return_value=True) as verifier: + with self.assertRaises(HTTPException) as raised: + auth._resolve_login_user(self._session_with_account(None), payload) + + verifier.assert_called_once_with(payload.password, DUMMY_PASSWORD_HASH) + self.assertEqual(raised.exception.status_code, 401) + self.assertEqual(raised.exception.detail, "Invalid login") + + def test_present_identity_verifies_account_hash_before_same_generic_failure( + self, + ) -> None: + account_hash = "pbkdf2_sha256$260000$account-salt$account-digest" + account = SimpleNamespace(password_hash=account_hash) + payload = LoginRequest(email="known@example.test", password="wrong-password") + + with patch.object(auth, "verify_password", return_value=False) as verifier: + with self.assertRaises(HTTPException) as raised: + auth._resolve_login_user(self._session_with_account(account), payload) # type: ignore[arg-type] + + verifier.assert_called_once_with(payload.password, account_hash) + self.assertEqual(raised.exception.status_code, 401) + self.assertEqual(raised.exception.detail, "Invalid login") + + def test_passwordless_account_cannot_authenticate_with_dummy_password(self) -> None: + account = SimpleNamespace(password_hash=None) + payload = LoginRequest( + email="passwordless@example.test", password="not-a-user-password" + ) + + with self.assertRaises(HTTPException) as raised: + auth._resolve_login_user(self._session_with_account(account), payload) # type: ignore[arg-type] + + self.assertEqual(raised.exception.status_code, 401) + self.assertEqual(raised.exception.detail, "Invalid login") + + def test_account_without_active_membership_uses_same_generic_failure(self) -> None: + account = SimpleNamespace( + id="account-1", + password_hash="pbkdf2_sha256$260000$account-salt$account-digest", + ) + session = self._session_with_account(account) + session.query.return_value.join.return_value.filter.return_value.order_by.return_value.first.return_value = None + payload = LoginRequest(email="known@example.test", password="correct-password") + + with patch.object(auth, "verify_password", return_value=True): + with self.assertRaises(HTTPException) as raised: + auth._resolve_login_user(session, payload) # type: ignore[arg-type] + + self.assertEqual(raised.exception.status_code, 401) + self.assertEqual(raised.exception.detail, "Invalid login") + + +if __name__ == "__main__": + unittest.main()