Harden admin login: RBAC-safe controls, 2FA, bot-scoring, rate limits (#9)

Adds a layered set of protections around the admin login and the app edge.

Trust proxy (server/src/utils/trustProxy.js)
- Configurable via TRUST_PROXY; pin to the newt agent ("ptero") LAN IP so
  X-Forwarded-For is trusted ONLY from that peer. A blanket "true" is
  rejected (coerced to 1) to prevent XFF spoofing that would dodge every
  IP-based control. DEBUG_TRUST_PROXY logs peer/XFF/req.ip to re-verify the
  proxy IP without a redeploy. Documents the Omada static-reservation
  assumption.

Login throttling (server/src/middleware/loginProtection.js, rateLimit.js)
- express-slow-down progressive delay + the existing hard rate cap + a
  separate per-IP exponential backoff that persists across the rate window.
  All failures return one generic message (no user/pass disclosure).

Honeypot (login form + auth.controller)
- Hidden, plausibly-named field ("company"); a filled value fails
  generically and is scored as an unambiguous bot.

Optional per-user TOTP 2FA (speakeasy/qrcode)
- totp_secret/totp_enabled columns (+ idempotent migration). Self-service
  Account page: enroll via QR, confirm a code to enable, code-gated disable.
- Login is two-step for enrolled users: after the password, a short-lived
  signed challenge (stage:'totp', not a session) is required before the
  real session is issued.

Bot / scanner scoring + IP ban (server/src/middleware/botScore.js)
- Weighted CMS-scanner paths (this app uses none). Junk paths 404 FIRST,
  unconditionally — independent of score/ban state, so a scanner rotating
  through fresh Cloudflare IPs gets no free pass. /wp-admin/install.php is
  the top-weighted near-1-hit ban (worst offender in prod logs). Per-IP
  score with quiet-period decay temp-bans an IP from ALL routes once past a
  (deliberately low) threshold, to protect /admin from credential stuffing.
  Failed logins and honeypot hits feed the same score.
- Periodic sweep evicts stale, unbanned, quiet entries so the in-memory
  store can't grow unbounded; the interval is unref'd and cleared on
  graceful shutdown.

Tests: node --test suite (40) covering trust-proxy parsing + live req.ip
(incl. pinned-IP), rate limiter + exponential backoff, honeypot rejection,
TOTP verify (enabled/disabled) + challenge-isn't-a-session, bot-score
threshold/decay/ban + junk-404-independence + install.php + store sweep.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-02 23:22:35 -05:00
parent ad9c556c9a
commit d38c98ad9e
30 changed files with 2038 additions and 54 deletions

21
server/test/_helper.js Normal file
View File

@@ -0,0 +1,21 @@
// Test helper: start a throwaway Express app on an ephemeral port and return its
// base URL + a close(). Uses the built-in fetch (Node 18+) so tests need no
// extra HTTP dependency. Tests here exercise middleware in isolation and do NOT
// touch the database.
const express = require('express')
async function startApp(configure) {
const app = express()
app.use(express.json())
configure(app)
const server = await new Promise((resolve) => {
const s = app.listen(0, '127.0.0.1', () => resolve(s))
})
const { port } = server.address()
return {
url: `http://127.0.0.1:${port}`,
close: () => new Promise((resolve) => server.close(resolve)),
}
}
module.exports = { startApp }

View File

@@ -0,0 +1,192 @@
const { test, beforeEach } = require('node:test')
const assert = require('node:assert/strict')
const botScore = require('../src/middleware/botScore')
const { startApp } = require('./_helper')
beforeEach(() => botScore._reset())
test('scoreForPath: scanner paths score, legitimate app paths do not', () => {
assert.ok(botScore.scoreForPath('/wp-admin') > 0)
assert.ok(botScore.scoreForPath('/wp-login.php') > 0)
assert.ok(botScore.scoreForPath('/.env') > 0)
assert.ok(botScore.scoreForPath('/xmlrpc.php') > 0)
// The real admin path and API are NOT scanner signal.
assert.equal(botScore.scoreForPath('/admin'), 0)
assert.equal(botScore.scoreForPath('/admin/login'), 0)
assert.equal(botScore.scoreForPath('/api/v1/auth/login'), 0)
assert.equal(botScore.scoreForPath('/'), 0)
})
test('/wp-admin/install.php is the single highest-weighted path (near 1-hit ban)', () => {
const install = botScore.scoreForPath('/wp-admin/install.php')
// Higher than the generic /wp-admin prefix (more specific entry wins first)...
assert.ok(install > botScore.scoreForPath('/wp-admin'))
// ...and higher than every other scanner path.
for (const p of ['/.env', '/.git', '/wp-login.php', '/xmlrpc.php', '/phpmyadmin', '/wp-json']) {
assert.ok(install > botScore.scoreForPath(p), `install.php should outweigh ${p}`)
}
// A single hit alone meets/exceeds the ban threshold → effectively a 1-hit ban.
assert.ok(install >= botScore.BAN_THRESHOLD)
})
test('install.php 404s on a first-time-seen IP (before any ban would trigger)', async () => {
const freshIp = '203.0.113.80'
// Precondition: this IP has never been seen, so it is NOT banned yet.
assert.equal(botScore.isBanned(freshIp), false)
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.use(botScore.guard)
a.get('/', (req, res) => res.json({ ok: true }))
})
try {
// The very first hit to the junk path must 404 — the 404 comes from the
// junk-path rule, independent of the (currently empty) ban state.
const res = await fetch(`${app.url}/wp-admin/install.php`, {
headers: { 'X-Forwarded-For': freshIp },
})
assert.equal(res.status, 404)
} finally {
await app.close()
}
})
test('a low-weight junk path 404s on first hit WITHOUT a ban (404 is ban-independent)', async () => {
const freshIp = '203.0.113.81'
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.use(botScore.guard)
a.get('/', (req, res) => res.json({ ok: true }))
})
try {
// /wp-content scores 40 (< threshold 80): the hit 404s but does NOT ban,
// proving the immediate 404 does not depend on the IP having crossed the
// ban threshold.
const res = await fetch(`${app.url}/wp-content/uploads/x.php`, {
headers: { 'X-Forwarded-For': freshIp },
})
assert.equal(res.status, 404)
assert.equal(botScore.isBanned(freshIp), false)
} finally {
await app.close()
}
})
test('addScore bans once the threshold is crossed', () => {
const ip = '198.51.100.1'
const now = 1_000_000
assert.equal(botScore.isBanned(ip, now), false)
botScore.addScore(ip, botScore.BAN_THRESHOLD - 1, now)
assert.equal(botScore.isBanned(ip, now), false) // just under
botScore.addScore(ip, 1, now)
assert.equal(botScore.isBanned(ip, now), true) // at threshold
})
test('a single .env probe (weight 100) is an instant ban', () => {
const ip = '198.51.100.9'
const now = 5_000
botScore.addScore(ip, botScore.scoreForPath('/.env'), now)
assert.equal(botScore.isBanned(ip, now), true)
})
test('login failures + one scan hit ban faster than either alone', () => {
const ip = '198.51.100.2'
const now = 2_000_000
// Two failed logins alone: below threshold, not banned.
botScore.recordLoginFailure(ip, now)
botScore.recordLoginFailure(ip, now)
assert.equal(botScore.isBanned(ip, now), false)
// Add one medium scanner hit (wp-admin, 50) → crosses threshold.
botScore.addScore(ip, botScore.scoreForPath('/wp-admin'), now)
assert.equal(botScore.isBanned(ip, now), true)
})
test('score decays to zero after a quiet period', () => {
const ip = '198.51.100.3'
const t0 = 10_000
botScore.addScore(ip, 50, t0) // below threshold
// Long quiet gap, then another 50 — should NOT ban because the first decayed.
const later = t0 + botScore.QUIET_MS + 1
botScore.addScore(ip, 50, later)
assert.equal(botScore.isBanned(ip, later), false)
assert.equal(botScore._snapshot(ip).score, 50)
})
test('ban persists for the ban window and lifts after it', () => {
const ip = '198.51.100.4'
const now = 3_000_000
botScore.addScore(ip, botScore.BAN_THRESHOLD, now)
assert.equal(botScore.isBanned(ip, now + botScore.BAN_MS - 1), true)
assert.equal(botScore.isBanned(ip, now + botScore.BAN_MS + 1), false)
})
test('sweep removes stale unbanned entries but keeps banned and recent ones', () => {
const t0 = 100_000_000
// (a) stale + unbanned: scored below threshold, then goes quiet past QUIET_MS.
botScore.addScore('10.0.0.1', 50, t0)
// (b) banned + quiet: banned now, and its lastSeen is old at sweep time — must
// survive because the ban is still active.
botScore.addScore('10.0.0.2', botScore.BAN_THRESHOLD, t0)
// (c) recently active: scored just before the sweep, still inside QUIET_MS.
const sweepAt = t0 + botScore.QUIET_MS + 1
botScore.addScore('10.0.0.3', 50, sweepAt)
const removed = botScore.sweep(sweepAt)
assert.equal(removed, 1) // only the stale unbanned entry
assert.equal(botScore._snapshot('10.0.0.1'), null) // (a) evicted
assert.notEqual(botScore._snapshot('10.0.0.2'), null) // (b) banned → survives
assert.equal(botScore.isBanned('10.0.0.2', sweepAt), true)
assert.notEqual(botScore._snapshot('10.0.0.3'), null) // (c) recent → survives
})
test('sweep keeps an unbanned entry that is exactly at the quiet boundary', () => {
const t0 = 200_000_000
botScore.addScore('10.0.1.1', 40, t0)
// now - lastSeen === QUIET_MS (not strictly greater) → not yet evictable.
const removed = botScore.sweep(t0 + botScore.QUIET_MS)
assert.equal(removed, 0)
assert.notEqual(botScore._snapshot('10.0.1.1'), null)
})
test('guard: scanner junk path returns 404, legit path passes through', async () => {
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.use(botScore.guard)
a.get('/', (req, res) => res.json({ ok: true }))
a.get('/admin', (req, res) => res.json({ ok: 'admin' }))
})
try {
const scan = await fetch(`${app.url}/wp-login.php`, { headers: { 'X-Forwarded-For': '203.0.113.20' } })
assert.equal(scan.status, 404)
const ok = await fetch(`${app.url}/admin`, { headers: { 'X-Forwarded-For': '203.0.113.21' } })
assert.equal(ok.status, 200)
} finally {
await app.close()
}
})
test('guard: once banned, an IP gets 404 on ALL routes', async () => {
const bannedIp = '203.0.113.30'
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.use(botScore.guard)
a.get('/', (req, res) => res.json({ ok: true }))
})
try {
// One .env probe → instant ban for this IP.
const probe = await fetch(`${app.url}/.env`, { headers: { 'X-Forwarded-For': bannedIp } })
assert.equal(probe.status, 404)
// Now a normal path from the same IP is also 404.
const blocked = await fetch(`${app.url}/`, { headers: { 'X-Forwarded-For': bannedIp } })
assert.equal(blocked.status, 404)
// A different IP still gets through.
const other = await fetch(`${app.url}/`, { headers: { 'X-Forwarded-For': '203.0.113.31' } })
assert.equal(other.status, 200)
} finally {
await app.close()
}
})

View File

@@ -0,0 +1,76 @@
// Point the DB at a closed port BEFORE requiring anything that builds the pool.
// The only code path here that reaches the database (the empty-honeypot case →
// username lookup) then fails fast with ECONNREFUSED instead of opening a real
// pooled connection that would keep this test process alive and hang 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 authCtrl = require('../src/router/v1/auth/auth.controller')
const botScore = require('../src/middleware/botScore')
const lp = require('../src/middleware/loginProtection')
const db = require('../src/utils/db')
// Release the DB pool so the process can exit cleanly even if a connection was
// created during module load.
after(() => db.close())
// Minimal res double capturing status/json; set() is a no-op for headers.
function mockRes() {
return {
statusCode: 200,
body: null,
status(c) {
this.statusCode = c
return this
},
json(b) {
this.body = b
return this
},
set() {
return this
},
}
}
beforeEach(() => {
botScore._reset()
lp._reset()
})
test('honeypot field name matches what the client renders', () => {
assert.equal(authCtrl.HONEYPOT_FIELD, 'company')
})
test('a filled honeypot fails generically and bans the IP', async () => {
const ip = '203.0.113.70'
const req = {
ip,
body: { username: 'admin', password: 'whatever', [authCtrl.HONEYPOT_FIELD]: 'Acme Corp' },
}
const res = mockRes()
await authCtrl.login(req, res)
// Generic failure — never says the honeypot was the reason.
assert.equal(res.statusCode, 401)
assert.match(res.body.message, /incorrect username or password/i)
assert.doesNotMatch(res.body.message, /honeypot|bot|company/i)
// Scored as an unambiguous bot: instant ban + backoff started.
assert.equal(botScore.isBanned(ip), true)
assert.ok(lp.retryAfterMs(ip) > 0)
})
test('an empty honeypot does NOT trigger bot scoring (branch not taken)', async () => {
const ip = '203.0.113.71'
const req = { ip, body: { username: 'admin', password: 'whatever', [authCtrl.HONEYPOT_FIELD]: '' } }
const res = mockRes()
// With an empty honeypot the code proceeds to the DB lookup, which has no
// connection in this unit test and is caught → 500. The point of this test is
// only that the honeypot branch did not fire, so the IP is not banned.
await authCtrl.login(req, res)
assert.equal(botScore.isBanned(ip), false)
})

View File

@@ -0,0 +1,96 @@
const { test, beforeEach } = require('node:test')
const assert = require('node:assert/strict')
const lp = require('../src/middleware/loginProtection')
const { loginLimiter } = require('../src/middleware/rateLimit')
const { startApp } = require('./_helper')
beforeEach(() => lp._reset())
test('recordFailure escalates the lockout exponentially', () => {
const ip = '198.51.100.50'
const now = 1_000_000
const first = lp.recordFailure(ip, now)
const second = lp.recordFailure(ip, now)
const third = lp.recordFailure(ip, now)
assert.equal(first, lp.BASE_MS) // 2^0
assert.equal(second, lp.BASE_MS * 2) // 2^1
assert.equal(third, lp.BASE_MS * 4) // 2^2
})
test('lockout is capped at MAX_MS', () => {
const ip = '198.51.100.51'
const now = 1_000_000
let last = 0
for (let i = 0; i < 40; i++) last = lp.recordFailure(ip, now)
assert.equal(last, lp.MAX_MS)
})
test('retryAfterMs reflects the active lockout and clears after it elapses', () => {
const ip = '198.51.100.52'
const now = 2_000_000
lp.recordFailure(ip, now) // locks BASE_MS
assert.ok(lp.retryAfterMs(ip, now) > 0)
assert.equal(lp.retryAfterMs(ip, now + lp.BASE_MS + 1), 0)
})
test('recordSuccess clears the failure streak', () => {
const ip = '198.51.100.53'
const now = 2_000_000
lp.recordFailure(ip, now)
lp.recordSuccess(ip)
assert.equal(lp.retryAfterMs(ip, now), 0)
})
test('streak resets after a long quiet period (does not grow forever)', () => {
const ip = '198.51.100.54'
const t0 = 3_000_000
lp.recordFailure(ip, t0)
lp.recordFailure(ip, t0)
// Come back after RESET_MS+ of quiet: next failure starts the streak over.
const later = t0 + lp.RESET_MS + 1
const delay = lp.recordFailure(ip, later)
assert.equal(delay, lp.BASE_MS) // back to 2^0
})
test('backoffGuard returns a generic 429 while locked out', async () => {
// Pre-lock this IP, then confirm the guard blocks it with a generic message.
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.post('/login', lp.backoffGuard, (req, res) => res.json({ ok: true }))
})
try {
lp.recordFailure('203.0.113.40') // lock the test client IP
const res = await fetch(`${app.url}/login`, {
method: 'POST',
headers: { 'X-Forwarded-For': '203.0.113.40' },
})
assert.equal(res.status, 429)
const body = await res.json()
assert.match(body.message, /too many login attempts/i)
// Message must not reveal whether username or password was the problem.
assert.doesNotMatch(body.message, /password|username/i)
assert.ok(res.headers.get('retry-after'))
} finally {
await app.close()
}
})
test('hard rate limiter caps attempts per IP (429 after the cap)', async () => {
const app = await startApp((a) => {
a.set('trust proxy', 1)
a.post('/login', loginLimiter, (req, res) => res.json({ ok: true }))
})
try {
const headers = { 'X-Forwarded-For': '203.0.113.41' }
let sawLimit = false
// Cap is 10/15min; the 11th should be blocked.
for (let i = 0; i < 12; i++) {
const res = await fetch(`${app.url}/login`, { method: 'POST', headers })
if (res.status === 429) sawLimit = true
}
assert.ok(sawLimit, 'expected the hard limiter to return 429 after the cap')
} finally {
await app.close()
}
})

65
server/test/totp.test.js Normal file
View File

@@ -0,0 +1,65 @@
// Set before requiring auth.js (reads JWT_SECRET at load) and db.js (builds the
// pool at load). Pointing the DB at a closed port stops the pool from eagerly
// opening idle connections that would keep this test process alive — none of
// these tests touch the database.
process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret'
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 speakeasy = require('speakeasy')
const totp = require('../src/utils/totp')
const { needsTotp } = require('../src/router/v1/auth/auth.controller')
const { signTotpChallenge, verifyTotpChallenge, getUserFromRequest } = require('../src/utils/auth')
const db = require('../src/utils/db')
after(() => db.close())
test('needsTotp: disabled user does not require a second factor', () => {
assert.equal(needsTotp({ id: 1, totp_enabled: 0 }), false)
assert.equal(needsTotp({ id: 1 }), false)
})
test('needsTotp: enabled user requires a second factor', () => {
assert.equal(needsTotp({ id: 1, totp_enabled: 1 }), true)
})
test('verifyCode accepts a current code and rejects a wrong/absent one', () => {
const { base32 } = totp.generateSecret('alice')
const good = speakeasy.totp({ secret: base32, encoding: 'base32' })
assert.equal(totp.verifyCode(base32, good), true)
assert.equal(totp.verifyCode(base32, '000000'), false)
assert.equal(totp.verifyCode(base32, ''), false)
assert.equal(totp.verifyCode(null, good), false)
})
test('generateSecret yields a base32 secret and an otpauth URL', () => {
const s = totp.generateSecret('bob')
assert.ok(s.base32 && s.base32.length >= 16)
assert.match(s.otpauthUrl, /^otpauth:\/\/totp\//)
})
test('qrDataUrl renders the otpauth URL to a PNG data URL', async () => {
const s = totp.generateSecret('carol')
const dataUrl = await totp.qrDataUrl(s.otpauthUrl)
assert.match(dataUrl, /^data:image\/png;base64,/)
})
// The password-verified challenge must never work as a real session token.
test('TOTP challenge token is not accepted as a session', () => {
const token = signTotpChallenge({ id: 42 })
// Valid as a challenge...
const challenge = verifyTotpChallenge(token)
assert.equal(challenge.id, 42)
// ...but rejected as a session (stage-tagged) when presented as a cookie/bearer.
const req = { cookies: {}, headers: { authorization: `Bearer ${token}` } }
assert.equal(getUserFromRequest(req), null)
})
test('a normal session token is not accepted as a TOTP challenge', () => {
const { signToken } = require('../src/utils/auth')
const session = signToken({ id: 7, username: 'x', role: 'admin' })
assert.equal(verifyTotpChallenge(session), null)
})

View File

@@ -0,0 +1,89 @@
const { test } = require('node:test')
const assert = require('node:assert/strict')
const { parseTrustProxy, applyTrustProxy } = require('../src/utils/trustProxy')
const { startApp } = require('./_helper')
test('parseTrustProxy: default (unset/empty) is a single hop', () => {
assert.equal(parseTrustProxy(''), 1)
assert.equal(parseTrustProxy(undefined), 1)
})
test('parseTrustProxy: integer hop count', () => {
assert.equal(parseTrustProxy('2'), 2)
assert.equal(parseTrustProxy('0'), 0)
})
test('parseTrustProxy: "false" disables proxy trust', () => {
assert.equal(parseTrustProxy('false'), false)
})
test('parseTrustProxy: blanket "true" is rejected and coerced to 1 (anti-spoof)', () => {
assert.equal(parseTrustProxy('true'), 1)
})
test('parseTrustProxy: CSV of IPs/CIDRs becomes an array; single stays a string', () => {
assert.deepEqual(parseTrustProxy('10.0.0.0/8, 172.18.0.1'), ['10.0.0.0/8', '172.18.0.1'])
assert.equal(parseTrustProxy('172.18.0.1'), '172.18.0.1')
})
test('applyTrustProxy: with 1 hop, req.ip reflects X-Forwarded-For client', async () => {
const app = await startApp((a) => {
applyTrustProxy(a, '1')
a.get('/ip', (req, res) => res.json({ ip: req.ip }))
})
try {
const res = await fetch(`${app.url}/ip`, { headers: { 'X-Forwarded-For': '203.0.113.7' } })
const body = await res.json()
assert.equal(body.ip, '203.0.113.7')
} finally {
await app.close()
}
})
test('applyTrustProxy: pinned to the peer IP, XFF from that peer is trusted', async () => {
// Mirrors the production setup: TRUST_PROXY = ptero's LAN IP. Here the test
// client's peer address is loopback, so pin to loopback and confirm XFF wins.
const app = await startApp((a) => {
applyTrustProxy(a, '127.0.0.1')
a.get('/ip', (req, res) => res.json({ ip: req.ip }))
})
try {
const res = await fetch(`${app.url}/ip`, { headers: { 'X-Forwarded-For': '203.0.113.9' } })
const body = await res.json()
assert.equal(body.ip, '203.0.113.9')
} finally {
await app.close()
}
})
test('applyTrustProxy: pinned to a DIFFERENT IP, XFF from this peer is NOT trusted', async () => {
// If ptero's IP is pinned but the connection comes from some other host, its
// X-Forwarded-For is ignored — nothing else on the LAN can spoof a client IP.
const app = await startApp((a) => {
applyTrustProxy(a, '10.11.12.13') // not the loopback peer this test connects from
a.get('/ip', (req, res) => res.json({ ip: req.ip }))
})
try {
const res = await fetch(`${app.url}/ip`, { headers: { 'X-Forwarded-For': '203.0.113.9' } })
const body = await res.json()
assert.notEqual(body.ip, '203.0.113.9')
} finally {
await app.close()
}
})
test('applyTrustProxy: with false, a forged X-Forwarded-For is ignored', async () => {
const app = await startApp((a) => {
applyTrustProxy(a, 'false')
a.get('/ip', (req, res) => res.json({ ip: req.ip }))
})
try {
const res = await fetch(`${app.url}/ip`, { headers: { 'X-Forwarded-For': '203.0.113.7' } })
const body = await res.json()
// The spoofed client IP must NOT be trusted — req.ip stays the loopback peer.
assert.notEqual(body.ip, '203.0.113.7')
} finally {
await app.close()
}
})