usernames are required, and unique case-insensitively
two platform fixes that make a plugin's job smaller, both from the owner: downstream services identify a person by name, so the platform has to be the one holding that name to a single meaning. `getUserByUsername` compared with `eq`, so `diogo` and `DIOGO` were two accounts. create-user.ts asks "is this taken?" through that function, which means the check could not see its own collision. it compares on `lower()` now. and the constraint is on `lower(username)`, not the column — a plain unique index is what allowed both to exist. it sits in the schema rather than only in create-user.ts so a second code path, a migration, or someone at a psql prompt cannot get around it. the column is NOT NULL. `validateUsername` has always demanded one, but the column allowed null, so anything skipping validation could write a user nobody could name — and a Linux account or a Jellyfin login then has nothing to work with. both databases were checked before the change: zero null usernames, zero case-collisions on either, so no migration was needed. proved rather than assumed, inside a transaction that was rolled back on the live database: `PROBEUSER` refused against `probeuser` by uq_users_username_lower, and a null username refused by the not-null. no rows written. this also removes a case the jellyfin reconcile had to carry: an officer account with no username could not be matched or created there, because jellyfin authenticates by name and has no email login.
This commit is contained in:
@@ -56,8 +56,23 @@ export async function getUserByEmail(email: string): Promise<UserSelect | undefi
|
|||||||
// `username` is unique in the schema, so this exists to turn a would-be constraint violation into a
|
// `username` is unique in the schema, so this exists to turn a would-be constraint violation into a
|
||||||
// sentence. Creating an account is a form someone fills in, and "duplicate key value violates unique
|
// sentence. Creating an account is a form someone fills in, and "duplicate key value violates unique
|
||||||
// constraint users_username_unique" is not an answer to give them.
|
// constraint users_username_unique" is not an answer to give them.
|
||||||
|
/**
|
||||||
|
* Case-INSENSITIVE, deliberately.
|
||||||
|
*
|
||||||
|
* This compared with `eq` until 2026-08-20, which made `diogo` and `DIOGO` two different accounts. That
|
||||||
|
* is a problem on its own — a person typing their own name with the wrong shift key is not a different
|
||||||
|
* person — and it becomes a real one downstream: Jellyfin enforces uniqueness on an uppercased
|
||||||
|
* `NormalizedUsername`, so two Officer accounts that Officer thinks are distinct collide there and the
|
||||||
|
* second cannot be provisioned at all.
|
||||||
|
*
|
||||||
|
* The unique index in `schema.ts` is on `lower(username)` for the same reason, so this cannot be the only
|
||||||
|
* thing standing between two colliding names.
|
||||||
|
*/
|
||||||
export async function getUserByUsername(username: string): Promise<UserSelect | undefined> {
|
export async function getUserByUsername(username: string): Promise<UserSelect | undefined> {
|
||||||
const [user] = await db.select().from(users).where(eq(users.username, username));
|
const [user] = await db
|
||||||
|
.select()
|
||||||
|
.from(users)
|
||||||
|
.where(sql`lower(${users.username}) = lower(${username})`);
|
||||||
return user;
|
return user;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -34,7 +34,14 @@ export const users = pgTable(
|
|||||||
// authorisation cannot accidentally mint an admin.
|
// authorisation cannot accidentally mint an admin.
|
||||||
role: text('role', { enum: USER_ROLES }).notNull().default('Member'),
|
role: text('role', { enum: USER_ROLES }).notNull().default('Member'),
|
||||||
name: text('name'),
|
name: text('name'),
|
||||||
username: text('username').unique(),
|
/**
|
||||||
|
* Required, and unique case-INSENSITIVELY — see the index below.
|
||||||
|
*
|
||||||
|
* `validateUsername` has always demanded one, but the column allowed null, so a code path that
|
||||||
|
* skipped validation could write a user nobody could name. Anything downstream that identifies a
|
||||||
|
* person by username — a Linux account, a Jellyfin login — then has nothing to work with.
|
||||||
|
*/
|
||||||
|
username: text('username').notNull(),
|
||||||
/**
|
/**
|
||||||
* The Linux account this platform account runs as, when per-user OS accounts are enabled.
|
* The Linux account this platform account runs as, when per-user OS accounts are enabled.
|
||||||
*
|
*
|
||||||
@@ -69,6 +76,15 @@ export const users = pgTable(
|
|||||||
// Two accounts must not share one Linux user — that would make "whose uid is this" ambiguous for a
|
// Two accounts must not share one Linux user — that would make "whose uid is this" ambiguous for a
|
||||||
// shell. An index rather than a constraint: see the note on the column.
|
// shell. An index rather than a constraint: see the note on the column.
|
||||||
uniqueIndex('uq_users_os_user').on(table.osUser),
|
uniqueIndex('uq_users_os_user').on(table.osUser),
|
||||||
|
// Usernames are unique CASE-INSENSITIVELY. `diogo` and `DIOGO` are one name.
|
||||||
|
//
|
||||||
|
// On `lower(username)` rather than the column, because a plain unique index would let both exist —
|
||||||
|
// which was the state until 2026-08-20. Two consequences, and the second is why it is a constraint
|
||||||
|
// rather than a check in `create-user.ts`: a person cannot accidentally register a shadow of their
|
||||||
|
// own account, and anything identifying them downstream by name gets one answer. Jellyfin enforces
|
||||||
|
// uniqueness on an uppercased `NormalizedUsername`, so two Officer accounts Officer thought were
|
||||||
|
// distinct collide there, and the second is unprovisionable with no way to see why from this side.
|
||||||
|
uniqueIndex('uq_users_username_lower').on(sql`lower(${table.username})`),
|
||||||
// The owner cannot be demoted. Enforced here rather than in application code because the whole
|
// The owner cannot be demoted. Enforced here rather than in application code because the whole
|
||||||
// point is that it holds "whatever happens" — a stray UPDATE, a migration script, someone at a psql
|
// point is that it holds "whatever happens" — a stray UPDATE, a migration script, someone at a psql
|
||||||
// prompt. Postgres rejects the write; there is no path around it.
|
// prompt. Postgres rejects the write; there is no path around it.
|
||||||
|
|||||||
Reference in New Issue
Block a user