Skip to content

OM-554: Rate Limiter - #194

Merged
leoAchu16 merged 5 commits into
devfrom
OM-554
Jun 5, 2026
Merged

leoAchu16 merged 5 commits into
devfrom
OM-554

Conversation

@SebaKisser

@SebaKisser SebaKisser commented Jun 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Limitadores de tasa para inicio de sesión (5 intentos / 15 min) y recuperación de contraseña (3 solicitudes / 1 hora).
  • Changes

    • Cambio del proveedor de envío de correos a Resend; se añadió la variable de entorno requerida para la API y nuevo remitente configurable.
  • Chores

    • Actualizadas dependencias relacionadas con rate limiting; se eliminó la dependencia de envío anterior.
  • Tests

    • Agregados tests que verifican el comportamiento de los limitadores de tasa.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b40b5999-a4f3-4d7c-9607-74e90e9c0276

📥 Commits

Reviewing files that changed from the base of the PR and between 7f33873 and 81b2ddd.

📒 Files selected for processing (1)
  • src/lib/email.service.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/email.service.js

📝 Walkthrough

Walkthrough

El PR migra el envío de correos de recuperación a Resend, exige RESEND_API_KEY, añade middlewares de rate limiting para login y forgot-password, los conecta a sus rutas y agrega tests unitarios que verifican límites, respuestas 429 y headers de rate limiting.

Changes

Endurecimiento de autenticación y recuperación

Layer / File(s) Summary
Base de correo y configuración
package.json, src/config/env.config.js, src/lib/email.service.js
Se actualizan dependencias (express-rate-limit añadido, nodemailer eliminado), validateEnv() requiere RESEND_API_KEY y sendPasswordResetEmail usa Resend con from configurable y manejo explícito de errores.
Rate limiting en endpoints sensibles
src/middlewares/rateLimiter.js, src/modules/session/routes/session.routes.js, src/modules/users/users/routes/users.routes.js
Se crean loginRateLimiter (5/15min) y passwordResetRateLimiter (3/1h) con respuestas JSON y bypass en tests; se aplican a POST / y POST /forgot-password.
Pruebas de límites y respuestas
tests/unit/session/rate-limit.test.js
Se agregan helpers para apps Express de prueba y casos que verifican límites permitidos, bloqueo con 429, mensajes JSON y header ratelimit para ambos middlewares.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • CrisNAC/BackendMarketplace#180: Relacionado con la validación de variables de entorno y la llamada a validateEnv() en el arranque, que hace efectiva la exigencia de RESEND_API_KEY.
  • CrisNAC/BackendMarketplace#185: Modifica validaciones en src/config/env.config.js, conexión directa con cambios en variables requeridas.
  • CrisNAC/BackendMarketplace#164: Implementa el flujo de reset password y la versión previa de sendPasswordResetEmail (con nodemailer), que ahora se reemplaza por Resend aquí.

Suggested reviewers

  • Benjakr04
  • CrisNAC

Poem

🐇 Brinco entre rutas y correo al compás,
cambié al cartero y puse un ceñidor,
controlo intentos para mayor paz,
y en pruebas la madriguera guarda calor.
¡Salud por el cambio y su pequeño error!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed El título es específico y se relaciona directamente con los cambios principales del PR que implementan rate limiting en dos endpoints críticos.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch OM-554

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/modules/session/routes/session.routes.js (1)

20-40: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Documentá la respuesta 429 del login.

Desde la Line 40 este endpoint puede devolver 429 por el loginRateLimiter, pero el Swagger de arriba no lo declara. Queda desalineado el contrato público justo en una ruta sensible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/session/routes/session.routes.js` around lines 20 - 40, El bloque
de Swagger para el endpoint definido por router.post("/", loginRateLimiter,
login) no documenta la respuesta 429 que puede devolver el loginRateLimiter;
actualizá el comentario OpenAPI/SWagger sobre este POST para agregar una 429
response (p. ej. description: "Demasiadas solicitudes / rate limit excedido") y
apuntá al esquema de error existente (por ejemplo ErrorResponse) para mantener
el contrato público alineado con el comportamiento real del middleware
loginRateLimiter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/email.service.js`:
- Around line 1-3: The module currently instantiates const resend = new
Resend(process.env.RESEND_API_KEY) at import time which will throw if
RESEND_API_KEY is missing; change to lazy initialization so importing the module
never reads the env. For example, remove the top-level new Resend(...) and
instead create a getResendClient() or initResendIfNeeded() used by
sendPasswordResetEmail that reads process.env.RESEND_API_KEY at call-time (or
after validateEnv() runs) and throws a clear error if the key is absent; update
any usages of the top-level resend variable to call that getter.

In `@tests/unit/session/rate-limit.test.js`:
- Around line 13-20: Los tests están creando un limiter de prueba con
makeTestLimiter en vez de usar los middlewares reales; reemplaza las instancias
de makeTestLimiter por los middlewares exportados loginRateLimiter y
passwordResetRateLimiter, elimina o deja de usar la función makeTestLimiter, e
importa los middlewares reales en la suite para que los tests ejerzan la
configuración shipped (headers, windowMs, limits, mensajes). Asegúrate además de
adaptar las aserciones para comprobar las propiedades relevantes (por ejemplo
windowMs, limit, standardHeaders/legacyHeaders y message) sobre los objetos
reales loginRateLimiter/passwordResetRateLimiter en lugar del stub.

---

Outside diff comments:
In `@src/modules/session/routes/session.routes.js`:
- Around line 20-40: El bloque de Swagger para el endpoint definido por
router.post("/", loginRateLimiter, login) no documenta la respuesta 429 que
puede devolver el loginRateLimiter; actualizá el comentario OpenAPI/SWagger
sobre este POST para agregar una 429 response (p. ej. description: "Demasiadas
solicitudes / rate limit excedido") y apuntá al esquema de error existente (por
ejemplo ErrorResponse) para mantener el contrato público alineado con el
comportamiento real del middleware loginRateLimiter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4b2c2ae6-780a-4280-a9f9-4a871d2941fa

📥 Commits

Reviewing files that changed from the base of the PR and between 4ef0145 and 7f33873.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • package.json
  • src/config/env.config.js
  • src/lib/email.service.js
  • src/middlewares/rateLimiter.js
  • src/modules/session/routes/session.routes.js
  • src/modules/users/users/routes/users.routes.js
  • tests/unit/session/rate-limit.test.js

Comment thread src/lib/email.service.js Outdated
Comment thread tests/unit/session/rate-limit.test.js
@leoAchu16
leoAchu16 merged commit b4e1381 into dev Jun 5, 2026
3 checks passed
@sonarqubecloud

sonarqubecloud Bot commented Jun 5, 2026

Copy link
Copy Markdown

This was referenced Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants