diff --git a/apps/api/src/app.js b/apps/api/src/app.js index 46a5985..c7fb20c 100644 --- a/apps/api/src/app.js +++ b/apps/api/src/app.js @@ -664,9 +664,14 @@ export async function buildServer(opts = {}) { await adminRoutes.register(rateLimit, { max: adminRateLimitMax, timeWindow: '1 minute', - keyGenerator: (req) => { - return authBearer(req) || req.ip; - } + // La cle DOIT etre l'IP, jamais le jeton fourni. Keyer sur le Bearer + // donnait a l'appelant le controle de son propre compteur : il suffisait + // d'envoyer un jeton different a chaque requete pour obtenir un seau + // neuf a chaque fois et contourner entierement le plafond -- or ce + // plafond existe precisement pour ralentir la recherche du jeton par + // force brute. En prime, chaque jeton inedit consommait une entree du + // cache LRU du limiteur, evincant les compteurs legitimes. + keyGenerator: (req) => req.ip, }); adminRoutes.post("/admin/import", async (req, reply) => { diff --git a/apps/api/test/rateLimit.test.js b/apps/api/test/rateLimit.test.js index d7b19f9..fb16e6c 100644 --- a/apps/api/test/rateLimit.test.js +++ b/apps/api/test/rateLimit.test.js @@ -64,21 +64,24 @@ describe("plafond de /admin/import", () => { await app.close(); }); - it("limite par jeton, pas globalement", async () => { + it("limite par IP, y compris pour un appelant authentifié", async () => { + // Ce test verrouillait auparavant l'inverse — un compteur par jeton, pour + // qu'un client bruyant n'affame pas les autres. L'intention se défend, mais + // le limiteur s'exécute AVANT l'authentification : la cle etait donc une + // valeur non verifiee, fournie par l'appelant. Isoler par identite reelle + // est impossible a cette couche ; on retombe sur l'IP, seule cle que + // l'appelant ne choisit pas. const app = await appWithRealLimits({ adminRateLimitMax: 2 }); const call = (token) => app.inject({ method: "POST", url: "/admin/import", - headers: { authorization: `Bearer ${token}` }, payload: { bands: [] }, + headers: { "x-forwarded-for": "4.4.4.4", authorization: `Bearer ${token}` }, + payload: { bands: [] }, }); await call(TOKEN); await call(TOKEN); - // Le 3e appel avec CE jeton est bloqué… expect((await call(TOKEN)).statusCode).toBe(429); - // …mais un autre jeton dispose de son propre compteur (il sera rejeté en - // 401, pas en 429 : c'est bien l'authentification qui tranche, pas le débit). - expect((await call("un-autre-jeton-de-la-meme-taille")).statusCode).toBe(401); await app.close(); }); }); @@ -107,6 +110,28 @@ describe("plafond de /admin/auth/login", () => { await app.close(); }); + it("le plafond d'import ne se contourne pas en changeant de jeton", async () => { + // Le limiteur etait keye sur le Bearer fourni, c'est-a-dire sur une valeur + // entierement controlee par l'appelant : envoyer un jeton different a + // chaque requete donnait un seau neuf a chaque fois, et le plafond ne + // freinait plus rien -- alors qu'il est justement la pour ralentir la + // recherche du jeton par force brute. + const app = await appWithRealLimits({ adminRateLimitMax: 2 }); + const call = (token) => + app.inject({ + method: "POST", + url: "/admin/import", + headers: { "x-forwarded-for": "9.9.9.9", authorization: `Bearer ${token}` }, + payload: { bands: [] }, + }); + + expect((await call("faux-a")).statusCode).toBe(401); + expect((await call("faux-b")).statusCode).toBe(401); + // 3e requete depuis la meme IP : bloquee, quel que soit le jeton presente. + expect((await call("faux-c")).statusCode).toBe(429); + await app.close(); + }); + it("limite par IP", async () => { const app = await appWithRealLimits({ authRateLimitMax: 1 }); const call = (ip) =>