fix(web): resourceId codificado en currentPath de pre-registro + test anti-drift - #344
Merged
vgpastor merged 5 commits intoJul 6, 2026
Conversation
… anti-drift de rutas Sigue GlobalEmergency#336/GlobalEmergency#278: el currentPath de la página de pre-registro interpolaba el resourceId sin codificar en el query string, así que un id con &, # o espacio podía romper el next del login en el round-trip de vuelta. Además, para el drift de los ~24 literales de ruta pasados a AppBar.currentPath (sin nada que garantice que coinciden con la carpeta real bajo src/app), se añade un test que recorre cada page.tsx y verifica que su currentPath coincide con la ruta derivada del fichero. Se prefiere esta opción ligera al refactor de exponer el pathname vía header de proxy: ese approach exigiría un matcher de proxy que corra en TODAS las rutas solo para fijar el header (impacto en cada request) y, si se reutilizara el matcher de auth existente, volvería a acoplar rutas no protegidas al gate de auth — el motivo exacto por el que GlobalEmergency#336 ya lo había descartado.
|
@nopestack is attempting to deploy a commit to the GlobalEmergency Team on Vercel. A member of the Team first needs to authorize it. |
…ceptionFilter OAuthExceptionFilter (GlobalEmergency#340) captura cualquier fallo del callback OAuth, incluido el UnauthorizedException por CSRF state inválido, y responde con un 302 a /login?error=oauth_failed en vez del 401 crudo anterior. Los 6 tests que esperaban 401 quedaron rotos porque el comportamiento nuevo es intencional (GlobalEmergency#340) y el test no se actualizó. Se actualizan las aserciones para esperar el 302 con el Location correcto y se verifica directamente lo que sí importa para CSRF: la cookie rh_oauth_state se invalida (Expires en el pasado) y nunca se emite un accessToken/sesión. Closes GlobalEmergency#346
Contributor
Author
|
Nota: incorporé un merge de |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #342
Contexto
Seguimiento de la review no bloqueante de #336 (fix #278), que añadió
AppBar.currentPathpara que elnextdel login apunte a la página exacta.Cambios
resourceIdcodificado en elcurrentPathde pre-registro. Enapps/web/src/app/e/[slug]/pre-registro/page.tsxelcurrentPathinterpolabaresourceIdcrudo en un query string:`/e/${slug}/pre-registro?resourceId=${resourceId}`. Un id con&,#o espacio podía dejar elredirect(next)de vuelta con una forma inesperada al reconstruir la query. Fix de una línea:encodeURIComponent(resourceId).Test anti-drift para los ~24 literales de ruta. Quedan ~24 páginas que pasan su
currentPathcomo literal duplicando el routing file-based desrc/app, sin nada que garantice que el string coincide con la carpeta real: un rename o un typo pasaría build/lint/tests y solo se notaría en runtime como unnextde login incorrecto.Se evaluaron dos opciones:
headers(), sin ampliar el matcher de auth deproxy.ts). Se descartó para este follow-up: obligaría a un matcher de proxy corriendo en todas las rutas solo para fijar un header (coste en cada request), y si en cambio se reutilizara el matcher de auth ya existente, se reintroduciría exactamente el acoplamiento que fix(web): AppBar usa el pathname exacto para el next del login (#278) #336 ya había descartado (rutas no protegidas entrando al gate de auth). Es una opción válida pero de mayor alcance que el de este issue.apps/web/src/lib/app-bar-current-path-drift.test.ts, que recorre cadapage.tsxbajosrc/app, deriva la ruta real desde la carpeta del fichero ([slug]→${slug}, etc.) y comprueba que cadacurrentPathdeclarado coincide. Verificado que detecta el drift: forzar un literal desincronizado (ej.registrar→registrarrr) hace fallar el test.Se optó por la segunda: mismo beneficio (atrapar el drift antes de runtime) con una fracción del riesgo/alcance para un follow-up pequeño.
Nota: los tests de
apps/web(pnpm --filter web test, incl. los preexistentes ensrc/lib/src/domain) no están cableados en el CI actual (.github/workflows/ci.ymlsolo correpnpm --filter web lint/build) ni en el gate documentado enAGENTS.md. Es una brecha preexistente, no introducida por este PR — se deja fuera de alcance, pero se señala aquí por si se quiere abrir un issue aparte.Test plan
pnpm --filter web test— 94/94 tests OK (92 preexistentes + 2 nuevos).currentPath(registrar→registrarrr) y el nuevo test lo detectó; se restauró después.pnpm install --frozen-lockfilepnpm --filter @reliefhub/api-client build && pnpm --filter web build— OKpnpm --filter web lint— 0 errores (1 warning preexistente sin relación, enopengraph-image.tsx)pnpm --filter api build— OK (sin cambios enapps/api)