diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index c112c32d..eef4708e 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -1,6 +1,68 @@ import Database from 'better-sqlite3'; import { encrypt_api_key } from '../services/apiKeyCrypto'; +export function trimUserWhitespace(db: Database.Database): void { + type DirtyRow = { id: number; username?: string; email?: string }; + + const dirtyUsernames = db.prepare( + `SELECT id, username FROM users WHERE username != TRIM(username)` + ).all() as DirtyRow[]; + + for (const row of dirtyUsernames) { + const trimmed = row.username!.trim(); + const collision = db.prepare( + `SELECT id FROM users WHERE LOWER(username) = LOWER(?) AND id != ?` + ).get(trimmed, row.id) as { id: number } | undefined; + + const final = collision ? `${trimmed}__migrated_${row.id}` : trimmed; + if (collision) { + console.warn( + `[migration] WHITESPACE COLLISION username: user id=${row.id} ` + + `original=${JSON.stringify(row.username)} trimmed="${trimmed}" ` + + `collides with user id=${collision.id}. Renamed to "${final}". ` + + `Manual review required.` + ); + } else { + console.warn( + `[migration] Trimmed username for user id=${row.id}: ` + + `${JSON.stringify(row.username)} → "${final}"` + ); + } + db.prepare(`UPDATE users SET username = ? WHERE id = ?`).run(final, row.id); + } + + const dirtyEmails = db.prepare( + `SELECT id, email FROM users WHERE email != TRIM(email)` + ).all() as DirtyRow[]; + + for (const row of dirtyEmails) { + const trimmed = row.email!.trim(); + const collision = db.prepare( + `SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ?` + ).get(trimmed, row.id) as { id: number } | undefined; + + let final = trimmed; + if (collision) { + const at = trimmed.lastIndexOf('@'); + final = at > 0 + ? `${trimmed.slice(0, at)}__migrated_${row.id}${trimmed.slice(at)}` + : `${trimmed}__migrated_${row.id}`; + console.warn( + `[migration] WHITESPACE COLLISION email: user id=${row.id} ` + + `original=${JSON.stringify(row.email)} trimmed="${trimmed}" ` + + `collides with user id=${collision.id}. Renamed to "${final}". ` + + `User cannot sign in with this email until manually corrected.` + ); + } else { + console.warn( + `[migration] Trimmed email for user id=${row.id}: ` + + `${JSON.stringify(row.email)} → "${final}"` + ); + } + db.prepare(`UPDATE users SET email = ? WHERE id = ?`).run(final, row.id); + } +} + function runMigrations(db: Database.Database): void { db.exec('CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL)'); const versionRow = db.prepare('SELECT version FROM schema_version').get() as { version: number } | undefined; @@ -2147,6 +2209,8 @@ function runMigrations(db: Database.Database): void { db.exec(`INSERT INTO migrations (timestamp, name) VALUES (1777810195344, 'InitialSchema1777810195344');`); db.exec(`INSERT INTO app_settings (key, value) VALUES ('app_version', '${process.env.APP_VERSION || '3.0.14'}')`); }, + // trim leading/trailing whitespace from stored usernames and emails + () => trimUserWhitespace(db), ]; if (currentVersion < migrations.length) { diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts index f0fc5420..b2c50438 100644 --- a/server/src/services/adminService.ts +++ b/server/src/services/adminService.ts @@ -112,7 +112,9 @@ export function createUser(data: { username: string; email: string; password: st } export function updateUser(id: string, data: { username?: string; email?: string; role?: string; password?: string }) { - const { username, email, role, password } = data; + const username = typeof data.username === 'string' ? data.username.trim() : data.username; + const email = typeof data.email === 'string' ? data.email.trim() : data.email; + const { role, password } = data; const user = db.prepare('SELECT * FROM users WHERE id = ?').get(id) as User | undefined; if (!user) return { error: 'User not found', status: 404 }; diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index ba949481..71d61a5a 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -343,7 +343,9 @@ export function registerUser(body: { password?: string; invite_token?: string; }): { error?: string; status?: number; token?: string; user?: Record; auditUserId?: number; auditDetails?: Record } { - const { username, email, password, invite_token } = body; + const username = typeof body.username === 'string' ? body.username.trim() : ''; + const email = typeof body.email === 'string' ? body.email.trim() : ''; + const { password, invite_token } = body; const userCount = (db.prepare('SELECT COUNT(*) as count FROM users').get() as { count: number }).count; diff --git a/server/src/services/oidcService.ts b/server/src/services/oidcService.ts index 42c0c5cd..edce054f 100644 --- a/server/src/services/oidcService.ts +++ b/server/src/services/oidcService.ts @@ -350,7 +350,7 @@ export function findOrCreateUser( config: OidcConfig, inviteToken?: string, ): { user: User } | { error: string } { - const email = userInfo.email!.toLowerCase(); + const email = userInfo.email!.trim().toLowerCase(); const name = userInfo.name || userInfo.preferred_username || email.split('@')[0]; const sub = userInfo.sub; diff --git a/server/tests/integration/admin.test.ts b/server/tests/integration/admin.test.ts index beeaa0a1..e96d2234 100644 --- a/server/tests/integration/admin.test.ts +++ b/server/tests/integration/admin.test.ts @@ -368,6 +368,53 @@ describe('Admin user management', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Admin user management — whitespace normalization +// ───────────────────────────────────────────────────────────────────────────── + +describe('Admin user management — whitespace normalization', () => { + it('ADMIN-UPDATE-TRIM-1 — PUT /admin/users/:id trims username before storing', async () => { + const { user: admin } = createAdmin(testDb); + const { user } = createUser(testDb); + + const res = await request(app) + .put(`/api/admin/users/${user.id}`) + .set('Cookie', authCookie(admin.id)) + .send({ username: ' trimmedadmin ' }); + + expect(res.status).toBe(200); + const row = testDb.prepare('SELECT username FROM users WHERE id = ?').get(user.id) as { username: string }; + expect(row.username).toBe('trimmedadmin'); + }); + + it('ADMIN-UPDATE-TRIM-2 — PUT /admin/users/:id trims email before storing', async () => { + const { user: admin } = createAdmin(testDb); + const { user } = createUser(testDb); + + const res = await request(app) + .put(`/api/admin/users/${user.id}`) + .set('Cookie', authCookie(admin.id)) + .send({ email: ' newemail@example.com ' }); + + expect(res.status).toBe(200); + const row = testDb.prepare('SELECT email FROM users WHERE id = ?').get(user.id) as { email: string }; + expect(row.email).toBe('newemail@example.com'); + }); + + it('ADMIN-UPDATE-TRIM-3 — PUT /admin/users/:id with whitespace-padded username that trims to existing returns 409', async () => { + const { user: admin } = createAdmin(testDb); + const { user: existing } = createUser(testDb, { username: 'carol' }); + const { user: target } = createUser(testDb); + + const res = await request(app) + .put(`/api/admin/users/${target.id}`) + .set('Cookie', authCookie(admin.id)) + .send({ username: ` ${existing.username} ` }); + + expect(res.status).toBe(409); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // System stats // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/auth.test.ts b/server/tests/integration/auth.test.ts index d60dbc0e..fb3c7ea2 100644 --- a/server/tests/integration/auth.test.ts +++ b/server/tests/integration/auth.test.ts @@ -218,6 +218,54 @@ describe('Registration', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Registration — whitespace normalization +// ───────────────────────────────────────────────────────────────────────────── + +describe('Registration — whitespace normalization', () => { + it('AUTH-REG-TRIM-1 — username with surrounding whitespace is trimmed before storage', async () => { + const res = await request(app).post('/api/auth/register').send({ + username: ' trimmeduser ', + email: 'trimmed@example.com', + password: 'Str0ng!Pass', + }); + expect(res.status).toBe(201); + const row = testDb.prepare('SELECT username FROM users WHERE email = ?').get('trimmed@example.com') as { username: string }; + expect(row.username).toBe('trimmeduser'); + }); + + it('AUTH-REG-TRIM-2 — email with surrounding whitespace is trimmed before storage', async () => { + const res = await request(app).post('/api/auth/register').send({ + username: 'emailtrimuser', + email: ' emailtrim@example.com ', + password: 'Str0ng!Pass', + }); + expect(res.status).toBe(201); + const row = testDb.prepare('SELECT email FROM users WHERE username = ?').get('emailtrimuser') as { email: string }; + expect(row.email).toBe('emailtrim@example.com'); + }); + + it('AUTH-REG-TRIM-3 — whitespace-padded username that trims to existing username returns 409', async () => { + createUser(testDb, { username: 'alice', email: 'alice@example.com' }); + const res = await request(app).post('/api/auth/register').send({ + username: ' alice ', + email: 'alice2@example.com', + password: 'Str0ng!Pass', + }); + expect(res.status).toBe(409); + }); + + it('AUTH-REG-TRIM-4 — whitespace-padded email that trims to existing email returns 409', async () => { + createUser(testDb, { username: 'bob', email: 'bob@example.com' }); + const res = await request(app).post('/api/auth/register').send({ + username: 'bob2', + email: ' bob@example.com ', + password: 'Str0ng!Pass', + }); + expect(res.status).toBe(409); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Session / Me // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index 01c718ee..487407cb 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -677,6 +677,20 @@ describe('Trip members', () => { expect(res.body.error).toMatch(/already/i); }); + it('TRIP-013 — Adding a member by whitespace-padded username resolves correctly → 201', async () => { + const { user: owner } = createUser(testDb); + const { user: invitee } = createUser(testDb, { username: 'paddeduser' }); + const trip = createTrip(testDb, owner.id, { title: 'Padded Trip' }); + + const res = await request(app) + .post(`/api/trips/${trip.id}/members`) + .set('Cookie', authCookie(owner.id)) + .send({ identifier: ' paddeduser ' }); + + expect(res.status).toBe(201); + expect(res.body.member.id).toBe(invitee.id); + }); + it('TRIP-014 — DELETE /api/trips/:id/members/:userId removes a member → 200', async () => { const { user: owner } = createUser(testDb); const { user: member } = createUser(testDb); diff --git a/server/tests/unit/services/trimUserWhitespace.test.ts b/server/tests/unit/services/trimUserWhitespace.test.ts new file mode 100644 index 00000000..0ac5afde --- /dev/null +++ b/server/tests/unit/services/trimUserWhitespace.test.ts @@ -0,0 +1,122 @@ +/** + * Unit tests for trimUserWhitespace — the backfill migration that normalises + * leading/trailing whitespace in stored usernames and emails. + * Tests TRIM-MIG-001 through TRIM-MIG-010. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import Database from 'better-sqlite3'; +import { trimUserWhitespace } from '../../../src/db/migrations'; + +function makeDb() { + const db = new Database(':memory:'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec(` + CREATE TABLE users ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + username TEXT UNIQUE NOT NULL, + email TEXT UNIQUE NOT NULL, + password_hash TEXT NOT NULL DEFAULT 'x', + role TEXT NOT NULL DEFAULT 'user' + ) + `); + return db; +} + +function insert(db: Database.Database, username: string, email: string): number { + const r = db.prepare('INSERT INTO users (username, email) VALUES (?, ?)').run(username, email); + return Number(r.lastInsertRowid); +} + +function row(db: Database.Database, id: number) { + return db.prepare('SELECT username, email FROM users WHERE id = ?').get(id) as { username: string; email: string }; +} + +describe('trimUserWhitespace — clean data (no-op)', () => { + it('TRIM-MIG-001 — leaves already-clean rows untouched', () => { + const db = makeDb(); + const id = insert(db, 'alice', 'alice@example.com'); + trimUserWhitespace(db); + expect(row(db, id)).toEqual({ username: 'alice', email: 'alice@example.com' }); + }); +}); + +describe('trimUserWhitespace — non-colliding dirty rows', () => { + it('TRIM-MIG-002 — trims trailing whitespace from username', () => { + const db = makeDb(); + const id = insert(db, 'alice ', 'alice@example.com'); + trimUserWhitespace(db); + expect(row(db, id).username).toBe('alice'); + }); + + it('TRIM-MIG-003 — trims leading whitespace from username', () => { + const db = makeDb(); + const id = insert(db, ' alice', 'alice@example.com'); + trimUserWhitespace(db); + expect(row(db, id).username).toBe('alice'); + }); + + it('TRIM-MIG-004 — trims surrounding whitespace from email', () => { + const db = makeDb(); + const id = insert(db, 'alice', ' alice@example.com '); + trimUserWhitespace(db); + expect(row(db, id).email).toBe('alice@example.com'); + }); + + it('TRIM-MIG-005 — emits a console.warn for each trimmed row', () => { + const db = makeDb(); + insert(db, 'bob ', 'bob@example.com'); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + trimUserWhitespace(db); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('[migration] Trimmed username')); + warn.mockRestore(); + }); +}); + +describe('trimUserWhitespace — username collision handling', () => { + it('TRIM-MIG-006 — renames the dirty row to __migrated_ on collision', () => { + const db = makeDb(); + insert(db, 'carol', 'carol@example.com'); + const dirtyId = insert(db, 'carol ', 'carol2@example.com'); + trimUserWhitespace(db); + expect(row(db, dirtyId).username).toBe(`carol__migrated_${dirtyId}`); + }); + + it('TRIM-MIG-007 — emits a WHITESPACE COLLISION warning for username collision', () => { + const db = makeDb(); + insert(db, 'dan', 'dan@example.com'); + insert(db, 'dan ', 'dan2@example.com'); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + trimUserWhitespace(db); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('WHITESPACE COLLISION username')); + warn.mockRestore(); + }); + + it('TRIM-MIG-008 — the renamed value does not conflict with the existing clean row', () => { + const db = makeDb(); + const cleanId = insert(db, 'eve', 'eve@example.com'); + const dirtyId = insert(db, 'eve ', 'eve2@example.com'); + trimUserWhitespace(db); + expect(row(db, cleanId).username).toBe('eve'); + expect(row(db, dirtyId).username).toBe(`eve__migrated_${dirtyId}`); + }); +}); + +describe('trimUserWhitespace — email collision handling', () => { + it('TRIM-MIG-009 — renames dirty email as __migrated_@ on collision', () => { + const db = makeDb(); + insert(db, 'frank', 'frank@example.com'); + const dirtyId = insert(db, 'frank2', ' frank@example.com '); + trimUserWhitespace(db); + expect(row(db, dirtyId).email).toBe(`frank__migrated_${dirtyId}@example.com`); + }); + + it('TRIM-MIG-010 — emits a WHITESPACE COLLISION warning for email collision', () => { + const db = makeDb(); + insert(db, 'grace', 'grace@example.com'); + insert(db, 'grace2', 'grace@example.com '); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + trimUserWhitespace(db); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('WHITESPACE COLLISION email')); + warn.mockRestore(); + }); +});