fix(employees): purge sessions on password reset; reject empty patches and duplicate emails cleanly

Review findings on the Phase 1-2 employee slice:

- setEmployeePassword now deletes the employee's sessions in the same
  transaction — resetting a suspected-compromised password must not leave
  the attacker's existing 14-day bearer token valid (deactivateEmployee
  already did this; the reset path is the natural lockout action).
- updateEmployee throws 'Nothing to update' when the patch resolves to
  zero recognized fields, instead of returning 200 and writing a no-op
  before==after audit row; PATCH /employees/:id with {} or wrongly-typed
  fields now 400s.
- createEmployee pre-checks UNIQUE(email) inside the transaction and
  throws 'Email already in use' instead of surfacing the raw
  engine-specific 'UNIQUE constraint failed: staff_user.email' text
  (would differ on Postgres, the locked production engine — D15).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 5 days ago
parent 3045b889ff
commit 5865aafe06

@ -64,6 +64,11 @@ export function createEmployee(db: DB, userId: string, input: {
const id = uuidv7()
const { salt, hash } = hashPin(input.password) // generic scrypt; PIN digit policy not applied
return db.transaction(() => {
// Pre-check the UNIQUE(email) so the caller gets a friendly, engine-neutral
// message instead of raw driver text (SQLite and Postgres word it differently).
if (db.prepare(`SELECT 1 FROM staff_user WHERE email=?`).get(email) !== undefined) {
throw new Error('Email already in use')
}
db.prepare(
`INSERT INTO staff_user (id, email, display_name, role, pw_salt, pw_hash) VALUES (?, ?, ?, ?, ?, ?)`,
).run(id, email, input.displayName, input.role, salt, hash)
@ -92,17 +97,20 @@ export function updateEmployee(db: DB, userId: string, id: string, patch: {
sets.push('display_name=?'); args.push(patch.displayName)
}
if (patch.role !== undefined) { sets.push('role=?'); args.push(patch.role) }
if (sets.length > 0) {
args.push(id)
db.prepare(`UPDATE staff_user SET ${sets.join(', ')} WHERE id=?`).run(...args)
}
// An empty patch is a caller error — succeeding silently would write a
// no-op audit row and tell the caller a change landed when nothing did.
if (sets.length === 0) throw new Error('Nothing to update')
args.push(id)
db.prepare(`UPDATE staff_user SET ${sets.join(', ')} WHERE id=?`).run(...args)
const after = getEmployee(db, id)!
writeAudit(db, userId, 'update', 'staff_user', id, before, after)
return after
})()
}
/** Reset an employee's password. Audits the action; never logs the hash. */
/** Reset an employee's password. Audits the action; never logs the hash.
* Existing sessions are purged in the same transaction a reset is the natural
* "lock out the old credential" action, so a stolen token must not outlive it. */
export function setEmployeePassword(db: DB, userId: string, id: string, password: string): void {
if (password.length < 8) throw new Error('Password must be at least 8 characters')
const { salt, hash } = hashPin(password)
@ -110,6 +118,7 @@ export function setEmployeePassword(db: DB, userId: string, id: string, password
const before = getEmployee(db, id)
if (!before) throw new Error('Employee not found')
db.prepare(`UPDATE staff_user SET pw_salt=?, pw_hash=? WHERE id=?`).run(salt, hash, id)
db.prepare(`DELETE FROM session WHERE staff_id=?`).run(id)
writeAudit(db, userId, 'reset_password', 'staff_user', id)
})()
}

@ -121,6 +121,20 @@ describe('/employees routes', () => {
expect((await call(token, 'POST', '/employees/nope/password', { password: 'password-9' })).status).toBe(404)
})
it('PATCH with no recognized fields is a 400, not a silent 200 no-op', async () => {
const token = await tokenOf('owner@test.in', 'owner-password')
const created = await call(token, 'POST', '/employees', {
email: 'noop@test.in', displayName: 'Noop', role: 'staff', password: 'password-9',
})
const id = created.json.employee.id as string
expect((await call(token, 'PATCH', `/employees/${id}`, {})).status).toBe(400)
// wrongly-typed field is dropped by the route → resolves to an empty patch → 400
expect((await call(token, 'PATCH', `/employees/${id}`, { role: 123 })).status).toBe(400)
// and the employee is untouched
const after = await call(token, 'GET', '/employees')
expect(after.json.employees.find((e: { id: string }) => e.id === id)).toMatchObject({ displayName: 'Noop', role: 'staff' })
})
it('mutations act as the signed-in user (audit userId = actor)', async () => {
const token = await tokenOf('owner@test.in', 'owner-password')
const created = await call(token, 'POST', '/employees', {

@ -72,6 +72,34 @@ describe('employee foundation', () => {
expect(login(db, 's@x.co', 'password2')).not.toBeNull()
})
it('password reset invalidates existing sessions immediately (compromised-credential lockout)', () => {
const { db, ownerId } = withOwner()
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
const session = login(db, 's@x.co', 'password2')!
expect(verifySession(db, session.token)).toMatchObject({ id: emp.id })
setEmployeePassword(db, ownerId, emp.id, 'new-secret-9')
expect(verifySession(db, session.token)).toBeNull()
const rows = db.prepare(`SELECT COUNT(*) AS n FROM session WHERE staff_id=?`).get(emp.id) as { n: number }
expect(rows.n).toBe(0)
})
it('rejects a duplicate email with a friendly, engine-neutral message', () => {
const { db, ownerId } = withOwner()
createEmployee(db, ownerId, { email: 'dup@x.co', displayName: 'A', role: 'staff', password: 'password1' })
expect(() =>
createEmployee(db, ownerId, { email: 'Dup@X.co', displayName: 'B', role: 'staff', password: 'password2' }),
).toThrow(/already in use/i) // not the raw 'UNIQUE constraint failed: …' SQLite text
})
it('rejects an empty patch instead of writing a no-op audit row', () => {
const { db, ownerId } = withOwner()
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })
const auditBefore = listAudit(db).filter((a) => a.entity === 'staff_user' && a.entity_id === emp.id).length
expect(() => updateEmployee(db, ownerId, emp.id, {})).toThrow(/nothing to update/i)
const auditAfter = listAudit(db).filter((a) => a.entity === 'staff_user' && a.entity_id === emp.id).length
expect(auditAfter).toBe(auditBefore)
})
it('password reset works and is audited without the hash', () => {
const { db, ownerId } = withOwner()
const emp = createEmployee(db, ownerId, { email: 's@x.co', displayName: 'Stf', role: 'staff', password: 'password2' })

Loading…
Cancel
Save