Skip to content

ALT-16: Implementar verificacion de email y reset de password#5

Merged
lapc506 merged 2 commits intomainfrom
ALT-16-auth-email-password
Mar 28, 2026
Merged

ALT-16: Implementar verificacion de email y reset de password#5
lapc506 merged 2 commits intomainfrom
ALT-16-auth-email-password

Conversation

@lapc506
Copy link
Copy Markdown
Collaborator

@lapc506 lapc506 commented Mar 28, 2026

Summary

  • Crea MailModule con NestJS Mailer + Handlebars templates en espanol
  • Implementa flujo de verificacion de email (token Redis 24h, endpoint REST GET /auth/verify-email)
  • Implementa flujo de password reset con anti-enumeracion (siempre retorna true)
  • Rate limiting en endpoints sensibles, 14 tests nuevos

Linear Issue

ALT-16

OpenSpec Change

openspec/changes/auth-email-password/ (4 fases completas)

Files Changed (12)

  • New: mail.module.ts, mail.service.ts, mail.service.spec.ts, templates (verification.hbs, reset.hbs)
  • New: auth.controller.ts, reset-password.input.ts
  • Modified: auth.service.ts, auth.resolver.ts, auth.module.ts, app.module.ts, .env.example, nest-cli.json

Test plan

  • Verificar npm run build compila sin errores
  • Verificar 46 tests pasan (14 nuevos)
  • Verificar que registro auto-envia email de verificacion
  • Verificar GET /auth/verify-email?token=xxx valida y redirige
  • Verificar requestPasswordReset retorna true para emails inexistentes
  • Verificar rate limiting en mutations sensibles

Created by Claude Code on behalf of @lapc506

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Nuevas Características

    • Flujo de verificación de correo por token con endpoint público y redirección.
    • Flujo de restablecimiento de contraseña por correo con enlaces seguros.
    • Envío de correos (verificación y restablecimiento) con plantillas en español y módulo de correo.
    • Límites de tasa aplicados a las operaciones de verificación y restablecimiento.
  • Tests

    • Cobertura añadida para envío de correos y flujos de verificación/restablecimiento.
  • Chores

    • Variables de entorno y configuración SMTP y de TTL añadidas; activos para plantillas en tiempo de ejecución.

@linear
Copy link
Copy Markdown

linear bot commented Mar 28, 2026

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 28, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 548d4d33-dd82-4a5e-a887-c290a1880955

📥 Commits

Reviewing files that changed from the base of the PR and between a8c14fa and eb3a5f4.

⛔ Files ignored due to path filters (1)
  • apps/backend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (16)
  • apps/backend/.env.example
  • apps/backend/nest-cli.json
  • apps/backend/package.json
  • apps/backend/src/app.module.ts
  • apps/backend/src/auth/auth.controller.ts
  • apps/backend/src/auth/auth.module.ts
  • apps/backend/src/auth/auth.resolver.ts
  • apps/backend/src/auth/auth.service.spec.ts
  • apps/backend/src/auth/auth.service.ts
  • apps/backend/src/auth/dto/reset-password.input.ts
  • apps/backend/src/auth/guards/gql-throttler.guard.ts
  • apps/backend/src/mail/mail.module.ts
  • apps/backend/src/mail/mail.service.spec.ts
  • apps/backend/src/mail/mail.service.ts
  • apps/backend/src/mail/templates/reset.hbs
  • apps/backend/src/mail/templates/verification.hbs

📝 Walkthrough

Walkthrough

Se añade un módulo de correo (SMTP + Handlebars), endpoints y resolvers para verificación de email y restablecimiento de contraseña, flujo de tokens almacenados en caché con TTLs configurables, protección mediante throttling/GQL guard, y ajustes de configuración/paquetes y tests asociados.

Changes

Cohort / File(s) Summary
Configuración y build
apps/backend/.env.example, apps/backend/nest-cli.json, apps/backend/package.json, apps/backend/src/app.module.ts
Se añaden variables de entorno para SMTP, tokens TTL, APP_URL, SENTRY_DSN, ENV_NAME; se habilitan assets de plantillas en build; se agregan dependencias @nestjs-modules/mailer y handlebars; MailModule se importa en AppModule.
Módulo y servicio de correo
apps/backend/src/mail/mail.module.ts, apps/backend/src/mail/mail.service.ts, apps/backend/src/mail/mail.service.spec.ts
Nuevo MailModule que configura MailerModule vía ConfigService (SMTP, from, templates Handlebars). MailService añade métodos sendVerificationEmail y sendPasswordResetEmail y dispone de pruebas unitarias que mockean MailerService.
Plantillas de correo
apps/backend/src/mail/templates/...
apps/backend/src/mail/templates/verification.hbs, apps/backend/src/mail/templates/reset.hbs
Se añaden plantillas Handlebars HTML (verificación y restablecimiento) con variables username, verificationLink/resetLink.
Autenticación — servicio
apps/backend/src/auth/auth.service.ts, apps/backend/src/auth/auth.service.spec.ts
AuthService inyecta MailService y ConfigService; se agregan métodos para enviar/verificar tokens (sendVerificationEmail, verifyEmailToken) y flujo de restablecimiento (requestPasswordReset, resetPasswordWithToken) usando caché y TTL configurables; tests extendidos y mocks actualizados.
Autenticación — API y guards
apps/backend/src/auth/auth.controller.ts, apps/backend/src/auth/auth.resolver.ts, apps/backend/src/auth/auth.module.ts, apps/backend/src/auth/guards/gql-throttler.guard.ts
Se añade AuthController con GET /auth/verify-email que redirige tras verificación; AuthResolver añade mutations para solicitar verificación, solicitar reset y resetear contraseña, protegidas por throttling/GQL guard y JWT según corresponda; AuthModule importa MailModule.
DTOs
apps/backend/src/auth/dto/reset-password.input.ts
Nuevo ResetPasswordInput GraphQL con validación (token, newPassword con minLength 8).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant AuthResolver as Auth Resolver
    participant AuthService as Auth Service
    participant Cache as Cache
    participant MailService as Mail Service
    participant SMTP as SMTP Server

    rect rgba(100,150,255,0.5)
    Note over User,SMTP: Flujo de verificación de email
    User->>AuthResolver: requestEmailVerification()
    AuthResolver->>AuthService: sendVerificationEmail(userId)
    AuthService->>Cache: set verify:{token} → userId (TTL)
    AuthService->>MailService: sendVerificationEmail(email, username, token, appUrl)
    MailService->>SMTP: enviar email de verificación
    SMTP-->>User: Email con enlace de verificación
    User->>User: Hace clic en enlace
    end

    rect rgba(150,200,255,0.5)
    Note over User,Cache: Flujo de restablecimiento de contraseña
    User->>AuthResolver: requestPasswordReset(email)
    AuthResolver->>AuthService: requestPasswordReset(email)
    AuthService->>Cache: set reset:{token} → userId (TTL)
    AuthService->>MailService: sendPasswordResetEmail(email, username, token, appUrl)
    MailService->>SMTP: enviar email de restablecimiento
    SMTP-->>User: Email con enlace de reset
    User->>AuthResolver: resetPassword(token, newPassword)
    AuthResolver->>AuthService: resetPasswordWithToken(token, newPassword)
    AuthService->>Cache: get reset:{token}
    AuthService->>AuthService: validar y hashear nueva contraseña
    AuthService->>Cache: del reset:{token}
    end

    rect rgba(200,150,255,0.5)
    Note over User,Cache: Verificación vía endpoint REST
    User->>AuthController: GET /auth/verify-email?token=X
    AuthController->>AuthService: verifyEmailToken(token)
    AuthService->>Cache: get verify:{token}
    AuthService->>Cache: del verify:{token}
    AuthController-->>User: redirect a /verified (o ?error=invalid_token)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed El título describe con precisión la función principal del cambio: implementar verificación de email y reset de contraseña, lo que se alinea perfectamente con los objetivos y archivos modificados en el PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 ALT-16-auth-email-password

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

@lapc506
Copy link
Copy Markdown
Collaborator Author

lapc506 commented Mar 28, 2026

@greptile review

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (1)
apps/backend/src/auth/auth.service.ts (1)

104-111: ⚠️ Potential issue | 🔴 Critical

🚨 Logging de contraseñas y hashes en logs de debug es un riesgo de seguridad crítico.

Las líneas 105-110 registran la contraseña en texto plano y los hashes parciales. Aunque sea a nivel DEBUG, estos logs pueden terminar en sistemas de agregación de logs (CloudWatch, Datadog, etc.) y violar normativas de cumplimiento.

🔒 Remover logging sensible
     this.logger.debug(`[validateUser] PASSWORD_SALT (first 16 chars): ${PASSWORD_SALT.substring(0, 16)}...`);
-    this.logger.debug(`[validateUser] Password: ${pass}`);
-    this.logger.debug(`[validateUser] Username: ${username.toLowerCase()}`);
-    this.logger.debug(`[validateUser] First hash (SHA256 of password): ${firstHash}`);
-    this.logger.debug(`[validateUser] Combined string (first 50 chars): ${combined.substring(0, 50)}...`);
-    this.logger.debug(`[validateUser] Input hash (first 16 chars): ${inputHash.substring(0, 16)}...`);
-    this.logger.debug(`[validateUser] Stored hash (first 16 chars): ${user.passwordHash?.substring(0, 16) || 'NULL'}...`);
+    this.logger.debug(`[validateUser] Validando hash para usuario: ${username.toLowerCase()}`);
     this.logger.debug(`[validateUser] ------------------------------------------------------------`);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/auth/auth.service.ts` around lines 104 - 111, Remove all
logging of sensitive data in the validateUser flow: eliminate calls that log the
plain password, full/partial password hashes, and any raw salt/hash substrings
(references: validateUser method and this.logger.debug lines that print
PASSWORD_SALT, pass, firstHash, combined, inputHash, user.passwordHash). Replace
them with non-sensitive diagnostics only (e.g., log that validation
started/ended, whether a user record was found, and boolean results or
lengths/counts if helpful), or redact values before logging (e.g., log
"<redacted>" or only the existence/length of a value) to avoid leaking secrets
to log aggregation systems. Ensure no code paths still emit the raw password,
salt, or hash values.
🧹 Nitpick comments (9)
apps/backend/src/mail/templates/verification.hbs (1)

37-37: Evita hardcodear la expiración mostrada en el correo.

En Line 37 el texto de expiración está fijo en 24 horas; si cambia EMAIL_VERIFICATION_TTL, el correo puede informar un valor incorrecto.

Propuesta de ajuste
-          Este enlace expira en <strong>24 horas</strong>. Si no solicitaste esta verificacion, puedes ignorar este correo.
+          Este enlace expira en <strong>{{verificationExpiryText}}</strong>. Si no solicitaste esta verificacion, puedes ignorar este correo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/mail/templates/verification.hbs` at line 37, La plantilla
verification.hbs tiene la expiración hardcodeada ("24 horas"); cambia la
plantilla para usar una variable (por ejemplo {{verificationTTL}}) en vez del
texto fijo, y modifica el código que arma/manda el correo (p. ej. la función que
construye el contexto de envío, como sendVerificationEmail / el renderizador de
templates) para calcular y pasar verificationTTL derivado de
EMAIL_VERIFICATION_TTL (convertir el valor a horas o al formato legible deseado)
al contexto antes de renderizar.
apps/backend/src/mail/templates/reset.hbs (1)

37-37: Sincroniza el texto de expiración con el TTL real del token.

En Line 37 se muestra 1 hora fijo; si PASSWORD_RESET_TTL cambia, el correo puede comunicar un tiempo incorrecto.

Propuesta de ajuste
-          Este enlace expira en <strong>1 hora</strong>. Si no solicitaste este cambio, puedes ignorar este correo.
+          Este enlace expira en <strong>{{resetExpiryText}}</strong>. Si no solicitaste este cambio, puedes ignorar este correo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/mail/templates/reset.hbs` at line 37, El texto de expiración
está hardcodeado como "1 hora"; sincroniza esto con el valor real de
PASSWORD_RESET_TTL pasando un valor calculado al template (p.ej.
tokenExpiryText) en lugar de texto fijo en reset.hbs: cambia "1 hora" por la
variable del template (ej. {{tokenExpiryText}}) y en la función que genera el
correo (la que usa PASSWORD_RESET_TTL) convierte el TTL en un texto legible
(horas/minutos) y pásalo como tokenExpiryText para que el correo siempre muestre
el TTL real.
apps/backend/src/auth/auth.controller.ts (2)

15-19: Falta validación del parámetro token.

Si token es undefined o vacío, el servicio fallará con un mensaje genérico. Considera agregar validación explícita o usar un ValidationPipe.

♻️ Agregar validación del token
+import { Controller, Get, Query, Res, Logger, BadRequestException } from '@nestjs/common';
-import { Controller, Get, Query, Res, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import type { Response } from 'express';
 import { AuthService } from './auth.service';

 `@Controller`('auth')
 export class AuthController {
   private readonly logger = new Logger(AuthController.name);

   constructor(
     private readonly authService: AuthService,
     private readonly configService: ConfigService,
   ) {}

   `@Get`('verify-email')
   async verifyEmail(
     `@Query`('token') token: string,
     `@Res`() res: Response,
   ) {
+    if (!token) {
+      const appUrl = this.configService.get<string>('APP_URL', 'http://localhost:3000');
+      return res.redirect(`${appUrl}/verified?error=missing_token`);
+    }
+
     const appUrl = this.configService.get<string>(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/auth/auth.controller.ts` around lines 15 - 19, El método
verifyEmail en AuthController no valida el parámetro token; agrega validación
para asegurar que token no sea undefined ni cadena vacía (por ejemplo lanzar
BadRequestException si falta/está vacío) o aplicar un ValidationPipe/DTO para
`@Query`('token') token antes de proceder con la verificación, actualizando la
firma y la lógica dentro de verifyEmail para devolver un error claro cuando el
token no sea válido.

28-33: Acceso a error.message sin validación de tipo.

En TypeScript estricto, error en un bloque catch es de tipo unknown. Acceder a error.message directamente puede causar errores de compilación o runtime.

♻️ Manejar el tipo del error correctamente
     } catch (error) {
+      const message = error instanceof Error ? error.message : 'Error desconocido';
       this.logger.error(
-        `[verifyEmail] Error al verificar email: ${error.message}`,
+        `[verifyEmail] Error al verificar email: ${message}`,
       );
       res.redirect(`${appUrl}/verified?error=invalid_token`);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/auth/auth.controller.ts` around lines 28 - 33, The catch
block in the verifyEmail handler accesses error.message even though catch errors
are unknown in strict TypeScript; change the catch to safely extract the message
by checking if (error instanceof Error) then use error.message, otherwise fall
back to String(error) or a default like 'Unknown error', and pass the safe
message (and optionally the original error object) into this.logger.error and
into the redirect decision so you no longer access error.message directly;
update the catch in auth.controller.ts (the [verifyEmail] block) to use that
guarded extraction.
apps/backend/src/auth/auth.service.spec.ts (1)

264-272: Uso de setTimeout para esperar promesas fire-and-forget es frágil.

El uso de setTimeout(r, 10) para esperar la cadena de promesas puede fallar intermitentemente en entornos con alta carga. Considera usar await flushPromises() o reestructurar para evitar dependencias de timing.

♻️ Usar flushPromises en lugar de setTimeout

Agregar helper al inicio del archivo:

const flushPromises = () => new Promise(setImmediate);

Y usarlo en el test:

       expect(mockCacheManager.set).toHaveBeenCalledWith(
         expect.stringMatching(/^verify:/),
         mockUser.id,
         86400000, // 86400 * 1000
       );
-      // Mail is fire-and-forget so we wait a tick for the promise chain
-      await new Promise((r) => setTimeout(r, 10));
+      // Esperar que las promesas pendientes se resuelvan
+      await flushPromises();
       expect(mockMailService.sendVerificationEmail).toHaveBeenCalledWith(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/auth/auth.service.spec.ts` around lines 264 - 272, Reemplaza
la espera frágil basada en setTimeout en la prueba que verifica
mockMailService.sendVerificationEmail por una espera determinista: añade un
helper const flushPromises = () => new Promise(setImmediate) al inicio del
archivo de pruebas y sustituye await new Promise((r) => setTimeout(r, 10)); por
await flushPromises(); de modo que la llamada fire-and-forget a
mockMailService.sendVerificationEmail sea aguardada de forma fiable antes de
ejecutar expect(mockMailService.sendVerificationEmail) en auth.service.spec.ts.
apps/backend/src/mail/mail.service.ts (2)

38-64: Aplicar la misma normalización de URL en sendPasswordResetEmail.

El mismo problema de doble barra existe aquí. Considera extraer la normalización a un método auxiliar para evitar duplicación.

♻️ Extraer normalización a método privado
+  private normalizeUrl(url: string): string {
+    return url.replace(/\/+$/, '');
+  }
+
   async sendPasswordResetEmail(
     email: string,
     username: string,
     token: string,
     appUrl: string,
   ): Promise<void> {
-    const resetLink = `${appUrl}/auth/reset-password?token=${token}`;
+    const normalizedUrl = this.normalizeUrl(appUrl);
+    const resetLink = `${normalizedUrl}/auth/reset-password?token=${token}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/mail/mail.service.ts` around lines 38 - 64, The
sendPasswordResetEmail function currently concatenates appUrl and the reset path
and can produce a double slash; extract a private helper (e.g., normalizeAppUrl
or buildResetLink) and use it from sendPasswordResetEmail to ensure the base URL
has no trailing slash (or the path always has a single leading slash) before
composing resetLink; update sendPasswordResetEmail to call that helper so URL
normalization is shared and duplicated logic is removed.

10-36: Posible doble barra en la URL si appUrl termina con /.

Si APP_URL se configura con barra final (ej: https://example.com/), el link resultante tendrá doble barra: https://example.com//auth/verify-email?token=.... Esto puede causar problemas en algunos servidores o proxies.

♻️ Normalizar appUrl removiendo barra final
   async sendVerificationEmail(
     email: string,
     username: string,
     token: string,
     appUrl: string,
   ): Promise<void> {
-    const verificationLink = `${appUrl}/auth/verify-email?token=${token}`;
+    const normalizedUrl = appUrl.replace(/\/+$/, '');
+    const verificationLink = `${normalizedUrl}/auth/verify-email?token=${token}`;

     try {
       await this.mailerService.sendMail({
         to: email,
         subject: 'AltruPets - Verifica tu correo electronico',
         template: 'verification',
         context: {
           username,
           verificationLink,
-          appUrl,
+          appUrl: normalizedUrl,
         },
       });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/mail/mail.service.ts` around lines 10 - 36, The
verificationLink construction in sendVerificationEmail can produce a double
slash when appUrl ends with '/' (verificationLink =
`${appUrl}/auth/verify-email?token=${token}`); normalize appUrl by stripping any
trailing slashes before building the link (e.g., compute a normalizedBase =
appUrl.replace(/\/+$/, '') or equivalent) and then build verificationLink from
normalizedBase + '/auth/verify-email?token=' + token to ensure no double
slashes; update the sendVerificationEmail function to use that normalized value.
apps/backend/src/auth/auth.service.ts (1)

306-309: Errores de envío de email en registro se descartan silenciosamente.

El .catch(() => {}) vacío hace que los fallos de envío de email sean invisibles. Considera al menos loguear el error para facilitar el debugging.

♻️ Agregar logging de errores
     // Fire-and-forget: send verification email if email is provided
     if (registerData.email) {
-      this.sendVerificationEmail(newUser.id).catch(() => {});
+      this.sendVerificationEmail(newUser.id).catch((err) => {
+        this.logger.error(
+          `[register] Error al enviar email de verificación: ${err.message}`,
+        );
+      });
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/auth/auth.service.ts` around lines 306 - 309, The current
fire-and-forget email call swallows failures via .catch(() => {}); replace the
empty catch with proper error logging so failures are visible: in the branch
where registerData.email is truthy call
this.sendVerificationEmail(newUser.id).catch(err => { /* log */ }); and log the
error with context (newUser.id and registerData.email) using the service's
logger (e.g., this.logger.error) or console.error if no logger exists; keep it
non-blocking (do not await) but ensure the catch records the exception and
contextual info.
apps/backend/src/mail/mail.module.ts (1)

8-42: Instanciación duplicada de MailModule causa conexiones SMTP redundantes.

MailModule se importa tanto en app.module.ts (línea 139) como en auth.module.ts (línea 18). Como no está marcado como @Global(), NestJS creará dos instancias independientes del módulo, cada una con su propio transporte SMTP.

Opciones para corregir:

  1. Marcar el módulo como @Global() para que una sola instancia sea compartida.
  2. Remover la importación de auth.module.ts ya que AppModule lo importa.
♻️ Opción recomendada: Marcar como `@Global`()
 import { Module } from '@nestjs/common';
+import { Global } from '@nestjs/common';
 import { MailerModule } from '@nestjs-modules/mailer';
 import { HandlebarsAdapter } from '@nestjs-modules/mailer/adapters/handlebars.adapter';
 import { ConfigModule, ConfigService } from '@nestjs/config';
 import { join } from 'path';
 import { MailService } from './mail.service';

+@Global()
 `@Module`({
   imports: [

Y remover la importación duplicada en auth.module.ts:

 imports: [
     PassportModule,
     ConfigModule,
     UsersModule,
-    MailModule,
     JwtModule.registerAsync({
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/mail/mail.module.ts` around lines 8 - 42, The MailModule is
being instantiated twice causing duplicate SMTP connections; mark the MailModule
class with `@Global`() and re-export MailService so the Nest container shares a
single instance, then remove the duplicate import of MailModule from the
AuthModule (leave it only imported by the AppModule) to ensure only one
MailModule instance is created; update MailModule to include `@Global`() above the
class and keep providers: [MailService], exports: [MailService].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/backend/.env.example`:
- Line 32: El valor de la variable MAIL_FROM contiene espacios y símbolos que
pueden romper el parseo; actualiza la definición de MAIL_FROM para evitar
ambigüedad envolviendo todo el valor entre comillas o separando nombre y
dirección en dos variables. Por ejemplo, reemplaza la entrada actual de
MAIL_FROM por una cadena completa que incluya el display name y el email entre
comillas (referencia: MAIL_FROM) o crea MAIL_FROM_NAME y MAIL_FROM_ADDRESS y usa
ambas donde se necesite; asegúrate de actualizar la documentación/example env
para reflejar la elección.

In `@apps/backend/src/auth/auth.resolver.ts`:
- Around line 114-119: La mutación resetPassword carece de rate limiting; añade
el decorador `@Throttle` con los mismos límites que se usan en
requestPasswordReset / requestEmailVerification encima del método resetPassword
(p. ej. `@Throttle`(...) con los mismos valores), e importa el decorador Throttle
desde el mismo módulo que usan los otros resolvers si aún no está importado;
esto aplica al método resetPassword y mantiene consistencia y defensa en
profundidad contra fuerza bruta.
- Around line 99-112: The `@Throttle` decorators on requestEmailVerification and
requestPasswordReset won't work because the existing ThrottlerBehindProxyGuard
uses context.switchToHttp().getRequest() and doesn't handle GraphQL context;
implement a GraphQL-aware guard (e.g., GqlThrottlerGuard) that extends the
ThrottlerGuard and overrides the request extraction to use GqlExecutionContext
(or equivalent) to return the GraphQL request object, then register that guard
as an APP_GUARD or apply it explicitly to the resolver (e.g., add
`@UseGuards`(GqlThrottlerGuard, JwtAuthGuard) to
requestEmailVerification/requestPasswordReset) so `@Throttle` is honored in
GraphQL mutations; reference ThrottlerBehindProxyGuard, GqlThrottlerGuard,
requestEmailVerification, requestPasswordReset, and APP_GUARD when making the
change.

---

Outside diff comments:
In `@apps/backend/src/auth/auth.service.ts`:
- Around line 104-111: Remove all logging of sensitive data in the validateUser
flow: eliminate calls that log the plain password, full/partial password hashes,
and any raw salt/hash substrings (references: validateUser method and
this.logger.debug lines that print PASSWORD_SALT, pass, firstHash, combined,
inputHash, user.passwordHash). Replace them with non-sensitive diagnostics only
(e.g., log that validation started/ended, whether a user record was found, and
boolean results or lengths/counts if helpful), or redact values before logging
(e.g., log "<redacted>" or only the existence/length of a value) to avoid
leaking secrets to log aggregation systems. Ensure no code paths still emit the
raw password, salt, or hash values.

---

Nitpick comments:
In `@apps/backend/src/auth/auth.controller.ts`:
- Around line 15-19: El método verifyEmail en AuthController no valida el
parámetro token; agrega validación para asegurar que token no sea undefined ni
cadena vacía (por ejemplo lanzar BadRequestException si falta/está vacío) o
aplicar un ValidationPipe/DTO para `@Query`('token') token antes de proceder con
la verificación, actualizando la firma y la lógica dentro de verifyEmail para
devolver un error claro cuando el token no sea válido.
- Around line 28-33: The catch block in the verifyEmail handler accesses
error.message even though catch errors are unknown in strict TypeScript; change
the catch to safely extract the message by checking if (error instanceof Error)
then use error.message, otherwise fall back to String(error) or a default like
'Unknown error', and pass the safe message (and optionally the original error
object) into this.logger.error and into the redirect decision so you no longer
access error.message directly; update the catch in auth.controller.ts (the
[verifyEmail] block) to use that guarded extraction.

In `@apps/backend/src/auth/auth.service.spec.ts`:
- Around line 264-272: Reemplaza la espera frágil basada en setTimeout en la
prueba que verifica mockMailService.sendVerificationEmail por una espera
determinista: añade un helper const flushPromises = () => new
Promise(setImmediate) al inicio del archivo de pruebas y sustituye await new
Promise((r) => setTimeout(r, 10)); por await flushPromises(); de modo que la
llamada fire-and-forget a mockMailService.sendVerificationEmail sea aguardada de
forma fiable antes de ejecutar expect(mockMailService.sendVerificationEmail) en
auth.service.spec.ts.

In `@apps/backend/src/auth/auth.service.ts`:
- Around line 306-309: The current fire-and-forget email call swallows failures
via .catch(() => {}); replace the empty catch with proper error logging so
failures are visible: in the branch where registerData.email is truthy call
this.sendVerificationEmail(newUser.id).catch(err => { /* log */ }); and log the
error with context (newUser.id and registerData.email) using the service's
logger (e.g., this.logger.error) or console.error if no logger exists; keep it
non-blocking (do not await) but ensure the catch records the exception and
contextual info.

In `@apps/backend/src/mail/mail.module.ts`:
- Around line 8-42: The MailModule is being instantiated twice causing duplicate
SMTP connections; mark the MailModule class with `@Global`() and re-export
MailService so the Nest container shares a single instance, then remove the
duplicate import of MailModule from the AuthModule (leave it only imported by
the AppModule) to ensure only one MailModule instance is created; update
MailModule to include `@Global`() above the class and keep providers:
[MailService], exports: [MailService].

In `@apps/backend/src/mail/mail.service.ts`:
- Around line 38-64: The sendPasswordResetEmail function currently concatenates
appUrl and the reset path and can produce a double slash; extract a private
helper (e.g., normalizeAppUrl or buildResetLink) and use it from
sendPasswordResetEmail to ensure the base URL has no trailing slash (or the path
always has a single leading slash) before composing resetLink; update
sendPasswordResetEmail to call that helper so URL normalization is shared and
duplicated logic is removed.
- Around line 10-36: The verificationLink construction in sendVerificationEmail
can produce a double slash when appUrl ends with '/' (verificationLink =
`${appUrl}/auth/verify-email?token=${token}`); normalize appUrl by stripping any
trailing slashes before building the link (e.g., compute a normalizedBase =
appUrl.replace(/\/+$/, '') or equivalent) and then build verificationLink from
normalizedBase + '/auth/verify-email?token=' + token to ensure no double
slashes; update the sendVerificationEmail function to use that normalized value.

In `@apps/backend/src/mail/templates/reset.hbs`:
- Line 37: El texto de expiración está hardcodeado como "1 hora"; sincroniza
esto con el valor real de PASSWORD_RESET_TTL pasando un valor calculado al
template (p.ej. tokenExpiryText) en lugar de texto fijo en reset.hbs: cambia "1
hora" por la variable del template (ej. {{tokenExpiryText}}) y en la función que
genera el correo (la que usa PASSWORD_RESET_TTL) convierte el TTL en un texto
legible (horas/minutos) y pásalo como tokenExpiryText para que el correo siempre
muestre el TTL real.

In `@apps/backend/src/mail/templates/verification.hbs`:
- Line 37: La plantilla verification.hbs tiene la expiración hardcodeada ("24
horas"); cambia la plantilla para usar una variable (por ejemplo
{{verificationTTL}}) en vez del texto fijo, y modifica el código que arma/manda
el correo (p. ej. la función que construye el contexto de envío, como
sendVerificationEmail / el renderizador de templates) para calcular y pasar
verificationTTL derivado de EMAIL_VERIFICATION_TTL (convertir el valor a horas o
al formato legible deseado) al contexto antes de renderizar.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5d3d06e9-de40-4296-9775-24644f94b705

📥 Commits

Reviewing files that changed from the base of the PR and between 53c48af and a8c14fa.

⛔ Files ignored due to path filters (1)
  • apps/backend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (15)
  • apps/backend/.env.example
  • apps/backend/nest-cli.json
  • apps/backend/package.json
  • apps/backend/src/app.module.ts
  • apps/backend/src/auth/auth.controller.ts
  • apps/backend/src/auth/auth.module.ts
  • apps/backend/src/auth/auth.resolver.ts
  • apps/backend/src/auth/auth.service.spec.ts
  • apps/backend/src/auth/auth.service.ts
  • apps/backend/src/auth/dto/reset-password.input.ts
  • apps/backend/src/mail/mail.module.ts
  • apps/backend/src/mail/mail.service.spec.ts
  • apps/backend/src/mail/mail.service.ts
  • apps/backend/src/mail/templates/reset.hbs
  • apps/backend/src/mail/templates/verification.hbs

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

lapc506 and others added 2 commits March 28, 2026 11:57
Add MailModule with Handlebars templates for verification and reset emails
(Spanish). AuthService gains sendVerificationEmail, verifyEmailToken,
requestPasswordReset, and resetPasswordWithToken methods. New REST
controller at GET /auth/verify-email for token-based redirect. GraphQL
mutations requestEmailVerification, requestPasswordReset, and resetPassword
are rate-limited via @Throttle. Unit tests cover all new paths (46 total).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… resetPassword, fix MAIL_FROM

- Create GqlThrottlerGuard that extracts req/res from GraphQL context
  so @Throttle() actually works on GraphQL resolvers
- Apply GqlThrottlerGuard to requestEmailVerification,
  requestPasswordReset, and resetPassword mutations
- Add @Throttle({ short: { ttl: 1000, limit: 3 } }) to resetPassword
- Fix MAIL_FROM in .env.example to use proper quoted email format

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lapc506 lapc506 force-pushed the ALT-16-auth-email-password branch from e285173 to eb3a5f4 Compare March 28, 2026 17:58
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@lapc506 lapc506 merged commit 8f618d2 into main Mar 28, 2026
3 of 10 checks passed
@lapc506 lapc506 deleted the ALT-16-auth-email-password branch March 28, 2026 17:58
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.

1 participant