Skip to content

fix(jdbi): make username uniqueness case-insensitive to match DynamoDB - #150

Merged
wolpert merged 1 commit into
mainfrom
fix/jdbi-username-case-insensitive
Aug 5, 2026
Merged

fix(jdbi): make username uniqueness case-insensitive to match DynamoDB#150
wolpert merged 1 commit into
mainfrom
fix/jdbi-username-case-insensitive

Conversation

@wolpert

@wolpert wolpert commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Closes finding 5 of the full-project security audit.

The problem

DynamoDbUserLookup keys identity on USERNAME#<lower(username)>, so Admin and admin are one account on that backend. JdbiUserLookup matched exactly against V5's plain UNIQUE constraint, so on Postgres they were two.

Same library, same SPI, two identity models. A host that assumed the DynamoDB semantics — or moved between backends — could end up with look-alike accounts.

The fix

Flyway V12 adds a unique index on lower(username); the lookup queries fold case to match. getOrCreateHandle's ON CONFLICT now infers that expression index, so a racing insert of any case variant resolves to the existing row instead of minting a second handle.

The stored username keeps its original casing — only uniqueness and lookup fold — mirroring DynamoDB's UserItem, which stores the supplied form and lower-cases only the key. Display values round-trip unchanged.

PkAuthJdbiSchema.CURRENT_SCHEMA_VERSION12.

⚠️ Upgrade note: the migration can refuse to run

V12 runs a pre-flight check and refuses to migrate, naming the conflicting groups, if the database already holds usernames differing only by case:

pk-auth V12 cannot make username uniqueness case-insensitive: 1 existing username(s)
differ only by case. Conflicting groups: admin -> [ADMIN, Admin, admin]. Resolve these
first — decide which row is authoritative, migrate or delete the credentials belonging
to the others (credentials.user_handle references users.user_handle), then re-run.

Which of Admin and admin is authoritative, and what becomes of the other's credentials, is a business decision. A schema change shouldn't make it silently — nor should the operator meet it as an opaque duplicate-key error from the index build.

Testing

The guard only executes on a dirty database, which is exactly the code that would otherwise ship untested. V12UsernameCaseMigrationGuardIntegrationTest drives it on real Postgres (its own container, migrated to V11 then seeded):

  • seeded conflicts → refused, all three variants named, and no index created so a re-run can succeed
  • clean data → migrates, and the index then rejects a case variant
  • the documented remediation (drop the loser, re-run) → succeeds

Plus case-insensitivity coverage on JdbiUserLookup itself. ./gradlew :pk-auth-persistence-jdbi:check passes (Docker required).

Also refreshes the DESIGN.md schema paragraph, which had drifted to "10".

🤖 Generated with Claude Code

@wolpert
wolpert force-pushed the fix/jdbi-username-case-insensitive branch from 8e05feb to 0a89956 Compare August 5, 2026 15:12
@wolpert
wolpert enabled auto-merge (rebase) August 5, 2026 15:19
@wolpert
wolpert force-pushed the fix/jdbi-username-case-insensitive branch from 0a89956 to 23ec0dd Compare August 5, 2026 15:21
DynamoDbUserLookup keys identity on `USERNAME#<lower(username)>`, so "Admin" and
"admin" are one account on that backend. JdbiUserLookup matched exactly against
V5's plain UNIQUE constraint, so on Postgres they were two. Same library, same
SPI, two identity models — a host that assumed the DynamoDB semantics, or moved
between backends, could end up with look-alike accounts.

Flyway V12 adds a unique index on lower(username) and the lookup queries fold
case to match. getOrCreateHandle's ON CONFLICT now infers that expression index,
so a racing insert of any case variant resolves to the existing row instead of
minting a second handle. The stored username keeps its original casing — only
uniqueness and lookup fold — mirroring DynamoDB's UserItem, which stores the
supplied form and lower-cases only the key.

The migration refuses to run on a database that already holds case-duplicate
usernames, and names the offending groups. Which of "Admin" and "admin" is
authoritative, and what becomes of the other's credentials, is a business
decision; a schema change should not make it silently, nor should the operator
meet it as an opaque duplicate-key error from the index build.

That guard only executes on a dirty database, which is exactly the case that
would otherwise ship untested, so V12UsernameCaseMigrationGuardIntegrationTest
drives it on real Postgres: seeded conflicts are refused with all variants named
and no index created, clean data migrates and is then enforced by the index, and
the documented remediation (drop the loser, re-run) succeeds.

Also refreshes the DESIGN.md schema paragraph, which had drifted to "10".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@wolpert
wolpert force-pushed the fix/jdbi-username-case-insensitive branch from 23ec0dd to 3937b71 Compare August 5, 2026 15:26
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@wolpert
wolpert merged commit 319145e into main Aug 5, 2026
8 checks passed
@wolpert
wolpert deleted the fix/jdbi-username-case-insensitive branch August 5, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant