wtclaude
fbb4b0bd91
Makes `users.email` unique, de-duplicates the addresses an upgrade will find, and builds the self-service change-and-verify flow that did not exist. The uniqueness index is on a generated `email_norm AS (LOWER(email)) STORED` column under `utf8mb4_bin`, NOT on `email` under a `_ci` collation as the plan specified. Every case-insensitive collation this server offers is also accent-insensitive: `josé@x.com` and `jose@x.com` compare equal, and those are two different mailboxes. The plan's index would have refused the second address forever and the de-duplication would have nulled a legitimate account's. A requested address is STAGED in `email_pending` and only a tokened link installs it, so a typo cannot silently redirect account-recovery mail. `isDuplicateUsername()` now distinguishes the two indexes. All five call sites branch on it; each answers differently on purpose, because a public form, an IdP callback, a half-completed invite and an admin screen do not owe the same person the same amount of truth. SSO reads the IdP's actual `email_verified`/`verified` claim instead of inferring verification from an address merely being present. Co-Authored-By: Claude <noreply@anthropic.com>
88 lines
3.8 KiB
JavaScript
88 lines
3.8 KiB
JavaScript
// Engagement Phase 1b — telling the two unique constraints on `users` apart.
|
|
//
|
|
// This is the piece the whole phase rests on: `users` grew a second unique index,
|
|
// and until Phase 1b the duplicate-key test could not tell which one fired. Every
|
|
// call site that creates or updates a user branches on these predicates, so a
|
|
// wrong answer here means a duplicate email reported as a taken username, an SSO
|
|
// sign-up retrying usernames against a conflict no username can clear, or an
|
|
// opaque 500 on the admin user form.
|
|
//
|
|
// The error strings below are VERBATIM from MariaDB 11.8 through the mariadb Node
|
|
// connector, captured against a real duplicate insert. The key name lives only in
|
|
// the message text — the driver exposes no structured field for it — which is
|
|
// exactly why this needs its own test: it is parsing, and parsing rots silently.
|
|
process.env.DB_HOST = '127.0.0.1'
|
|
process.env.DB_PORT = '59999'
|
|
|
|
const { test, after } = require('node:test')
|
|
const assert = require('node:assert/strict')
|
|
|
|
const users = require('../src/model/users/users.model')
|
|
const db = require('../src/utils/db')
|
|
|
|
after(() => db.close())
|
|
|
|
// Shaped exactly as the connector delivers them, including the trailing `sql:`
|
|
// section — which is also the reason these must never be echoed to a client: note
|
|
// the bound parameters, and therefore the address, are in the text.
|
|
function dupError(key, value) {
|
|
const err = new Error(
|
|
`(conn:60, no: 1062, SQLState: 23000) Duplicate entry '${value}' for key '${key}'\n` +
|
|
`sql: INSERT INTO users (username, email) VALUES (?, ?) - parameters:['someone','${value}']`,
|
|
)
|
|
err.code = 'ER_DUP_ENTRY'
|
|
err.errno = 1062
|
|
err.sqlState = '23000'
|
|
err.sqlMessage = `Duplicate entry '${value}' for key '${key}'`
|
|
return err
|
|
}
|
|
|
|
test('an email collision is reported as email, not username', () => {
|
|
const err = dupError('uq_users_email_norm', 'taken@example.com')
|
|
assert.equal(users.isDuplicateEmail(err), true)
|
|
assert.equal(users.isDuplicateUsername(err), false, 'must NOT masquerade as a username collision')
|
|
assert.equal(users.duplicateKey(err), 'uq_users_email_norm')
|
|
})
|
|
|
|
test('a username collision is still reported as username', () => {
|
|
const err = dupError('username', 'someone')
|
|
assert.equal(users.isDuplicateUsername(err), true)
|
|
assert.equal(users.isDuplicateEmail(err), false)
|
|
assert.equal(users.duplicateKey(err), 'username')
|
|
})
|
|
|
|
// The permissive fallback is deliberate. Only the case we can positively identify
|
|
// — email — is carved out; anything else keeps the pre-Phase-1b behaviour so no
|
|
// call site newly falls through to a 500 on a database whose index carries an
|
|
// unexpected name.
|
|
test('an unrecognised unique index keeps the old permissive behaviour', () => {
|
|
const err = dupError('some_other_uq', 'x')
|
|
assert.equal(users.isDuplicateUsername(err), true)
|
|
assert.equal(users.isDuplicateEmail(err), false)
|
|
})
|
|
|
|
test('a duplicate-key error the message does not name is treated as username', () => {
|
|
const err = new Error('Duplicate entry - no key clause here')
|
|
err.code = 'ER_DUP_ENTRY'
|
|
err.errno = 1062
|
|
assert.equal(users.duplicateKey(err), null)
|
|
assert.equal(users.isDuplicateUsername(err), true)
|
|
assert.equal(users.isDuplicateEmail(err), false)
|
|
})
|
|
|
|
test('non-duplicate errors are neither', () => {
|
|
for (const err of [null, undefined, new Error('boom'), { code: 'ER_NO_SUCH_TABLE' }]) {
|
|
assert.equal(users.isDuplicateUsername(err), false)
|
|
assert.equal(users.isDuplicateEmail(err), false)
|
|
assert.equal(users.duplicateKey(err), null)
|
|
}
|
|
})
|
|
|
|
// errno alone, with no `code`, is how some driver paths surface it.
|
|
test('errno 1062 without a code still counts', () => {
|
|
const err = new Error("Duplicate entry 'a@b.com' for key 'uq_users_email_norm'")
|
|
err.errno = 1062
|
|
assert.equal(users.isDuplicateEmail(err), true)
|
|
assert.equal(users.isDuplicateUsername(err), false)
|
|
})
|