IIC2513-2021-2 / project

Repositorio oficial para el proyecto del curso IIC2513, período 2021-2
32 stars 1 forks source link

checkAuth - Renderizar página de login y entregar código 401 al mismo tiempo #29

Open benjavicente opened 2 years ago

benjavicente commented 2 years ago

Hola!

La implementación de soundify (auth, manejo de errores en routes) funciona de la siguiente manera:

checkAuth --no-current-user--> throw(401) --|-> catch 401 --> redirect('log-in')
    |                                       |
   ok!          middlewares/auth.js         |           routes.js#L12-L25
    |                                       |
    V                         
  next()  

Esto genera un problema en la aplicación que nos toca hacer nosotros, porque hay 2 formas en que un usuario puede autentificarse al recibir un error 401, en el log-in de usuarios comunes y en el log-in de administradores.

Entonces mi pregunta es: ¿existe una manera facil para re-derigir y entregar un código de error al mismo tiempo, para no depender de un catch global?

 checkAuth --no-current-user--> ( throw(401) & redirect('log-in'))
checkAdmin --no-current-admin-> ( throw(401) & redirect('admin/log-in'))

Traté lo que se menciona en la documentación de koa y parece no funcionar con errores que no son 3XX:

ctx.status = 301;
ctx.redirect('/cart');

También probé con curl el deploy de soundify, y me di cuenta que la estrategia de usar catch no entrega 401

$ curl -i "https://soundify-iic2513.herokuapp.com/albums/new"
HTTP/1.1 302 Found

Será un render de un HTML con redirect la única opción? 🤔

meretamal commented 2 years ago

Hola 😁, es probable que el request que hiciste a soundify entregó un 302 debido a que se hace un catch del error (401) y luego se hace un redirect al login (lo cual explica el 302).

En torno al problema que estás teniendo, muy a la rápida, se me ocurre que:

  1. Todos los urls que involucren al admin contengan el string '/admin' en el path.
  2. En el catch del 401 accedas a la url del request (prueba con ctx.url) y veas si '/admin' es parte del url del request y ahí manejes a donde redirigir.

No estoy muy satisfecho con esa solución jaja, así que, si se me ocurre algo más limpio, te aviso.

meretamal commented 2 years ago

Ahora, lo anterior es para poder mantener tanto el status 401 y 3xx que se levanta y seguir con el flujo ya implementado. Encuentro raro que no haya funcionado hacer un redirect a mano dentro del router del admin.

Tienes tu código en alguna rama, para poder revisarlo?

benjavicente commented 2 years ago

Todavía no, pero con lo he investigado voy a hacer simplemente un redirect por ahora, sin el catch global:

function checkAuth(ctx, next) {
  return ctx.state.currentUser ? next() : ctx.redirect('login')
}

Tal vez vea si se puede crear una función tipo redirect_with_html(code, url) para poder tener el 401 y el redirect 🤔

Ahora, lo anterior es para poder mantener tanto el status 401 y 3xx que se levanta y seguir con el flujo ya implementado.

¿Pero el 401 se pierde o no? Como que al cacth hace que cambie de 401 a un redirect 3xx.


Edit: probando páginas que requieren login en varios servicios como Google, GitHub y Stripe, noté que la mayoría entrega un error 302, 303 o hasta 200, como que 401 no se usa (al menos en las rutas que entregan HTML).

meretamal commented 2 years ago

Sisi, me refería a seguir levantando el 401 para que entre al catch y así seguir con la misma lógica que ya está implementada

sivicencio commented 2 years ago

Una alternativa a lo que plantea @meretamal como solución dentro del middleware de errores es utilizar el nombre de la ruta que se le hizo match, en vez del path o url. En general si sigues un esquema ordenado y jerárquico de nombres de rutas, deberías ser capaz de identificar el nombre de una ruta de admin.

De la documentación de koa-router:

When a route is matched, its path is available at ctx._matchedRoute and if named, the name is available at ctx._matchedRouteName