From 82fcaa1e0fb2b32a33b070a27d7f2b6515aa0aae Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 07:22:33 +0000 Subject: [PATCH] Corrections issues de la revue de code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Limite de connexion par (e-mail, IP) et par IP : un tiers ne peut plus bloquer le compte admin depuis une autre adresse ; mémoire bornée. - Ids canoniques uniquement (« 07 » contournait la protection du propre compte de l'admin). - Export : un cadre réactivé entre deux requêtes ne fait plus planter. - /auth/me lit le compte déjà chargé par requireAuth ; /auth/password et e-mail non textuel ne renvoient plus 500. - Planning : une écriture réussie n'est plus perdue quand un rechargement du même mois répond avant elle. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D5Bdayziw6tybgqETZoNSt --- backend/src/index.js | 5 +++ backend/src/middleware/auth.js | 7 +++- backend/src/routes/auth.js | 74 ++++++++++++++++++++-------------- backend/src/routes/export.js | 4 +- backend/src/routes/users.js | 4 +- backend/src/utils/validate.js | 4 +- frontend/public/js/app.js | 11 +++-- 7 files changed, 68 insertions(+), 41 deletions(-) diff --git a/backend/src/index.js b/backend/src/index.js index a82df35..0263756 100644 --- a/backend/src/index.js +++ b/backend/src/index.js @@ -16,6 +16,11 @@ const app = express(); const PORT = process.env.PORT || 4790; app.disable('x-powered-by'); +// Requests arrive through nginx (and often a reverse proxy in front of it), +// both on private Docker networks: take the client address from +// X-Forwarded-For, trusting only private-network hops so it cannot be spoofed +// from the Internet. Used by the login rate limit. +app.set('trust proxy', 'loopback, linklocal, uniquelocal'); // The API is also reachable on its own port, without the nginx headers. app.use((req, res, next) => { res.set({ diff --git a/backend/src/middleware/auth.js b/backend/src/middleware/auth.js index 7373d96..56f639c 100644 --- a/backend/src/middleware/auth.js +++ b/backend/src/middleware/auth.js @@ -66,8 +66,10 @@ async function requireAuth(req, res, next) { return res.status(401).json({ error: 'Session invalide ou expirée' }); } const { rows } = await db.query( - `SELECT id, full_name, email, role, company_id, active, password_changed_at - FROM users WHERE id = $1`, + `SELECT u.id, u.full_name, u.email, u.role, u.company_id, u.active, u.password_changed_at, + c.name AS company_name + FROM users u LEFT JOIN companies c ON c.id = u.company_id + WHERE u.id = $1`, [payload.id] ); const u = rows[0]; @@ -80,6 +82,7 @@ async function requireAuth(req, res, next) { id: u.id, role: u.role, companyId: u.company_id, + companyName: u.company_name, fullName: u.full_name, email: u.email, }; diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 61cae37..d5bba1c 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -6,31 +6,47 @@ const { passwordError } = require('../utils/validate'); const router = express.Router(); -// Brute-force guard: after MAX_FAILURES wrong passwords for one email within -// WINDOW_MS, further attempts on that email are refused until the window ends. -const MAX_FAILURES = 10; +// Brute-force guard, two counters over WINDOW_MS: +// - per (email, IP): guessing one account's password. Keyed by IP too, so +// that someone failing on purpose cannot lock the real owner out from +// another address; +// - per IP, all emails together: trying a few passwords on many accounts. const WINDOW_MS = 15 * 60 * 1000; -const failures = new Map(); // email -> { count, since } +const MAX_PER_ACCOUNT = 10; +const MAX_PER_IP = 50; +const MAX_KEYS = 50000; // bounds memory whatever the attempt volume +const failures = new Map(); // key -> { count, since } -function lockedFor(email) { - const f = failures.get(email); - if (!f) return 0; - const left = f.since + WINDOW_MS - Date.now(); - if (left <= 0) { - failures.delete(email); - return 0; - } - return f.count >= MAX_FAILURES ? left : 0; +function failuresOf(key, now) { + const f = failures.get(key); + if (f && f.since + WINDOW_MS > now) return f; + if (f) failures.delete(key); + return null; } -function recordFailure(email) { +// Milliseconds until a new attempt is allowed, 0 if allowed now. +function lockedFor(email, ip) { const now = Date.now(); - if (failures.size > 10000) { - for (const [k, f] of failures) if (f.since + WINDOW_MS <= now) failures.delete(k); + let wait = 0; + for (const [key, max] of [[`a:${ip}:${email}`, MAX_PER_ACCOUNT], [`i:${ip}`, MAX_PER_IP]]) { + const f = failuresOf(key, now); + if (f && f.count >= max) wait = Math.max(wait, f.since + WINDOW_MS - now); + } + return wait; +} + +function recordFailure(email, ip) { + const now = Date.now(); + for (const key of [`a:${ip}:${email}`, `i:${ip}`]) { + const f = failuresOf(key, now); + if (f) { + f.count += 1; + continue; + } + // Map keeps insertion order: the first key is the oldest window. + if (failures.size >= MAX_KEYS) failures.delete(failures.keys().next().value); + failures.set(key, { count: 1, since: now }); } - const f = failures.get(email); - if (!f || f.since + WINDOW_MS <= now) failures.set(email, { count: 1, since: now }); - else f.count += 1; } // Compared against when the email is unknown, so that a wrong email and a @@ -54,7 +70,7 @@ router.post('/login', async (req, res) => { return res.status(400).json({ error: 'Email et mot de passe requis' }); } const key = email.trim().toLowerCase(); - const wait = lockedFor(key); + const wait = lockedFor(key, req.ip); if (wait) { const minutes = Math.ceil(wait / 60000); return res.status(429).json({ error: `Trop de tentatives. Réessayez dans ${minutes} min.` }); @@ -71,10 +87,10 @@ router.post('/login', async (req, res) => { const user = rows[0]; const ok = await bcrypt.compare(password, user ? user.password_hash : DUMMY_HASH); if (!user || !ok || !user.active) { - recordFailure(key); + recordFailure(key, req.ip); return res.status(401).json({ error: 'Identifiants incorrects' }); } - failures.delete(key); + failures.delete(`a:${req.ip}:${key}`); setAuthCookie(res, signToken(user)); res.json(publicUser(user)); }); @@ -84,15 +100,10 @@ router.post('/logout', (req, res) => { res.json({ ok: true }); }); -router.get('/me', requireAuth, async (req, res) => { - const { rows } = await db.query( - `SELECT u.id, u.full_name, u.email, u.role, u.company_id, c.name AS company_name - FROM users u - LEFT JOIN companies c ON c.id = u.company_id - WHERE u.id = $1`, - [req.user.id] - ); - res.json(publicUser(rows[0])); +// requireAuth has just read the account from the database. +router.get('/me', requireAuth, (req, res) => { + const { id, fullName, email, role, companyId, companyName } = req.user; + res.json({ id, fullName, email, role, companyId, companyName }); }); // Any signed-in user changes their own password; the current one is required. @@ -104,6 +115,7 @@ router.put('/password', requireAuth, async (req, res) => { return res.status(400).json({ error: 'Mot de passe actuel requis' }); } const { rows } = await db.query('SELECT password_hash FROM users WHERE id = $1', [req.user.id]); + if (!rows[0]) return res.status(401).json({ error: 'Session invalide ou expirée' }); if (!(await bcrypt.compare(current, rows[0].password_hash))) { return res.status(400).json({ error: 'Mot de passe actuel incorrect' }); } diff --git a/backend/src/routes/export.js b/backend/src/routes/export.js index 3fc6194..7b50631 100644 --- a/backend/src/routes/export.js +++ b/backend/src/routes/export.js @@ -31,7 +31,9 @@ async function loadCompanyData(companyId, year, month) { const cadres = cadreRows.map((c) => ({ id: c.id, fullName: c.full_name, entries: new Map() })); const byId = new Map(cadres.map((c) => [c.id, c])); for (const e of entryRows) { - byId.get(e.user_id).entries.set(`${e.entry_date}:${e.period}`, e.status); + // a cadre (re)activated between the two queries is simply left out + const cadre = byId.get(e.user_id); + if (cadre) cadre.entries.set(`${e.entry_date}:${e.period}`, e.status); } return { company, cadres }; diff --git a/backend/src/routes/users.js b/backend/src/routes/users.js index 919df9f..c55909f 100644 --- a/backend/src/routes/users.js +++ b/backend/src/routes/users.js @@ -12,8 +12,8 @@ const USER_COLUMNS = 'id, full_name, email, role, company_id, active, created_at // Shared checks for create and update. Returns an error message or null. function profileError({ full_name, email, role, company_id }) { - if (!isName(full_name) || !email || !role) return 'Champs requis manquants'; - if (!isEmail(String(email).trim())) return 'Adresse e-mail invalide'; + if (!isName(full_name) || typeof email !== 'string' || !email || !role) return 'Champs requis manquants'; + if (!isEmail(email.trim())) return 'Adresse e-mail invalide'; if (!['admin', 'cadre'].includes(role)) return 'Rôle invalide'; if (role === 'cadre' && !isId(String(company_id ?? ''))) return 'Une société doit être attribuée au cadre'; return null; diff --git a/backend/src/utils/validate.js b/backend/src/utils/validate.js index 044c93d..98da6db 100644 --- a/backend/src/utils/validate.js +++ b/backend/src/utils/validate.js @@ -6,7 +6,9 @@ const MAX_INT = 2147483647; function isId(v) { if (typeof v === 'number') return Number.isInteger(v) && v > 0 && v <= MAX_INT; - return typeof v === 'string' && /^\d{1,10}$/.test(v) && Number(v) > 0 && Number(v) <= MAX_INT; + // Canonical form only: '07' would reach SQL as 7 yet differ from '7' in + // the string comparisons that guard an admin's own account. + return typeof v === 'string' && /^[1-9]\d{0,9}$/.test(v) && Number(v) <= MAX_INT; } function isDate(s) { diff --git a/frontend/public/js/app.js b/frontend/public/js/app.js index 24026ee..bdcbe5d 100644 --- a/frontend/public/js/app.js +++ b/frontend/public/js/app.js @@ -51,7 +51,7 @@ try { data = await res.json(); } catch (e) { /* empty body */ } // The session ended under us (expired, password reset, account // disabled): go back to the login screen instead of failing every call. - if (res.status === 401 && state.user && !path.startsWith('/auth/')) { + if (res.status === 401 && state.user && path !== '/auth/login') { location.reload(); return new Promise(() => {}); } @@ -490,6 +490,7 @@ if (state.viewing) qs.set('user_id', state.viewing.id); const cal = await api(`/attendance?${qs}`); if (req !== planningReq) return; + cal.key = qs.toString(); state.cal = cal; renderPlanning(); } @@ -624,10 +625,12 @@ // After a successful write, apply it to the month already in memory // instead of reloading the month: painting stays fluid. `cal` is the month - // the write was made on; if the user moved to another month meanwhile, - // there is nothing to patch. + // the write was made on. It is matched by month and person, not by object: + // a reload that answered before this write landed must get it too, while a + // different month on screen must not. function patchEntries(cal, changes) { - if (state.cal !== cal) return; + if (!state.cal || state.cal.key !== cal.key) return; + cal = state.cal; const map = new Map((cal.entries || []).map((e) => [`${e.date}|${e.period}`, e])); changes.forEach(({ date, period, status }) => { if (status) map.set(`${date}|${period}`, { date, period, status });