Cómo corregí un redirect, y el caso límite que una línea dejó pasar

Este no salió de la nada, sino de una corrección anterior. Revisando la corrección del error 500 del panel de trabajos sin autenticar hace un par de días, noté que el flujo de login hace algo un poco imprudente: la URL a la que se intentaba llegar cuando la sesión expiraba queda guardada, y después de volver a iniciar sesión, se redirige directo hacia ella. Conveniente, cuando funciona. El error es que guarda la URL para cualquier solicitud, incluidas las que solo tienen sentido como POST, y después redirige ciegamente hacia ella con un GET. Un GET a una ruta exclusiva para POST no coincide con nada. 404, justo después de volver a iniciar sesión.

Qué construí

La corrección parecía pequeña al empezar: recordar la URL solo si la solicitud original era un GET (o un HEAD, que Rails trata como GET para efectos de enrutamiento). Omitirlo para cualquier otra cosa y dejar que el respaldo existente mande a la página principal en su lugar. Una línea, una palabra agregada.

Lo que me sorprendió

Lo pasé por Brakeman antes de abrir el PR, más que nada por costumbre a esta altura, y señaló la línea que acababa de escribir. No la lógica, el chequeo específico. Había escrito request.get? solo, y Brakeman señaló que una solicitud HEAD se colaría, aunque Rails enruta HEAD al mismo lugar que un GET. Hace una semana lo habría llamado una exquisitez sin importancia. No lo es. Si un navegador o una herramienta de monitoreo manda una solicitud HEAD a una página protegida, mi corrección de una línea la trataría como insegura y se saltearía guardar la ruta de retorno, cuando todo el punto era conservarla justamente para este tipo de solicitud de solo lectura. Corregido con || request.head?, que, según descubrí después al revisar mi propio diff, es la expresión literal que Rails usa internamente en su propio código de protección CSRF. No fue una decisión de estilo que inventé; es el nombre de una categoría que Rails ya reconoce y que yo solo había implementado a medias.

Pero el hallazgo real vino de la revisión de código. Señaló que saltearse la escritura para una solicitud insegura no es lo mismo que borrar lo que ya estaba guardado ahí. El escenario: alguien entra a una página sin autenticarse, rebota hacia el login, y la URL queda guardada. Antes de que esa persona llegue a iniciar sesión, algo más dispara un POST en segundo plano, un botón obsoleto en una página que estaba abierta, un reintento, lo que sea. Ese POST también queda interceptado y redirigido al login, pero bajo mi corrección, simplemente se saltea guardar algo nuevo. No toca lo que ya está en la sesión desde el GET anterior. Entonces cuando finalmente se inicia sesión, se termina en esa primera página sin relación en vez de algún lugar predecible. Esta vez no es un 404, es simplemente algo silenciosamente incorrecto.

Escribí la reproducción como prueba antes de tocar el código: GET, después POST, después iniciar sesión, verificar dónde termino. Falló exactamente como predijo la revisión. La corrección real fue una línea más, limpiar explícitamente el valor guardado en vez de solo saltearse la escritura, y la misma prueba se puso en verde.

Qué sigue

Nada pendiente. Lo que se me quedó grabado acá es cuánta distancia hubo entre “la corrección que escribí” y “la corrección que realmente cierra el hueco.” Tanto la herramienta como una segunda revisión encontraron cosas que yo estaba seguro de haber cubierto. Vale la pena recordar que la confianza no es lo mismo que la cobertura.