wtclaude
6e61146678
Self-service account security had three URL surfaces onto one controller. All
three mounted the same `admin/account.controller.js` handlers; each of the three
router files carried a header comment apologising for the arrangement.
`/auth/me/account` was already a strict superset, which settles which to keep:
/admin/account 6 routes noindex, isLoggedIn, staffOnly
/player/account 8 routes noindex, requireAuth
/auth/me/account 10 routes noindex, requireAuth
Neither of the deleted surfaces carried recovery codes, and /admin/account
carried no username or password change at all — so client.js already called
/auth/me/account/recovery-codes/* for two operations on a screen it otherwise
served from /admin/account. The split was leaking before this change.
Gating is equivalent where it overlapped: /player and /auth/me apply identical
`noindex, requireAuth`, and `staffOnly` on /admin/account was strictly narrower
while buying nothing, since every handler is self-scoped to req.user.id. There
is no CSRF layer to differ.
- 14 routes deleted, 0 added, no handler changed.
- account.controller.js moves router/v1/admin/ -> router/v1/auth/, beside the
one router that still reaches it.
- Web client: 14 call sites move onto a root-level api.myAccount /
api.changeUsername / ... group, matching the /auth/me methods already there.
- Android app: no change. MeApi.kt was already 100% /auth/me/account/*.
- Two swagger tags, `Admin · Account` and `Player`, were declared only by the
deleted routes and go with them. The orphaned `AccountStatus` schema goes
too; `PlayerAccount` is re-described as the any-role /auth/me/account shape
(the name is kept so existing $refs resolve).
Breaking to the published OpenAPI surface, accepted deliberately: both consumers
are in this org, and deprecate-then-delete would leave the next phase deciding
whether to add routes to surfaces already marked for removal.
Verification: routes.manifest.json shows exactly 14 deletions and 0 additions.
The OpenAPI spec loses the same 14 paths with zero surviving path definitions
changed; its large textual diff is pure reordering, because removing the
first-mounted router shifts every later path. 1203 server tests, 288 client
tests, 53 bot tests green; check:modules, check:hosts and routes:manifest
--check all pass.
Design of record: docs/website/ENGAGEMENT.md Phase 1a. This lands ahead of
engagement Phase 1b, which adds a self-service email field — written once here
rather than three times.
Co-Authored-By: Claude <noreply@anthropic.com>
113 lines
4.6 KiB
JavaScript
113 lines
4.6 KiB
JavaScript
// Point the DB at a closed port BEFORE requiring anything that builds the pool,
|
|
// so the one branch that reaches the DB fails fast instead of hanging the runner.
|
|
process.env.DB_HOST = '127.0.0.1'
|
|
process.env.DB_PORT = '59999'
|
|
|
|
const { test, beforeEach, after } = require('node:test')
|
|
const assert = require('node:assert/strict')
|
|
const bcrypt = require('bcryptjs')
|
|
|
|
const authCtrl = require('../src/router/v1/auth/auth.controller')
|
|
const account = require('../src/router/v1/auth/account.controller')
|
|
const users = require('../src/model/users/users.model')
|
|
const settings = require('../src/model/settings/settings.model')
|
|
const botScore = require('../src/middleware/botScore')
|
|
const lp = require('../src/middleware/loginProtection')
|
|
const db = require('../src/utils/db')
|
|
|
|
after(() => db.close())
|
|
|
|
function mockRes() {
|
|
return {
|
|
statusCode: 200,
|
|
body: null,
|
|
status(c) {
|
|
this.statusCode = c
|
|
return this
|
|
},
|
|
json(b) {
|
|
this.body = b
|
|
return this
|
|
},
|
|
set() {
|
|
return this
|
|
},
|
|
cookie() {
|
|
return this
|
|
},
|
|
}
|
|
}
|
|
|
|
beforeEach(() => {
|
|
botScore._reset()
|
|
lp._reset()
|
|
})
|
|
|
|
// ── Derived public registration flags ─────────────────────────────────────
|
|
test('registrationFlags maps each mode to password/sso booleans', () => {
|
|
assert.deepEqual(settings.registrationFlags('disabled'), { password: false, sso: false })
|
|
assert.deepEqual(settings.registrationFlags('password'), { password: true, sso: false })
|
|
assert.deepEqual(settings.registrationFlags('sso'), { password: false, sso: true })
|
|
assert.deepEqual(settings.registrationFlags('both'), { password: true, sso: true })
|
|
})
|
|
|
|
test('REGISTRATION_MODES is the closed set of allowed values', () => {
|
|
assert.deepEqual(settings.REGISTRATION_MODES, ['disabled', 'password', 'sso', 'both'])
|
|
})
|
|
|
|
// ── Null-hash password rule ────────────────────────────────────────────────
|
|
test('validatePassword rejects an SSO-only account with a null hash', async () => {
|
|
assert.equal(await users.validatePassword({ password_hash: null }, 'anything'), false)
|
|
assert.equal(await users.validatePassword(null, 'anything'), false)
|
|
})
|
|
|
|
test('validatePassword accepts a correct password against a real hash', async () => {
|
|
const password_hash = await bcrypt.hash('correct horse', 10)
|
|
assert.equal(await users.validatePassword({ password_hash }, 'correct horse'), true)
|
|
assert.equal(await users.validatePassword({ password_hash }, 'wrong'), false)
|
|
})
|
|
|
|
test('isDuplicateUsername recognizes the driver duplicate-key error', () => {
|
|
assert.equal(users.isDuplicateUsername({ code: 'ER_DUP_ENTRY' }), true)
|
|
assert.equal(users.isDuplicateUsername({ errno: 1062 }), true)
|
|
assert.equal(users.isDuplicateUsername({ code: 'ER_NO_SUCH_TABLE' }), false)
|
|
assert.equal(users.isDuplicateUsername(null), false)
|
|
})
|
|
|
|
// ── getAccount.has_password reads the RAW row ─────────────────────────────
|
|
// Regression: req.user is the sanitized row (password_hash stripped), so
|
|
// has_password must come from users.getRawById, not req.user.password_hash —
|
|
// otherwise a real password account is mis-rendered as "set a password".
|
|
test('getAccount reports has_password from the raw row, not the sanitized req.user', async () => {
|
|
const origGetRaw = users.getRawById
|
|
try {
|
|
users.getRawById = async () => ({ id: 1, password_hash: '$2a$hash' }) // has a password
|
|
const req = { user: { id: 1, username: 'p', role: 'player', status: 'active', totp_enabled: 0 } } // sanitized: no hash
|
|
const res = mockRes()
|
|
await account.getAccount(req, res)
|
|
assert.equal(res.body.has_password, true)
|
|
|
|
users.getRawById = async () => ({ id: 1, password_hash: null }) // SSO-only, no password
|
|
const res2 = mockRes()
|
|
await account.getAccount(req, res2)
|
|
assert.equal(res2.body.has_password, false)
|
|
} finally {
|
|
users.getRawById = origGetRaw
|
|
}
|
|
})
|
|
|
|
// ── Registration honeypot (does not need the DB) ──────────────────────────
|
|
test('register with a filled honeypot fails and bans the IP before any DB hit', async () => {
|
|
const ip = '203.0.113.90'
|
|
const req = {
|
|
ip,
|
|
body: { username: 'newplayer', password: 'password123', [authCtrl.HONEYPOT_FIELD]: 'Acme' },
|
|
}
|
|
const res = mockRes()
|
|
await authCtrl.register(req, res)
|
|
|
|
assert.equal(res.statusCode, 400)
|
|
assert.doesNotMatch(res.body.message, /honeypot|bot|company/i)
|
|
assert.equal(botScore.isBanned(ip), true)
|
|
})
|