feat(access): add guarded local password lifecycle and recovery
This commit is contained in:
@@ -29,6 +29,7 @@ from govoplan_core.tenancy.scope import (
|
||||
create_scope_tables,
|
||||
scope_registry,
|
||||
)
|
||||
from govoplan_core.settings import settings
|
||||
|
||||
|
||||
class AutomationPrincipalTests(unittest.TestCase):
|
||||
@@ -189,6 +190,14 @@ class AutomationPrincipalTests(unittest.TestCase):
|
||||
suspended.provenance["status"],
|
||||
)
|
||||
|
||||
def test_required_local_password_change_denies_delegated_automation(self) -> None:
|
||||
self.account.password_reset_required = True
|
||||
self.session.commit()
|
||||
with patch.object(settings, "auth_local_password_recovery_enabled", True):
|
||||
result = self.provider.resolve_automation_principal(self.session, request=self._request())
|
||||
self.assertFalse(result.allowed)
|
||||
self.assertEqual("password_change_required", result.provenance["status"])
|
||||
|
||||
def test_service_account_resolution_uses_current_scope_ceiling(self) -> None:
|
||||
account = Account(
|
||||
id="service-account-backing",
|
||||
|
||||
@@ -6,11 +6,36 @@ from govoplan_access.backend.manifest import manifest
|
||||
|
||||
|
||||
class InterfaceDocumentationContractTests(unittest.TestCase):
|
||||
def test_password_change_flag_is_documented_as_unenforced(self) -> None:
|
||||
def test_password_f1_contexts_resolve_once_to_concise_bilingual_topics(self) -> None:
|
||||
expected = {
|
||||
"access.password.change": "access.help.password-change",
|
||||
"access.password.recover": "access.help.password-recovery",
|
||||
"access.password.issue-recovery": "access.help.password-issue-recovery",
|
||||
}
|
||||
for context, topic_id in expected.items():
|
||||
with self.subTest(context=context):
|
||||
matches = [topic for topic in manifest.documentation if context in (topic.metadata or {}).get("help_contexts", ())]
|
||||
self.assertEqual([topic_id], [topic.id for topic in matches])
|
||||
topic = matches[0]
|
||||
self.assertEqual({"admin", "user"}, set(topic.documentation_types))
|
||||
for body in (topic.body, topic.translations["de"]["body"]):
|
||||
self.assertLessEqual(len(body.split()), 180)
|
||||
self.assertIn("API", body)
|
||||
self.assertIn("URLs", body)
|
||||
if context != "access.password.change":
|
||||
self.assertIn("15 minutes", topic.body)
|
||||
self.assertIn("15 Minuten", topic.translations["de"]["body"])
|
||||
|
||||
def test_password_change_flag_documents_opt_in_and_complete_recovery(self) -> None:
|
||||
topic = next(item for item in manifest.documentation if item.id == "access.reference.authentication-fields")
|
||||
self.assertIn("currently advisory metadata", topic.body)
|
||||
self.assertIn("server-side enforcement are not implemented", topic.body)
|
||||
self.assertIn("serverseitige Durchsetzung sind noch nicht umgesetzt", topic.translations["de"]["body"])
|
||||
self.assertIn("defaults to false", topic.body)
|
||||
self.assertIn("remains advisory metadata", topic.body)
|
||||
self.assertIn("standardmäßig false", topic.translations["de"]["body"])
|
||||
recovery = next(item for item in manifest.documentation if item.id == "access.workflow.local-password-recovery")
|
||||
for required in ("15 minutes", "single-use", "system:*", "human API keys", "e9a2c5f8b1d4", "does not send email"):
|
||||
self.assertIn(required, recovery.body)
|
||||
self.assertIn("issued by that account for other people", recovery.body)
|
||||
self.assertIn("von diesem Konto für andere Personen ausgestellte", recovery.translations["de"]["body"])
|
||||
|
||||
def test_all_static_topics_have_complete_german_content(self) -> None:
|
||||
for topic in manifest.documentation:
|
||||
|
||||
@@ -18,7 +18,7 @@ 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 = (
|
||||
session.query.return_value.filter.return_value.populate_existing.return_value.with_for_update.return_value.one_or_none.return_value = (
|
||||
account
|
||||
)
|
||||
return session
|
||||
@@ -43,7 +43,7 @@ class LoginSecurityTests(unittest.TestCase):
|
||||
self,
|
||||
) -> None:
|
||||
account_hash = "pbkdf2_sha256$260000$account-salt$account-digest"
|
||||
account = SimpleNamespace(password_hash=account_hash)
|
||||
account = SimpleNamespace(password_hash=account_hash, auth_provider="local")
|
||||
payload = LoginRequest(email="known@example.test", password="wrong-password")
|
||||
|
||||
with patch.object(auth, "verify_password", return_value=False) as verifier:
|
||||
@@ -55,7 +55,7 @@ class LoginSecurityTests(unittest.TestCase):
|
||||
self.assertEqual(raised.exception.detail, "Invalid login")
|
||||
|
||||
def test_passwordless_account_cannot_authenticate_with_dummy_password(self) -> None:
|
||||
account = SimpleNamespace(password_hash=None)
|
||||
account = SimpleNamespace(password_hash=None, auth_provider="local")
|
||||
payload = LoginRequest(
|
||||
email="passwordless@example.test", password="not-a-user-password"
|
||||
)
|
||||
@@ -69,6 +69,7 @@ class LoginSecurityTests(unittest.TestCase):
|
||||
def test_account_without_active_membership_uses_same_generic_failure(self) -> None:
|
||||
account = SimpleNamespace(
|
||||
id="account-1",
|
||||
auth_provider="local",
|
||||
password_hash="pbkdf2_sha256$260000$account-salt$account-digest",
|
||||
)
|
||||
session = self._session_with_account(account)
|
||||
|
||||
@@ -0,0 +1,607 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from datetime import timedelta
|
||||
from unittest.mock import patch
|
||||
|
||||
from fastapi import Depends, FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
from sqlalchemy.pool import StaticPool
|
||||
|
||||
from govoplan_access.backend.api.v1 import auth
|
||||
from govoplan_access.backend.auth.dependencies import (
|
||||
AccessApiPrincipalProvider,
|
||||
get_api_principal,
|
||||
)
|
||||
from govoplan_access.backend.auth.principal_cache import principal_summary_cache
|
||||
from govoplan_access.backend.db.base import AccessBase
|
||||
from govoplan_access.backend.db.models import (
|
||||
Account,
|
||||
ApiKey,
|
||||
AuthSession,
|
||||
PasswordRecovery,
|
||||
Role,
|
||||
SystemRoleAssignment,
|
||||
User,
|
||||
)
|
||||
from govoplan_access.backend.security.api_keys import create_api_key
|
||||
from govoplan_access.backend.security.login_throttle import (
|
||||
InMemoryLoginAttemptStore,
|
||||
LoginThrottle,
|
||||
)
|
||||
from govoplan_access.backend.security.passwords import hash_password, verify_password
|
||||
from govoplan_access.backend.security.password_change import replace_password
|
||||
from govoplan_access.backend.security.sessions import create_auth_session
|
||||
from govoplan_core.admin.models import SystemSettings
|
||||
from govoplan_core.auth import get_api_principal as get_core_api_principal
|
||||
from govoplan_core.core.access import CAPABILITY_AUTH_API_PRINCIPAL_PROVIDER
|
||||
from govoplan_core.core.modules import ModuleContext
|
||||
from govoplan_core.core.registry import PlatformRegistry
|
||||
from govoplan_core.core.change_sequence import (
|
||||
ChangeSequenceEntry,
|
||||
ChangeSequenceRetentionFloor,
|
||||
)
|
||||
from govoplan_core.db.base import Base
|
||||
from govoplan_core.db.session import get_session
|
||||
from govoplan_core.security.time import utc_now
|
||||
from govoplan_core.settings import settings
|
||||
from govoplan_core.tenancy.scope import Tenant, create_scope_tables
|
||||
|
||||
|
||||
class PasswordRecoveryTests(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.engine = create_engine(
|
||||
"sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool
|
||||
)
|
||||
create_scope_tables(self.engine)
|
||||
AccessBase.metadata.create_all(self.engine)
|
||||
Base.metadata.create_all(
|
||||
self.engine,
|
||||
tables=[
|
||||
SystemSettings.__table__,
|
||||
ChangeSequenceEntry.__table__,
|
||||
ChangeSequenceRetentionFloor.__table__,
|
||||
],
|
||||
)
|
||||
self.factory = sessionmaker(bind=self.engine)
|
||||
self.db = self.factory()
|
||||
self.tenant = Tenant(id="tenant", slug="tenant", name="Tenant")
|
||||
self.account = Account(
|
||||
id="person",
|
||||
email="person@example.test",
|
||||
normalized_email="person@example.test",
|
||||
password_hash=hash_password("Initial-password"),
|
||||
password_reset_required=True,
|
||||
)
|
||||
self.owner = Account(
|
||||
id="owner",
|
||||
email="owner@example.test",
|
||||
normalized_email="owner@example.test",
|
||||
password_hash=hash_password("Owner-password"),
|
||||
)
|
||||
self.user = User(
|
||||
id="person-member",
|
||||
account_id="person",
|
||||
tenant_id="tenant",
|
||||
email=self.account.email,
|
||||
)
|
||||
self.owner_user = User(
|
||||
id="owner-member",
|
||||
account_id="owner",
|
||||
tenant_id="tenant",
|
||||
email=self.owner.email,
|
||||
)
|
||||
role = Role(
|
||||
id="owner-role",
|
||||
tenant_id=None,
|
||||
slug="system_owner",
|
||||
name="System owner",
|
||||
permissions=["system:*"],
|
||||
)
|
||||
self.owner_assignment = SystemRoleAssignment(
|
||||
id="owner-assignment", account_id="owner", role_id=role.id
|
||||
)
|
||||
self.db.add_all(
|
||||
[
|
||||
self.tenant,
|
||||
self.account,
|
||||
self.owner,
|
||||
self.user,
|
||||
self.owner_user,
|
||||
role,
|
||||
self.owner_assignment,
|
||||
]
|
||||
)
|
||||
self.db.flush()
|
||||
self.current = create_auth_session(self.db, user=self.user)
|
||||
self.other = create_auth_session(self.db, user=self.user)
|
||||
self.owner_session = create_auth_session(self.db, user=self.owner_user)
|
||||
self.key = create_api_key(
|
||||
self.db, user=self.user, name="Human automation", scopes=[]
|
||||
)
|
||||
self.db.commit()
|
||||
self.audit = patch.object(auth, "audit_event").start()
|
||||
self.addCleanup(patch.stopall)
|
||||
patch.object(settings, "auth_local_password_recovery_enabled", True).start()
|
||||
patch.object(settings, "auth_principal_cache_enabled", True).start()
|
||||
patch.object(settings, "auth_login_throttle_enabled", False).start()
|
||||
self.throttle = LoginThrottle(
|
||||
InMemoryLoginAttemptStore(),
|
||||
identity_limit=50,
|
||||
client_limit=100,
|
||||
window_seconds=900,
|
||||
)
|
||||
patch.object(
|
||||
auth, "_password_operation_throttle", return_value=self.throttle
|
||||
).start()
|
||||
app = FastAPI()
|
||||
registry = PlatformRegistry()
|
||||
registry.configure_capability_context(
|
||||
ModuleContext(registry=registry, settings=settings)
|
||||
)
|
||||
registry.register_capability_factory(
|
||||
"access",
|
||||
CAPABILITY_AUTH_API_PRINCIPAL_PROVIDER,
|
||||
lambda _context: AccessApiPrincipalProvider(),
|
||||
)
|
||||
app.state.govoplan_registry = registry
|
||||
app.include_router(auth.router, prefix="/api/v1")
|
||||
|
||||
@app.get("/protected")
|
||||
def protected(principal=Depends(get_api_principal)):
|
||||
return {"account": principal.account_id}
|
||||
|
||||
@app.get("/core-protected")
|
||||
def core_protected(principal=Depends(get_core_api_principal)):
|
||||
return {"account": principal.account_id}
|
||||
|
||||
def session_dependency():
|
||||
with self.factory() as session:
|
||||
try:
|
||||
yield session
|
||||
except BaseException:
|
||||
session.rollback()
|
||||
raise
|
||||
|
||||
app.dependency_overrides[get_session] = session_dependency
|
||||
self.client = TestClient(app)
|
||||
principal_summary_cache.clear()
|
||||
|
||||
def tearDown(self) -> None:
|
||||
self.client.close()
|
||||
self.db.close()
|
||||
self.engine.dispose()
|
||||
principal_summary_cache.clear()
|
||||
|
||||
def headers(self, created=None):
|
||||
return {"authorization": "Bearer " + (created or self.current).token}
|
||||
|
||||
def change(self, **overrides):
|
||||
return self.client.post(
|
||||
"/api/v1/auth/password/change",
|
||||
headers=self.headers(),
|
||||
json={
|
||||
"current_password": "Initial-password",
|
||||
"new_password": "New-secret-password",
|
||||
**overrides,
|
||||
},
|
||||
)
|
||||
|
||||
def issue(self):
|
||||
return self.client.post(
|
||||
"/api/v1/auth/password/recovery/person",
|
||||
headers=self.headers(self.owner_session),
|
||||
json={"current_password": "Owner-password", "identity_verified": True},
|
||||
)
|
||||
|
||||
def recover(self, code, **overrides):
|
||||
return self.client.post(
|
||||
"/api/v1/auth/password/recover",
|
||||
json={
|
||||
"email": "person@example.test",
|
||||
"recovery_code": code,
|
||||
"new_password": "Recovered-password",
|
||||
**overrides,
|
||||
},
|
||||
)
|
||||
|
||||
def test_first_login_restricts_then_rotates_and_revokes_all_credentials(self):
|
||||
login = self.client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"email": self.account.email, "password": "Initial-password"},
|
||||
)
|
||||
self.assertEqual(200, login.status_code, login.text)
|
||||
self.assertEqual(
|
||||
"change_password", login.json()["user"]["required_auth_action"]
|
||||
)
|
||||
self.assertEqual([], login.json()["scopes"])
|
||||
self.assertEqual(
|
||||
403, self.client.get("/protected", headers=self.headers()).status_code
|
||||
)
|
||||
self.assertEqual(
|
||||
403,
|
||||
self.client.patch(
|
||||
"/api/v1/auth/profile",
|
||||
headers=self.headers(),
|
||||
json={"display_name": "Escaped"},
|
||||
).status_code,
|
||||
)
|
||||
for path in ("session", "shell"):
|
||||
result = self.client.get("/api/v1/auth/" + path, headers=self.headers())
|
||||
self.assertEqual(200, result.status_code, result.text)
|
||||
self.assertEqual(
|
||||
"change_password", result.json()["user"]["required_auth_action"]
|
||||
)
|
||||
changed = self.change()
|
||||
self.assertEqual(200, changed.status_code, changed.text)
|
||||
self.assertIsNone(changed.json()["user"]["required_auth_action"])
|
||||
self.assertNotEqual(self.current.token, changed.json()["access_token"])
|
||||
for old in (self.current.token, self.other.token, self.key.secret):
|
||||
self.assertEqual(
|
||||
401,
|
||||
self.client.get(
|
||||
"/protected", headers={"authorization": "Bearer " + old}
|
||||
).status_code,
|
||||
)
|
||||
self.assertEqual(
|
||||
200,
|
||||
self.client.get(
|
||||
"/protected",
|
||||
headers={"authorization": "Bearer " + changed.json()["access_token"]},
|
||||
).status_code,
|
||||
)
|
||||
self.db.expire_all()
|
||||
self.assertTrue(
|
||||
verify_password("New-secret-password", self.account.password_hash)
|
||||
)
|
||||
self.assertTrue(verify_password("New-secret-password", self.user.password_hash))
|
||||
self.assertFalse(self.account.password_reset_required)
|
||||
self.assertNotIn("New-secret-password", str(self.audit.call_args_list))
|
||||
self.assertNotIn("Initial-password", str(self.audit.call_args_list))
|
||||
|
||||
def test_failed_current_password_leaves_credentials_and_flag_unchanged(self):
|
||||
result = self.change(current_password="wrong-password")
|
||||
self.assertEqual(403, result.status_code, result.text)
|
||||
self.db.expire_all()
|
||||
self.assertTrue(self.account.password_reset_required)
|
||||
self.assertIsNone(self.db.get(AuthSession, self.current.model.id).revoked_at)
|
||||
self.assertTrue(verify_password("Initial-password", self.account.password_hash))
|
||||
self.assertEqual(422, self.change(new_password="Initial-password").status_code)
|
||||
|
||||
def test_core_api_provider_enforces_current_flag_for_sessions_and_keys(self):
|
||||
for headers in (self.headers(), {"x-api-key": self.key.secret}):
|
||||
with patch.object(settings, "auth_local_password_recovery_enabled", False):
|
||||
self.assertEqual(
|
||||
200, self.client.get("/core-protected", headers=headers).status_code
|
||||
)
|
||||
result = self.client.get("/core-protected", headers=headers)
|
||||
self.assertEqual(403, result.status_code)
|
||||
self.assertEqual(
|
||||
"password_change_required", result.json()["detail"]["code"]
|
||||
)
|
||||
|
||||
def test_validation_responses_do_not_echo_rejected_secrets(self):
|
||||
response = self.change(new_password="tiny")
|
||||
self.assertEqual(422, response.status_code)
|
||||
self.assertNotIn("tiny", response.text)
|
||||
self.assertNotIn("Initial-password", response.text)
|
||||
self.assertNotIn("input", response.json()["detail"][0])
|
||||
|
||||
def test_cookie_change_requires_matching_csrf_and_replaces_csrf(self):
|
||||
self.client.cookies.set(settings.auth_session_cookie_name, self.current.token)
|
||||
self.client.cookies.set(settings.auth_csrf_cookie_name, self.current.csrf_token)
|
||||
payload = {
|
||||
"current_password": "Initial-password",
|
||||
"new_password": "New-secret-password",
|
||||
}
|
||||
self.assertEqual(
|
||||
403,
|
||||
self.client.post("/api/v1/auth/password/change", json=payload).status_code,
|
||||
)
|
||||
response = self.client.post(
|
||||
"/api/v1/auth/password/change",
|
||||
headers={"x-csrf-token": self.current.csrf_token},
|
||||
json=payload,
|
||||
)
|
||||
self.assertEqual(200, response.status_code, response.text)
|
||||
self.assertIn("HttpOnly", response.headers["set-cookie"])
|
||||
self.assertNotIn(self.current.csrf_token, response.headers["set-cookie"])
|
||||
|
||||
def test_opt_in_preserves_advisory_behavior_and_still_allows_change(self):
|
||||
with patch.object(settings, "auth_local_password_recovery_enabled", False):
|
||||
self.assertEqual(
|
||||
200, self.client.get("/protected", headers=self.headers()).status_code
|
||||
)
|
||||
self.assertIsNone(
|
||||
self.client.get("/api/v1/auth/session", headers=self.headers()).json()[
|
||||
"user"
|
||||
]["required_auth_action"]
|
||||
)
|
||||
self.assertEqual(409, self.issue().status_code)
|
||||
self.assertEqual(409, self.recover("unknown").status_code)
|
||||
self.assertEqual(200, self.change().status_code)
|
||||
|
||||
def test_warm_cache_and_human_api_keys_do_not_bypass_flag(self):
|
||||
self.account.password_reset_required = False
|
||||
self.db.commit()
|
||||
self.assertEqual(
|
||||
200, self.client.get("/protected", headers=self.headers()).status_code
|
||||
)
|
||||
self.assertEqual(
|
||||
200,
|
||||
self.client.get(
|
||||
"/protected", headers={"x-api-key": self.key.secret}
|
||||
).status_code,
|
||||
)
|
||||
self.account.password_reset_required = True
|
||||
self.db.commit()
|
||||
self.assertEqual(
|
||||
403, self.client.get("/protected", headers=self.headers()).status_code
|
||||
)
|
||||
self.assertEqual(
|
||||
403,
|
||||
self.client.get(
|
||||
"/protected", headers={"x-api-key": self.key.secret}
|
||||
).status_code,
|
||||
)
|
||||
self.assertEqual(
|
||||
403,
|
||||
self.client.get(
|
||||
"/api/v1/auth/session", headers={"x-api-key": self.key.secret}
|
||||
).status_code,
|
||||
)
|
||||
self.assertEqual(
|
||||
403,
|
||||
self.client.post(
|
||||
"/api/v1/auth/password/change",
|
||||
headers={"x-api-key": self.key.secret},
|
||||
json={
|
||||
"current_password": "Initial-password",
|
||||
"new_password": "New-secret-password",
|
||||
},
|
||||
).status_code,
|
||||
)
|
||||
self.account.password_reset_required = False
|
||||
self.db.commit()
|
||||
self.assertEqual(
|
||||
400,
|
||||
self.client.post(
|
||||
"/api/v1/auth/password/change",
|
||||
headers={"x-api-key": self.key.secret},
|
||||
json={
|
||||
"current_password": "Initial-password",
|
||||
"new_password": "New-secret-password",
|
||||
},
|
||||
).status_code,
|
||||
)
|
||||
|
||||
def test_external_provider_is_not_forced_or_allowed_to_change_local_password(self):
|
||||
for provider in ("oidc", "service_account"):
|
||||
with self.subTest(provider=provider):
|
||||
self.account.auth_provider = provider
|
||||
self.db.commit()
|
||||
session_info = self.client.get(
|
||||
"/api/v1/auth/session", headers=self.headers()
|
||||
)
|
||||
self.assertIsNone(session_info.json()["user"]["required_auth_action"])
|
||||
self.assertFalse(session_info.json()["user"]["local_password"])
|
||||
self.assertEqual(403, self.change().status_code)
|
||||
self.assertEqual(
|
||||
401,
|
||||
self.client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={
|
||||
"email": self.account.email,
|
||||
"password": "Initial-password",
|
||||
},
|
||||
).status_code,
|
||||
)
|
||||
|
||||
def test_recovery_code_is_hashed_one_use_and_revokes_sessions_and_keys(self):
|
||||
response = self.issue()
|
||||
self.assertEqual(200, response.status_code, response.text)
|
||||
code = response.json()["recovery_code"]
|
||||
stored = self.db.query(PasswordRecovery).one()
|
||||
self.assertNotEqual(code, stored.code_hash)
|
||||
self.assertNotIn(code, str(self.audit.call_args_list))
|
||||
self.assertEqual("no-store", response.headers["cache-control"])
|
||||
self.assertEqual(200, self.recover(code).status_code)
|
||||
self.assertEqual(
|
||||
400, self.recover(code, new_password="Another-secret").status_code
|
||||
)
|
||||
self.db.expire_all()
|
||||
self.assertTrue(
|
||||
verify_password("Recovered-password", self.account.password_hash)
|
||||
)
|
||||
self.assertFalse(self.account.password_reset_required)
|
||||
self.assertEqual(
|
||||
0,
|
||||
self.db.query(AuthSession)
|
||||
.filter(
|
||||
AuthSession.account_id == self.account.id,
|
||||
AuthSession.revoked_at.is_(None),
|
||||
)
|
||||
.count(),
|
||||
)
|
||||
self.assertIsNotNone(self.db.get(ApiKey, self.key.model.id).revoked_at)
|
||||
|
||||
def test_expiry_supersession_and_current_issuer_authority(self):
|
||||
first = self.issue().json()["recovery_code"]
|
||||
second = self.issue().json()["recovery_code"]
|
||||
self.assertEqual(400, self.recover(first).status_code)
|
||||
latest = (
|
||||
self.db.query(PasswordRecovery)
|
||||
.filter(PasswordRecovery.consumed_at.is_(None))
|
||||
.one()
|
||||
)
|
||||
latest.expires_at = utc_now() - timedelta(seconds=1)
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(second).status_code)
|
||||
third = self.issue().json()["recovery_code"]
|
||||
self.db.delete(self.owner_assignment)
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(third).status_code)
|
||||
self.assertEqual(403, self.issue().status_code)
|
||||
|
||||
def test_recovery_rechecks_account_and_membership_state(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
self.user.is_active = False
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(code).status_code)
|
||||
self.user.is_active = True
|
||||
self.account.auth_provider = "oidc"
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(code).status_code)
|
||||
|
||||
def test_recovery_rechecks_tenant_and_issuer_activation(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
self.tenant.is_active = False
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(code).status_code)
|
||||
self.tenant.is_active = True
|
||||
self.owner.is_active = False
|
||||
self.db.commit()
|
||||
self.assertEqual(400, self.recover(code).status_code)
|
||||
|
||||
def test_recovery_requires_current_owner_password_and_identity_verification(self):
|
||||
response = self.client.post(
|
||||
"/api/v1/auth/password/recovery/person",
|
||||
headers=self.headers(self.owner_session),
|
||||
json={"current_password": "wrong-password", "identity_verified": True},
|
||||
)
|
||||
self.assertEqual(403, response.status_code)
|
||||
response = self.client.post(
|
||||
"/api/v1/auth/password/recovery/person",
|
||||
headers=self.headers(self.owner_session),
|
||||
json={"current_password": "Owner-password", "identity_verified": False},
|
||||
)
|
||||
self.assertEqual(422, response.status_code)
|
||||
self.assertEqual(0, self.db.query(PasswordRecovery).count())
|
||||
role = self.db.get(Role, "owner-role")
|
||||
role.permissions = ["system:accounts:update"]
|
||||
self.db.commit()
|
||||
self.assertEqual(403, self.issue().status_code)
|
||||
|
||||
def test_invalid_recovery_email_or_unchanged_password_does_not_consume_code(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
response = self.recover(code, email="different@example.test")
|
||||
self.assertEqual(400, response.status_code)
|
||||
self.assertEqual(
|
||||
422, self.recover(code, new_password="Initial-password").status_code
|
||||
)
|
||||
self.assertEqual(200, self.recover(code).status_code)
|
||||
|
||||
def test_recovery_abuse_is_bounded_even_with_login_throttle_disabled(self):
|
||||
throttle = LoginThrottle(
|
||||
InMemoryLoginAttemptStore(),
|
||||
identity_limit=2,
|
||||
client_limit=100,
|
||||
window_seconds=900,
|
||||
)
|
||||
with patch.object(auth, "_password_operation_throttle", return_value=throttle):
|
||||
self.assertEqual(400, self.recover("wrong-code").status_code)
|
||||
response = self.recover("different-wrong-code")
|
||||
self.assertEqual(429, response.status_code)
|
||||
self.assertIn("retry-after", response.headers)
|
||||
|
||||
def test_failed_audit_rolls_back_password_and_session_rotation(self):
|
||||
with patch.object(
|
||||
auth, "audit_event", side_effect=RuntimeError("audit unavailable")
|
||||
):
|
||||
with self.assertRaises(RuntimeError):
|
||||
self.change()
|
||||
self.db.expire_all()
|
||||
self.assertTrue(verify_password("Initial-password", self.account.password_hash))
|
||||
self.assertIsNone(self.db.get(AuthSession, self.current.model.id).revoked_at)
|
||||
|
||||
def test_stale_password_authorization_cannot_overwrite_concurrent_change(self):
|
||||
from fastapi import HTTPException
|
||||
|
||||
with self.factory() as competing:
|
||||
current = competing.get(Account, self.account.id)
|
||||
replace_password(competing, account=current, password="Concurrent-password")
|
||||
competing.commit()
|
||||
with self.assertRaises(HTTPException) as conflict:
|
||||
# This intentionally uses the account state read before the other
|
||||
# transaction, modelling a database without row-lock support.
|
||||
replace_password(self.db, account=self.account, password="Stale-password")
|
||||
self.assertEqual(409, conflict.exception.status_code)
|
||||
self.db.rollback()
|
||||
self.db.refresh(self.account)
|
||||
self.assertTrue(
|
||||
verify_password("Concurrent-password", self.account.password_hash)
|
||||
)
|
||||
|
||||
def test_current_password_change_invalidates_outstanding_recovery(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
self.assertEqual(200, self.change().status_code)
|
||||
self.assertEqual(400, self.recover(code).status_code)
|
||||
|
||||
def test_owner_password_change_invalidates_codes_issued_for_other_accounts(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
changed = self.client.post(
|
||||
"/api/v1/auth/password/change",
|
||||
headers=self.headers(self.owner_session),
|
||||
json={
|
||||
"current_password": "Owner-password",
|
||||
"new_password": "Owner-new-password",
|
||||
},
|
||||
)
|
||||
self.assertEqual(200, changed.status_code, changed.text)
|
||||
rejected = self.recover(code)
|
||||
self.assertEqual(400, rejected.status_code, rejected.text)
|
||||
self.assertEqual("recovery_invalid", rejected.json()["detail"]["code"])
|
||||
self.db.expire_all()
|
||||
self.assertIsNotNone(self.db.query(PasswordRecovery).one().consumed_at)
|
||||
self.assertTrue(verify_password("Initial-password", self.account.password_hash))
|
||||
self.assertEqual(
|
||||
1, self.audit.call_args.kwargs["details"]["revoked_password_recoveries"]
|
||||
)
|
||||
|
||||
def test_owner_password_recovery_invalidates_codes_issued_for_other_accounts(self):
|
||||
code = self.issue().json()["recovery_code"]
|
||||
owner_recovery = self.client.post(
|
||||
"/api/v1/auth/password/recovery/owner",
|
||||
headers=self.headers(self.owner_session),
|
||||
json={"current_password": "Owner-password", "identity_verified": True},
|
||||
)
|
||||
self.assertEqual(200, owner_recovery.status_code, owner_recovery.text)
|
||||
recovered = self.recover(
|
||||
owner_recovery.json()["recovery_code"],
|
||||
email=self.owner.email,
|
||||
new_password="Owner-recovered-password",
|
||||
)
|
||||
self.assertEqual(200, recovered.status_code, recovered.text)
|
||||
rejected = self.recover(code)
|
||||
self.assertEqual(400, rejected.status_code, rejected.text)
|
||||
self.assertEqual("recovery_invalid", rejected.json()["detail"]["code"])
|
||||
self.db.expire_all()
|
||||
self.assertTrue(verify_password("Initial-password", self.account.password_hash))
|
||||
self.assertTrue(
|
||||
verify_password("Owner-recovered-password", self.owner.password_hash)
|
||||
)
|
||||
self.assertEqual(
|
||||
0,
|
||||
self.db.query(PasswordRecovery)
|
||||
.filter(PasswordRecovery.consumed_at.is_(None))
|
||||
.count(),
|
||||
)
|
||||
|
||||
def test_flagged_account_can_sign_out_and_requires_csrf_for_cookie_logout(self):
|
||||
self.client.cookies.set(settings.auth_session_cookie_name, self.current.token)
|
||||
self.client.cookies.set(settings.auth_csrf_cookie_name, self.current.csrf_token)
|
||||
self.assertEqual(403, self.client.post("/api/v1/auth/logout").status_code)
|
||||
result = self.client.post(
|
||||
"/api/v1/auth/logout", headers={"x-csrf-token": self.current.csrf_token}
|
||||
)
|
||||
self.assertEqual(200, result.status_code)
|
||||
self.assertEqual(
|
||||
401,
|
||||
self.client.get("/api/v1/auth/session", headers=self.headers()).status_code,
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,106 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
from alembic import command
|
||||
from sqlalchemy import create_engine, inspect, text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from govoplan_access.backend.db.models import Account
|
||||
from govoplan_access.backend.security.passwords import hash_password
|
||||
from govoplan_core.db.migrations import alembic_config
|
||||
|
||||
|
||||
class PasswordRecoveryMigrationTests(unittest.TestCase):
|
||||
def test_release_and_development_upgrade_preserve_existing_credentials(self):
|
||||
for track in ("release", "dev"):
|
||||
with (
|
||||
self.subTest(track=track),
|
||||
tempfile.TemporaryDirectory(
|
||||
prefix="govoplan-password-migration-"
|
||||
) as directory,
|
||||
):
|
||||
url = f"sqlite:///{Path(directory) / 'isolated-upgrade.db'}"
|
||||
config = alembic_config(
|
||||
database_url=url, enabled_modules=("access",), migration_track=track
|
||||
)
|
||||
command.upgrade(config, "4f2a9c8e7b6d")
|
||||
command.upgrade(config, "d8f1b4e7a0c3")
|
||||
engine = create_engine(url)
|
||||
try:
|
||||
with Session(engine) as session:
|
||||
session.add(
|
||||
Account(
|
||||
id="existing",
|
||||
email="existing@example.test",
|
||||
normalized_email="existing@example.test",
|
||||
password_hash=hash_password("Existing-password"),
|
||||
password_reset_required=True,
|
||||
)
|
||||
)
|
||||
session.commit()
|
||||
with engine.connect() as connection:
|
||||
before = list(
|
||||
connection.execute(
|
||||
text("SELECT * FROM access_accounts")
|
||||
).mappings()
|
||||
)
|
||||
tables = set(inspect(connection).get_table_names())
|
||||
command.upgrade(config, "e9a2c5f8b1d4")
|
||||
command.upgrade(config, "e9a2c5f8b1d4")
|
||||
with engine.connect() as connection:
|
||||
inspector = inspect(connection)
|
||||
self.assertEqual(
|
||||
tables | {"access_password_recoveries"},
|
||||
set(inspector.get_table_names()),
|
||||
)
|
||||
self.assertEqual(
|
||||
before,
|
||||
list(
|
||||
connection.execute(
|
||||
text("SELECT * FROM access_accounts")
|
||||
).mappings()
|
||||
),
|
||||
)
|
||||
self.assertEqual(
|
||||
{
|
||||
"id",
|
||||
"account_id",
|
||||
"issuer_account_id",
|
||||
"issuer_membership_id",
|
||||
"code_hash",
|
||||
"expires_at",
|
||||
"consumed_at",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
},
|
||||
{
|
||||
column["name"]
|
||||
for column in inspector.get_columns(
|
||||
"access_password_recoveries"
|
||||
)
|
||||
},
|
||||
)
|
||||
self.assertEqual(
|
||||
3,
|
||||
len(
|
||||
inspector.get_foreign_keys("access_password_recoveries")
|
||||
),
|
||||
)
|
||||
self.assertIn(
|
||||
["code_hash"],
|
||||
[
|
||||
constraint["column_names"]
|
||||
for constraint in inspector.get_unique_constraints(
|
||||
"access_password_recoveries"
|
||||
)
|
||||
],
|
||||
)
|
||||
finally:
|
||||
engine.dispose()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,215 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import threading
|
||||
import unittest
|
||||
import uuid
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from unittest.mock import patch
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import create_engine, text
|
||||
from sqlalchemy.engine import make_url
|
||||
|
||||
import test_password_recovery as recovery_fixture
|
||||
from govoplan_access.backend.api.v1 import auth
|
||||
from govoplan_access.backend.auth.principal_cache import principal_summary_cache
|
||||
from govoplan_access.backend.db.models import PasswordRecovery
|
||||
from govoplan_access.backend.security.passwords import verify_password
|
||||
|
||||
|
||||
@unittest.skipUnless(
|
||||
os.environ.get("GOVOPLAN_ACCESS_TEST_POSTGRES_URL"),
|
||||
"set GOVOPLAN_ACCESS_TEST_POSTGRES_URL to a disposable PostgreSQL test database",
|
||||
)
|
||||
class PasswordRecoveryPostgresTests(unittest.TestCase):
|
||||
"""Opt-in real transaction races; no application/default database fallback."""
|
||||
|
||||
def setUp(self) -> None:
|
||||
url = make_url(os.environ["GOVOPLAN_ACCESS_TEST_POSTGRES_URL"])
|
||||
if url.get_backend_name() != "postgresql" or not url.database:
|
||||
raise ValueError("An explicit disposable PostgreSQL database is required")
|
||||
self.schema = "access_password_race_" + uuid.uuid4().hex
|
||||
self.admin_engine = create_engine(url)
|
||||
self.addCleanup(self.admin_engine.dispose)
|
||||
with self.admin_engine.begin() as connection:
|
||||
connection.execute(text(f'CREATE SCHEMA "{self.schema}"'))
|
||||
self.addCleanup(self._drop_schema)
|
||||
self.engine = create_engine(
|
||||
url,
|
||||
connect_args={
|
||||
"options": (
|
||||
f"-c search_path={self.schema} -c statement_timeout=30000 "
|
||||
"-c lock_timeout=15000 -c idle_in_transaction_session_timeout=60000"
|
||||
)
|
||||
},
|
||||
)
|
||||
self.addCleanup(self.engine.dispose)
|
||||
self.access = recovery_fixture.PasswordRecoveryTests(methodName="runTest")
|
||||
self.addCleanup(self._close_fixture)
|
||||
# Reuse the synthetic HTTP/security fixture, replacing only its SQLite
|
||||
# engine. The production account locks/CAS and request SQL sessions run.
|
||||
with patch.object(recovery_fixture, "create_engine", return_value=self.engine):
|
||||
self.access.setUp()
|
||||
|
||||
def _drop_schema(self) -> None:
|
||||
# The identifier is generated here, never read from the supplied URL.
|
||||
with self.admin_engine.begin() as connection:
|
||||
connection.execute(text(f'DROP SCHEMA "{self.schema}" CASCADE'))
|
||||
|
||||
def _close_fixture(self) -> None:
|
||||
try:
|
||||
if hasattr(self.access, "client"):
|
||||
self.access.client.close()
|
||||
if hasattr(self.access, "db"):
|
||||
self.access.db.close()
|
||||
finally:
|
||||
self.access.doCleanups()
|
||||
principal_summary_cache.clear()
|
||||
|
||||
def _post(self, endpoint, payload, headers=None):
|
||||
# Separate cookie jars and real per-request SQL sessions in each thread.
|
||||
with TestClient(self.access.client.app) as client:
|
||||
return client.post(
|
||||
"/api/v1/auth/password/" + endpoint,
|
||||
json=payload,
|
||||
headers=headers or {},
|
||||
)
|
||||
|
||||
def _race(self, endpoint, payloads, *, headers=None, locked_account="person"):
|
||||
barrier = threading.Barrier(2)
|
||||
real_lock = auth.locked_local_account
|
||||
|
||||
def synchronized_lock(session, account_id):
|
||||
if account_id == locked_account:
|
||||
barrier.wait(timeout=10)
|
||||
return real_lock(session, account_id)
|
||||
|
||||
with patch.object(auth, "locked_local_account", side_effect=synchronized_lock):
|
||||
with ThreadPoolExecutor(max_workers=2) as executor:
|
||||
futures = [
|
||||
executor.submit(self._post, endpoint, payload, headers)
|
||||
for payload in payloads
|
||||
]
|
||||
return [future.result(timeout=35) for future in futures]
|
||||
|
||||
def test_same_recovery_code_has_exactly_one_winner(self):
|
||||
issued = self.access.issue()
|
||||
self.assertEqual(200, issued.status_code)
|
||||
code = issued.json()["recovery_code"]
|
||||
passwords = ["Concurrent-recovery-one", "Concurrent-recovery-two"]
|
||||
responses = self._race(
|
||||
"recover",
|
||||
[
|
||||
{
|
||||
"email": "person@example.test",
|
||||
"recovery_code": code,
|
||||
"new_password": value,
|
||||
}
|
||||
for value in passwords
|
||||
],
|
||||
)
|
||||
self.assertEqual([200, 400], sorted(item.status_code for item in responses))
|
||||
winner = next(
|
||||
index for index, item in enumerate(responses) if item.status_code == 200
|
||||
)
|
||||
self.access.db.expire_all()
|
||||
self.assertTrue(
|
||||
verify_password(passwords[winner], self.access.account.password_hash)
|
||||
)
|
||||
self.assertIsNotNone(self.access.db.query(PasswordRecovery).one().consumed_at)
|
||||
|
||||
def test_same_old_password_has_exactly_one_winner(self):
|
||||
passwords = ["Concurrent-change-one", "Concurrent-change-two"]
|
||||
responses = self._race(
|
||||
"change",
|
||||
[
|
||||
{"current_password": "Initial-password", "new_password": value}
|
||||
for value in passwords
|
||||
],
|
||||
headers=self.access.headers(),
|
||||
)
|
||||
self.assertEqual([200, 401], sorted(item.status_code for item in responses))
|
||||
winner = next(
|
||||
index for index, item in enumerate(responses) if item.status_code == 200
|
||||
)
|
||||
self.access.db.expire_all()
|
||||
self.assertTrue(
|
||||
verify_password(passwords[winner], self.access.account.password_hash)
|
||||
)
|
||||
|
||||
def test_competing_issuance_leaves_exactly_one_usable_code(self):
|
||||
responses = self._race(
|
||||
"recovery/person",
|
||||
[{"current_password": "Owner-password", "identity_verified": True}] * 2,
|
||||
headers=self.access.headers(self.access.owner_session),
|
||||
locked_account="owner",
|
||||
)
|
||||
self.assertEqual([200, 200], [item.status_code for item in responses])
|
||||
codes = [item.json()["recovery_code"] for item in responses]
|
||||
self.assertEqual(2, len(set(codes)))
|
||||
self.access.db.expire_all()
|
||||
self.assertEqual(
|
||||
1,
|
||||
self.access.db.query(PasswordRecovery)
|
||||
.filter(PasswordRecovery.consumed_at.is_(None))
|
||||
.count(),
|
||||
)
|
||||
self.assertEqual(
|
||||
[200, 400], sorted(self.access.recover(code).status_code for code in codes)
|
||||
)
|
||||
|
||||
def test_issuer_password_revocation_wins_paused_redemption(self):
|
||||
issued = self.access.issue()
|
||||
self.assertEqual(200, issued.status_code)
|
||||
code = issued.json()["recovery_code"]
|
||||
reached = threading.Event()
|
||||
proceed = threading.Event()
|
||||
real_authorized = auth.recovery_issuer_authorized
|
||||
|
||||
def paused_authorization(*args, **kwargs):
|
||||
reached.set()
|
||||
self.assertTrue(proceed.wait(timeout=35))
|
||||
return real_authorized(*args, **kwargs)
|
||||
|
||||
with patch.object(
|
||||
auth, "recovery_issuer_authorized", side_effect=paused_authorization
|
||||
):
|
||||
with ThreadPoolExecutor(max_workers=1) as executor:
|
||||
redemption = executor.submit(
|
||||
self._post,
|
||||
"recover",
|
||||
{
|
||||
"email": "person@example.test",
|
||||
"recovery_code": code,
|
||||
"new_password": "Must-not-replace-password",
|
||||
},
|
||||
)
|
||||
try:
|
||||
self.assertTrue(reached.wait(timeout=10))
|
||||
changed = self._post(
|
||||
"change",
|
||||
{
|
||||
"current_password": "Owner-password",
|
||||
"new_password": "Owner-race-password",
|
||||
},
|
||||
self.access.headers(self.access.owner_session),
|
||||
)
|
||||
self.assertEqual(200, changed.status_code)
|
||||
finally:
|
||||
proceed.set()
|
||||
rejected = redemption.result(timeout=35)
|
||||
self.assertEqual(400, rejected.status_code)
|
||||
self.assertEqual("recovery_invalid", rejected.json()["detail"]["code"])
|
||||
self.access.db.expire_all()
|
||||
self.assertTrue(
|
||||
verify_password("Initial-password", self.access.account.password_hash)
|
||||
)
|
||||
self.assertTrue(
|
||||
verify_password("Owner-race-password", self.access.owner.password_hash)
|
||||
)
|
||||
self.assertIsNotNone(self.access.db.query(PasswordRecovery).one().consumed_at)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user