Equalize Access login failure handling
This commit is contained in:
@@ -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.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.auth.tenant_context import AccessTenantContextSwitcher
|
||||||
from govoplan_access.backend.security.api_keys import authenticate_api_key
|
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 (
|
from govoplan_access.backend.security.sessions import (
|
||||||
authenticate_session_token,
|
authenticate_session_token,
|
||||||
collect_user_authorization_context,
|
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))
|
.filter(Account.normalized_email == normalize_email(payload.email), Account.is_active.is_(True))
|
||||||
.one_or_none()
|
.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")
|
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid login")
|
||||||
|
|
||||||
query = (
|
query = (
|
||||||
@@ -269,7 +271,9 @@ def _resolve_login_user(session: Session, payload: LoginRequest) -> tuple[Accoun
|
|||||||
query = query.filter(Tenant.slug == payload.tenant_slug)
|
query = query.filter(Tenant.slug == payload.tenant_slug)
|
||||||
row = query.order_by(Tenant.name.asc()).first()
|
row = query.order_by(Tenant.name.asc()).first()
|
||||||
if row is None:
|
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]
|
return account, row[0], row[1]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -9,6 +9,14 @@ _ALGORITHM = "pbkdf2_sha256"
|
|||||||
_DEFAULT_ITERATIONS = 260_000
|
_DEFAULT_ITERATIONS = 260_000
|
||||||
_SALT_BYTES = 16
|
_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:
|
def hash_password(password: str, *, iterations: int = _DEFAULT_ITERATIONS) -> str:
|
||||||
salt = os.urandom(_SALT_BYTES)
|
salt = os.urandom(_SALT_BYTES)
|
||||||
@@ -37,4 +45,3 @@ def verify_password(password: str, encoded: str | None) -> bool:
|
|||||||
return False
|
return False
|
||||||
actual = hashlib.pbkdf2_hmac("sha256", password.encode("utf-8"), salt, iterations)
|
actual = hashlib.pbkdf2_hmac("sha256", password.encode("utf-8"), salt, iterations)
|
||||||
return hmac.compare_digest(actual, expected)
|
return hmac.compare_digest(actual, expected)
|
||||||
|
|
||||||
|
|||||||
87
tests/test_login_security.py
Normal file
87
tests/test_login_security.py
Normal file
@@ -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()
|
||||||
Reference in New Issue
Block a user