From f03d509eb4c25fcf64e86f803d5465cfa12b09a2 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Fri, 5 Jun 2026 13:51:09 +0200 Subject: [PATCH] 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) --- .../003_webauthn_credential_id_unique.sql | 5 +++++ tests/test_store/test_migrations.py | 4 ++-- .../test_store/test_sqlite_credential_repo.py | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 src/porchlight/store/sqlite/migrations/003_webauthn_credential_id_unique.sql diff --git a/src/porchlight/store/sqlite/migrations/003_webauthn_credential_id_unique.sql b/src/porchlight/store/sqlite/migrations/003_webauthn_credential_id_unique.sql new file mode 100644 index 0000000..23eaa20 --- /dev/null +++ b/src/porchlight/store/sqlite/migrations/003_webauthn_credential_id_unique.sql @@ -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); diff --git a/tests/test_store/test_migrations.py b/tests/test_store/test_migrations.py index 386aa5a..4466d76 100644 --- a/tests/test_store/test_migrations.py +++ b/tests/test_store/test_migrations.py @@ -13,7 +13,7 @@ async def test_run_migrations_applies_initial() -> None: async with aiosqlite.connect(":memory:") as db: await db.execute("PRAGMA foreign_keys=ON") 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: row = await cursor.fetchone() 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") first_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 diff --git a/tests/test_store/test_sqlite_credential_repo.py b/tests/test_store/test_sqlite_credential_repo.py index 1c3396a..3b54b61 100644 --- a/tests/test_store/test_sqlite_credential_repo.py +++ b/tests/test_store/test_sqlite_credential_repo.py @@ -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 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") + )