fix(security): make self-service last-credential guard atomic
The self-service credential delete handlers counted credentials and then deleted in separate steps, so concurrent deletes could each see >1 and both proceed, removing the user's last credential and locking them out. Add atomic delete_password_if_not_last / delete_webauthn_if_not_last repo methods (count + delete in one conditional statement) and use them in the manage delete handlers. Removes the now-unused _count_credentials helper. Refs: porchlight-2nv Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
407db57279
commit
1bb76899a5
4 changed files with 88 additions and 15 deletions
|
|
@ -182,3 +182,48 @@ async def test_create_duplicate_webauthn(credential_repo: SQLiteCredentialReposi
|
|||
|
||||
with pytest.raises(DuplicateError):
|
||||
await credential_repo.create_webauthn(cred)
|
||||
|
||||
|
||||
async def test_delete_password_if_not_last_refuses_last(
|
||||
credential_repo: SQLiteCredentialRepository, alice: User
|
||||
) -> None:
|
||||
await credential_repo.create_password(PasswordCredential(user_id=alice.userid, password_hash="h"))
|
||||
|
||||
# Only credential -> must refuse and leave it in place.
|
||||
assert await credential_repo.delete_password_if_not_last(alice.userid) is False
|
||||
assert await credential_repo.get_password_by_user(alice.userid) is not None
|
||||
|
||||
|
||||
async def test_delete_password_if_not_last_allows_when_others_exist(
|
||||
credential_repo: SQLiteCredentialRepository, alice: User
|
||||
) -> None:
|
||||
await credential_repo.create_password(PasswordCredential(user_id=alice.userid, password_hash="h"))
|
||||
await credential_repo.create_webauthn(
|
||||
WebAuthnCredential(user_id=alice.userid, credential_id=b"\x01", public_key=b"\x02")
|
||||
)
|
||||
|
||||
assert await credential_repo.delete_password_if_not_last(alice.userid) is True
|
||||
assert await credential_repo.get_password_by_user(alice.userid) is None
|
||||
|
||||
|
||||
async def test_delete_webauthn_if_not_last_refuses_last(
|
||||
credential_repo: SQLiteCredentialRepository, alice: User
|
||||
) -> None:
|
||||
await credential_repo.create_webauthn(
|
||||
WebAuthnCredential(user_id=alice.userid, credential_id=b"\x01", public_key=b"\x02")
|
||||
)
|
||||
|
||||
assert await credential_repo.delete_webauthn_if_not_last(alice.userid, b"\x01") is False
|
||||
assert len(await credential_repo.get_webauthn_by_user(alice.userid)) == 1
|
||||
|
||||
|
||||
async def test_delete_webauthn_if_not_last_allows_when_others_exist(
|
||||
credential_repo: SQLiteCredentialRepository, alice: User
|
||||
) -> None:
|
||||
await credential_repo.create_webauthn(
|
||||
WebAuthnCredential(user_id=alice.userid, credential_id=b"\x01", public_key=b"\x02")
|
||||
)
|
||||
await credential_repo.create_password(PasswordCredential(user_id=alice.userid, password_hash="h"))
|
||||
|
||||
assert await credential_repo.delete_webauthn_if_not_last(alice.userid, b"\x01") is True
|
||||
assert len(await credential_repo.get_webauthn_by_user(alice.userid)) == 0
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue