fix(mam-api): real dummy bcrypt hash + log last_login_at failures
Code-review feedback: - Dummy hash for user-enumeration-defense timing was 63 chars (bcrypt strings are 60 chars). Worked by accident because bcrypt 5.x is lenient about trailing chars; a future tightening would silently regress the timing defense. Replaced with a real pre-computed bcrypt hash. - last_login_at UPDATE now logs errors instead of silently swallowing them, matching the pattern in requireAuth for api_tokens.last_used_at. - Removed dead import of comparePassword from auth.test.js.
This commit is contained in:
parent
f8b6f7d5ef
commit
bcfc19e530
2 changed files with 8 additions and 4 deletions
|
|
@ -3,6 +3,8 @@ import pool from '../db/pool.js';
|
|||
import { DEV_USER_ID } from '../middleware/auth.js';
|
||||
import { hashPassword, comparePassword } from '../auth/passwords.js';
|
||||
|
||||
const DUMMY_PASSWORD_HASH = '$2b$12$gSeC58PregWedNFK/8Q61OephUo.JJ7EUs0LCTdnJV5AzCS5qQH7K';
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
// Real users = anyone except the seeded dev row.
|
||||
|
|
@ -70,8 +72,10 @@ router.post('/login', async (req, res, next) => {
|
|||
[username.trim(), DEV_USER_ID]
|
||||
);
|
||||
if (rows.length === 0) {
|
||||
// Still hash the supplied password against a dummy to keep response time uniform.
|
||||
await comparePassword(password, '$2b$12$dummyhashthatwillalwaysfailtocomparexxxxxxxxxxxxxxxxxxxx');
|
||||
// Pre-computed bcrypt hash of a value that no real password input will match.
|
||||
// Used to keep the user-not-found response time uniform with the wrong-password
|
||||
// path (~180ms at cost 12) so user enumeration via timing isn't possible.
|
||||
await comparePassword(password, DUMMY_PASSWORD_HASH);
|
||||
return res.status(401).json({ error: 'invalid credentials' });
|
||||
}
|
||||
const user = rows[0];
|
||||
|
|
@ -87,7 +91,8 @@ router.post('/login', async (req, res, next) => {
|
|||
// the prior bounce-to-login logic produced an infinite loop.
|
||||
await new Promise((resolve, reject) => req.session.save(err => err ? reject(err) : resolve()));
|
||||
|
||||
await pool.query(`UPDATE users SET last_login_at = NOW() WHERE id = $1`, [user.id]).catch(() => {});
|
||||
pool.query(`UPDATE users SET last_login_at = NOW() WHERE id = $1`, [user.id])
|
||||
.catch(err => console.error('[auth] last_login_at update failed:', err.message));
|
||||
|
||||
res.json({ user: { id: user.id, username: user.username, display_name: user.display_name } });
|
||||
} catch (err) { next(err); }
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ import express from 'express';
|
|||
import session from 'express-session';
|
||||
import authRouter from '../../src/routes/auth.js';
|
||||
import { hashPassword } from '../../src/auth/passwords.js';
|
||||
import { comparePassword } from '../../src/auth/passwords.js';
|
||||
import { requireAuth } from '../../src/middleware/auth.js';
|
||||
|
||||
async function appWithAuth(pool) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue