Skip to content

feat: RF13, RF23, RF27 + fix de seguridad OAuth2 (state) + revert RF17

belen.varsi requested to merge fix/RF13-23-27-+-fix-de-state-OAuth2 into master

Qué hace este MR

Cierra los tres gaps de RF que quedaban pendientes de la matriz de trazabilidad (RF13, RF23, RF27), corrige un hallazgo de seguridad de la corrección del profesor sobre el flujo OAuth2, y revierte el endpoint de clonación de instancias que el equipo descartó en el chat del 2/7.

Por dónde entrar a revisar

Si tenés poco tiempo, revisá en este orden:

  1. OAuthStateStoreTest + OAuthStateStore — es el fix de seguridad, el cambio más importante del MR.
  2. InstanciaComunService.java — tiene los tres RFs nuevos (crear() modificado para RF13, crearDesdeRecordatorio() para RF27) y es donde más lógica nueva hay.
  3. El resto es bastante mecánico (DTOs, tests, el cron de RF23).

RF13 — Recordatorio inicial al crear una instancia

POST /api/instancias ahora acepta un campo opcional recordatorioInicial. Si viene, se crea el recordatorio en la misma transacción (delegando en RecordatorioService.crear(), que ya sincroniza con Google Calendar desde RF22 — no hubo que tocar nada de esa parte).

Decisión a validar: si el recordatorio anidado falla validación (ej. categoría inexistente), se revierte también la instancia. No lo hicimos "best effort" como el email/GCal porque me pareció raro crear una instancia "a medias" sin el recordatorio que se pidió explícitamente. Avisen si no están de acuerdo.

RF23 — Notificaciones de recordatorios

Dos partes:

  • Email al crear el recordatorio (mismo patrón que RF15).
  • Cron propio (RecordatorioNotificacionScheduler, corre cada hora) que avisa recordatorios que vencen en las próximas 24hs. WhatsApp queda fuera de alcance (documentado en README), como ya habíamos decidido con CSV/Excel en RF31.

Requiere columna nueva (notificado_proximidad) — ver nota de Docker abajo.

RF27 — Crear instancia desde recordatorio cumplido

POST /api/recordatorios/{id}/crear-instancia. Marca el recordatorio como cumplido (columna nueva) y no deja generar una segunda instancia desde el mismo recordatorio (409 si ya está cumplido).

Dato de arquitectura importante: esto vive en InstanciaComunService y no en RecordatorioService, para no crear una dependencia circular (InstanciaComunService ya depende de RecordatorioService desde RF13). Usa self-invocation (this.crear(...)) a propósito — está comentado en el código, pero avisen si genera dudas en la revisión.

Fix de seguridad — state en OAuth2 (Google Calendar)

El profesor marcó en la corrección que el state del flujo OAuth2 no protegía nada. Tenía razón: era el idUsuario en texto plano, adivinable. Cualquiera podía completar /api/auth/google/callback en nombre de otro funcionario y asociarle su propio refresh_token — filtrando los recordatorios de la víctima al Google Calendar del atacante.

Ahora state es un token aleatorio de un solo uso (OAuthStateStore, en memoria, TTL 10 min). Limitación documentada: no sirve si escalamos a múltiples instancias del backend (no aplica a nuestro despliegue actual).

Revert — Endpoint de clonación (RF17)

Habíamos agregado POST /api/instancias/{id}/clonar, pero en el chat del 2/7 quedamos en que no tiene sentido clonar sin dejar editar antes (¿para qué dos instancias idénticas?). RF17 queda resuelto con GET /api/instancias/{id} + POST /api/instancias ya existentes — el Frontend se encarga de precargar el form. README actualizado para que no vuelva a pasar lo que señaló el profesor (documentar algo que el código no hace).

⚠️ Antes de mergear / probar localmente

Hay dos columnas nuevas en recordatorios (notificado_proximidad, cumplido). Hace falta: docker compose down -v docker compose up -d --remove-orphans

Tests

8 clases de test nuevas + 2 ampliadas. Cobertura completa en RecurrenceRuleBuilderTest (se extrajo esa lógica de GoogleCalendarService a una clase propia para poder testearla sin mockear el SDK de Google). GoogleOAuthServiceTest y GoogleCalendarServiceTest tienen alcance limitado a lo que no pega contra la red real de Google — está documentado en un comentario al inicio de cada clase, por si preguntan por qué no cubren el happy path completo.

mvn clean test corre todo sin necesidad de Docker (usa H2).

Merge request reports

Loading