fix(security): enforce globally-unique WebAuthn credential_id
The webauthn_credentials primary key is (user_id, credential_id), which does not stop the same credential_id from existing under two users. Usernameless authentication looks up the credential by id alone, so a duplicate could resolve to the wrong account. Add a unique index on credential_id (migration 003); duplicate registration now raises DuplicateError. Refs: porchlight-as2 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0f04a7daf9
commit
f03d509eb4
3 changed files with 25 additions and 2 deletions
|
|
@ -0,0 +1,5 @@
|
||||||
|
-- A WebAuthn credential_id must be globally unique: lookups by credential_id
|
||||||
|
-- alone (during usernameless authentication) must never resolve to the wrong
|
||||||
|
-- user. The table PK is (user_id, credential_id), which does not prevent the
|
||||||
|
-- same credential_id under two users, so add an explicit unique index.
|
||||||
|
CREATE UNIQUE INDEX ix_webauthn_credential_id ON webauthn_credentials (credential_id);
|
||||||
|
|
@ -13,7 +13,7 @@ async def test_run_migrations_applies_initial() -> None:
|
||||||
async with aiosqlite.connect(":memory:") as db:
|
async with aiosqlite.connect(":memory:") as db:
|
||||||
await db.execute("PRAGMA foreign_keys=ON")
|
await db.execute("PRAGMA foreign_keys=ON")
|
||||||
count = await run_migrations(db, MIGRATIONS_DIR)
|
count = await run_migrations(db, MIGRATIONS_DIR)
|
||||||
assert count == 2
|
assert count == 3
|
||||||
async with db.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='users'") as cursor:
|
async with db.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='users'") as cursor:
|
||||||
row = await cursor.fetchone()
|
row = await cursor.fetchone()
|
||||||
assert row is not None
|
assert row is not None
|
||||||
|
|
@ -24,7 +24,7 @@ async def test_run_migrations_skips_already_applied() -> None:
|
||||||
await db.execute("PRAGMA foreign_keys=ON")
|
await db.execute("PRAGMA foreign_keys=ON")
|
||||||
first_count = await run_migrations(db, MIGRATIONS_DIR)
|
first_count = await run_migrations(db, MIGRATIONS_DIR)
|
||||||
second_count = await run_migrations(db, MIGRATIONS_DIR)
|
second_count = await run_migrations(db, MIGRATIONS_DIR)
|
||||||
assert first_count == 2
|
assert first_count == 3
|
||||||
assert second_count == 0
|
assert second_count == 0
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -227,3 +227,21 @@ async def test_delete_webauthn_if_not_last_allows_when_others_exist(
|
||||||
|
|
||||||
assert await credential_repo.delete_webauthn_if_not_last(alice.userid, b"\x01") is True
|
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
|
assert len(await credential_repo.get_webauthn_by_user(alice.userid)) == 0
|
||||||
|
|
||||||
|
|
||||||
|
async def test_webauthn_credential_id_is_globally_unique(
|
||||||
|
credential_repo: SQLiteCredentialRepository, user_repo: SQLiteUserRepository
|
||||||
|
) -> None:
|
||||||
|
# The same credential_id must not be registrable for two different users,
|
||||||
|
# otherwise lookup-by-credential-id could resolve to the wrong account.
|
||||||
|
await user_repo.create(User(userid="u-alice", username="alice2"))
|
||||||
|
await user_repo.create(User(userid="u-bob", username="bob2"))
|
||||||
|
cred_id = b"\xde\xad\xbe\xef"
|
||||||
|
|
||||||
|
await credential_repo.create_webauthn(
|
||||||
|
WebAuthnCredential(user_id="u-alice", credential_id=cred_id, public_key=b"\x01")
|
||||||
|
)
|
||||||
|
with pytest.raises(DuplicateError):
|
||||||
|
await credential_repo.create_webauthn(
|
||||||
|
WebAuthnCredential(user_id="u-bob", credential_id=cred_id, public_key=b"\x02")
|
||||||
|
)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue