Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sistema de Administradores y Moderadores #47

Merged
merged 2 commits into from
May 29, 2023
Merged

Sistema de Administradores y Moderadores #47

merged 2 commits into from
May 29, 2023

Conversation

fagiannoni
Copy link
Contributor

@fagiannoni fagiannoni commented May 29, 2023

¿Qué se implementa?

  • Manejo de roles con admin y mods (PAN-19).
  • Un usuario hardcodeado en el .env será admin. *Nota sobre esto al final.
  • El admin puede ver la lista de mods vigentes (GET), agregar mods (POST) o eliminar mods (DELETE) a través del endpoint /auth/mod (PAN-162).
  • Función para requerir acceso de mod (require_mod_auth) o admin (require_admin_auth) para ciertas rutas (e.g. /sync requiere permisos de admin PAN-161).
  • Endpoints para checkear autorización de accesos admin (GET /auth/check/admin) y mod (GET /auth/check/mod).

¿Qué falta?

  • En el PAN-19 se propone reconocer el rol de invitado, pero creo que esto se debería implementar en otra rama. Pensaba agregar simplemente agregar otra clase key como GuestKey para entregar ciertos permisos limitados a este tipo de usuario. Aunque igual pueden haber otras formas de proceder con la implementación así que prefiero dejar esto aquí para discutirlo.
  • Trackear JWTs emitidos a moderadores con el fin de poder revocar su acceso de forma instantánea, incluso antes de que el token expire. No creo que sea prioritario (quedó anotado como TODO).

Detalle de la implementación

  • Nuevo modelo AccessLevel en la base de datos que relaciona user_ruts a un atributo is_mod. Actualmente todos los registros de dicha tabla deberían tener is_mod=True y ser removidos en caso contrario, pero a futuro podría agregarse is_admin si se quiere tener la opción de múltiples admins (*como se comenta abajo en la nota).
  • Clases UserKey, ModKey(UserKey) y AdminKey(ModKey) para generar permisos de autorización. Notar que por herencia todo admin es mod, y todo mod es usuario.

Notas sobre la implementación

  • La idea propuesta en Linear conlleva poder tener múltiples admins, donde estos pueden dar/quitar accesos de admin. Decidí optar por otra vía (aunque la mayoría del código funciona para cualquier caso por lo que debería ser fácil cambiarlo) donde solamente existe 1 admin y puede entregar/quitar accesos de mod pero no admin. El RUT del admin estará hardcodeado en una variable de entorno directamente en la máquina. Pensé en los casos de uso y consideré que esta era la mejor forma de hacerlo, pero obviamente se puede extender todavía a la idea original de varios admins.

@fagiannoni fagiannoni requested a review from kovaxis May 29, 2023 10:59
@fagiannoni fagiannoni changed the title Sistema de Administradores y Moderadores Sistema de Administradores y Moderadores (PAN-19 PAN-161 PAN-162) May 29, 2023
@linear
Copy link

linear bot commented May 29, 2023

PAN-162 El administrador puede cambiar la lista de administradores y moderadores

PAN-161 Solo permitir a los administradores sincronizar la base de datos con fuentes externas

PAN-19 Manejo de roles

El sistema debe reconocer los distintos roles del sistema (invitado, usuario, moderador y administrador) y debe haber una forma establecida para revisar el rol desde las rutas del back.

@fagiannoni fagiannoni changed the title Sistema de Administradores y Moderadores (PAN-19 PAN-161 PAN-162) Sistema de Administradores y Moderadores May 29, 2023
@kovaxis
Copy link
Contributor

kovaxis commented May 29, 2023

Genial! Voy a revisarlo.

Respecto al GuestKey, no creo que sea necesario. Los invitados no necesitan autenticacion, por lo que me seria raro agregar algun tipo de GuestKey. Los endpoints que sean accesibles a invitados simplemente no debieran requerir ninguna key.

Copy link
Contributor

@kovaxis kovaxis left a comment

Choose a reason for hiding this comment

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

Me parece excelente, mergeable 👍 .

Respecto a hardcodear un admin en el .env, lo veo bien. Justo habia pensado en algo asi jasjdfjasj. Simplifica bien las cosas.

@kovaxis
Copy link
Contributor

kovaxis commented May 29, 2023

Respecto a revocar permisos antes que expiren los tokens, seria una buena mejora, pero estoy de acuerdo que no nos preocupemos tanto por ahora.

Si lo implementamos en algun momento, mas que trackear los tokens y revocarlos, no guardaria ninguna informacion de admin en el token mismo, y en vez buscaria en la tabla AccessLevel al decodear el token.

@kovaxis kovaxis merged commit f14d43f into main May 29, 2023
@fagiannoni fagiannoni deleted the admin-roles branch May 30, 2023 04:07
fagiannoni pushed a commit that referenced this pull request May 31, 2023
Sistema de Administradores y Moderadores
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.

2 participants