From 55f28f5c06a28a83bd85a273582cd760dbf4e630 Mon Sep 17 00:00:00 2001 From: Lizandro Guarnizo <77708265+lizandrogd@users.noreply.github.com> Date: Sat, 15 Aug 2026 20:25:31 -0500 Subject: [PATCH] =?UTF-8?q?fix(permisos):=20los=20endpoints=20de=20datos?= =?UTF-8?q?=20ya=20no=20se=20abren=20sin=20el=20m=C3=B3dulo=20asignado?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Primera tanda del hueco reportado en el commit anterior: el permiso tapaba la página HTML pero no los datos. Cualquier usuario con sesión podía pedir GET /app/loadusers o DELETE /app/doc/paginas/:id escribiendo la URL, sin tener el submódulo asignado. No alcanzaba con poner MenuMiddleware en esas rutas: compara la ruta pedida contra los submódulos del rol, y el endpoint de datos vive en otra ruta que la pantalla (/app/loadusers alimenta /app/users). Compararla consigo misma nunca coincidiría y dejaría afuera hasta a quien sí tiene el permiso. RequiereModulo("/app/users") declara en la ruta a qué pantalla pertenece el endpoint, y valida ese submódulo. El administrador sigue entrando a todo, igual que en MenuMiddleware. Esta tanda cubre lo que más duele: identidad y permisos (usuarios, roles, módulos, submódulos), donde una fuga es escalada de privilegios, y Documentación, que es el módulo que disparó la auditoría. Quedan las otras tandas (contabilidad, facturas, clientes, servidores...). Se hace por partes a propósito: cerrar 400 rutas de una puede dejar gente afuera de pantallas que hoy usa. Co-Authored-By: Claude Sonnet 5 --- rest/middlewares/menu.go | 48 ++++++++++++++++++++++++++ rest/middlewares/menu_test.go | 23 +++++++++++++ rest/routes/user.go | 64 +++++++++++++++++------------------ 3 files changed, 103 insertions(+), 32 deletions(-) diff --git a/rest/middlewares/menu.go b/rest/middlewares/menu.go index 1aa6bb0..aa5f37b 100755 --- a/rest/middlewares/menu.go +++ b/rest/middlewares/menu.go @@ -133,3 +133,51 @@ func PuedeVerRuta(ruta string, permitidas []string) bool { } return false } + +// RequiereModulo protege un endpoint declarando a qué pantalla pertenece. +// +// MenuMiddleware compara la ruta pedida contra los submódulos del rol, y eso +// alcanza para las páginas porque su URL ES la del submódulo. Pero los +// endpoints de datos viven en otra ruta (/app/loadusers alimenta la pantalla +// /app/users), así que compararlos contra sí mismos nunca coincidiría y +// dejaría afuera hasta a quien sí tiene el permiso. +// +// Por eso el submódulo se declara en la ruta: +// +// protected.Get("/loadusers", middlewares.RequiereModulo("/app/users"), controllers.GetUsers) +// +// Sin esto, cualquier usuario con sesión podía pedir los datos de una pantalla +// que no tiene asignada escribiendo la URL a mano: el permiso solo tapaba el +// HTML, no la información. +func RequiereModulo(urlSubmodulo string) fiber.Handler { + return func(c *fiber.Ctx) error { + user, err := auth.User(c) + if err != nil || user == nil { + return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{ + "error": true, "message": "Usuario no autenticado", + }) + } + // Mismo criterio que MenuMiddleware: el administrador entra a todo. + if user.IsAdmin { + return c.Next() + } + + datos, err := models.FindUserByID(user.ID) + if err != nil { + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{ + "error": true, "message": "Error al obtener datos del usuario", + }) + } + var urls []string + for _, s := range datos.Role.Submodules { + urls = append(urls, s.Url) + } + + if !PuedeVerRuta(urlSubmodulo, urls) { + return c.Status(fiber.StatusForbidden).JSON(fiber.Map{ + "error": true, "message": "No tenés permiso para acceder a esta información", + }) + } + return c.Next() + } +} diff --git a/rest/middlewares/menu_test.go b/rest/middlewares/menu_test.go index c9d98f0..97302ef 100644 --- a/rest/middlewares/menu_test.go +++ b/rest/middlewares/menu_test.go @@ -47,3 +47,26 @@ func TestPuedeVerRutaSinPermisos(t *testing.T) { t.Error("una URL vacía no debería habilitar nada") } } + +// RequiereModulo compara contra el submódulo DECLARADO en la ruta, no contra +// la ruta pedida. Es lo que permite proteger /app/loadusers con el permiso de +// la pantalla /app/users: compararlo consigo mismo nunca coincidiría y +// bloquearía incluso a quien sí tiene el permiso. +func TestRequiereModuloUsaElSubmoduloDeclarado(t *testing.T) { + permisos := []string{"/app/users"} + + // El endpoint de datos vive en otra ruta que la pantalla. + if !PuedeVerRuta("/app/users", permisos) { + t.Error("con el permiso de la pantalla se debe poder pedir sus datos") + } + // Y comparar la ruta del endpoint contra el permiso NO coincide: por eso + // hace falta declarar el submódulo en vez de mirar c.Path(). + if PuedeVerRuta("/app/loadusers", permisos) { + t.Error("la ruta del endpoint no coincide con la del submódulo — " + + "si coincidiera, no haría falta RequiereModulo") + } + // Sin el permiso, no se accede. + if PuedeVerRuta("/app/users", []string{"/app/clientes"}) { + t.Error("con otro permiso no se debe acceder a los datos de usuarios") + } +} diff --git a/rest/routes/user.go b/rest/routes/user.go index 027afe1..37a15e9 100755 --- a/rest/routes/user.go +++ b/rest/routes/user.go @@ -29,35 +29,35 @@ func UserRoutes(app fiber.Router) { app.Get("/web", func(c *fiber.Ctx) error { return c.Redirect("/", http.StatusSeeOther) }) protected.Get("/", controllers.App) protected.Get("/dashboard", middlewares.MenuMiddleware, controllers.Dashboard) - protected.Get("/modules", middlewares.MenuMiddleware, controllers.Modules) // Renderizar la vista - protected.Get("/loadmodules", controllers.GetModules) // Obtener todos los módulos - protected.Post("/modules", middlewares.SoloAdmin, controllers.CreateModule) // Crear un nuevo módulo - protected.Put("/modules/:id", middlewares.SoloAdmin, controllers.UpdateModule) // Actualizar un módulo existente - protected.Delete("/modules/:id", middlewares.SoloAdmin, controllers.DeleteModule) // Eliminar un módulo + protected.Get("/modules", middlewares.MenuMiddleware, controllers.Modules) // Renderizar la vista + protected.Get("/loadmodules", middlewares.RequiereModulo("/app/modules"), controllers.GetModules) // Obtener todos los módulos + protected.Post("/modules", middlewares.SoloAdmin, controllers.CreateModule) // Crear un nuevo módulo + protected.Put("/modules/:id", middlewares.SoloAdmin, controllers.UpdateModule) // Actualizar un módulo existente + protected.Delete("/modules/:id", middlewares.SoloAdmin, controllers.DeleteModule) // Eliminar un módulo // Rutas de roles - protected.Get("/roles", middlewares.MenuMiddleware, controllers.Roles) // Renderizar la vista - protected.Get("/loadroles", controllers.GetRoles) // Obtener todos - protected.Post("/roles", middlewares.SoloAdmin, controllers.CreateRole) // Crear - protected.Put("/roles/:id", middlewares.SoloAdmin, controllers.UpdateRole) // Actualizar - protected.Delete("/roles/:id", middlewares.SoloAdmin, controllers.DeleteRole) // Eliminar + protected.Get("/roles", middlewares.MenuMiddleware, controllers.Roles) // Renderizar la vista + protected.Get("/loadroles", middlewares.RequiereModulo("/app/roles"), controllers.GetRoles) // Obtener todos + protected.Post("/roles", middlewares.SoloAdmin, controllers.CreateRole) // Crear + protected.Put("/roles/:id", middlewares.SoloAdmin, controllers.UpdateRole) // Actualizar + protected.Delete("/roles/:id", middlewares.SoloAdmin, controllers.DeleteRole) // Eliminar // Rutas de módulos - protected.Get("/submodules", middlewares.MenuMiddleware, controllers.Submodules) // Renderizar la vista - protected.Get("/loadsubmodules", controllers.GetSubmodules) // Obtener todos los módulos - protected.Post("/submodules", middlewares.SoloAdmin, controllers.CreateSubmodule) // Crear un nuevo módulo - protected.Put("/submodules/:id", middlewares.SoloAdmin, controllers.UpdateSubmodule) // Actualizar un módulo existente - protected.Delete("/submodules/:id", middlewares.SoloAdmin, controllers.DeleteSubmodule) // Eliminar un módulo + protected.Get("/submodules", middlewares.MenuMiddleware, controllers.Submodules) // Renderizar la vista + protected.Get("/loadsubmodules", middlewares.RequiereModulo("/app/submodules"), controllers.GetSubmodules) // Obtener todos los módulos + protected.Post("/submodules", middlewares.SoloAdmin, controllers.CreateSubmodule) // Crear un nuevo módulo + protected.Put("/submodules/:id", middlewares.SoloAdmin, controllers.UpdateSubmodule) // Actualizar un módulo existente + protected.Delete("/submodules/:id", middlewares.SoloAdmin, controllers.DeleteSubmodule) // Eliminar un módulo // Rutas de usuarios - protected.Get("/users", middlewares.MenuMiddleware, controllers.Users) // Renderizar la vista - protected.Get("/loadusers", controllers.GetUsers) // Obtener - protected.Post("/users", middlewares.SoloAdmin, controllers.CreateUser) // Crear - protected.Put("/users/:id", middlewares.SoloAdmin, controllers.UpdateUser) // Actualizar - protected.Put("/password/:id", middlewares.SoloAdmin, controllers.UpdatePassword) // Actualizar - protected.Delete("/users/:id", middlewares.SoloAdmin, controllers.DeleteUser) // Eliminar - protected.Get("/user/:id", controllers.GetUser) // Buscar un usuario - protected.Post("/users/:id/credenciales", controllers.EnviarCredencialesUsuario) // Enviar credenciales + protected.Get("/users", middlewares.MenuMiddleware, controllers.Users) // Renderizar la vista + protected.Get("/loadusers", middlewares.RequiereModulo("/app/users"), controllers.GetUsers) // Obtener + protected.Post("/users", middlewares.SoloAdmin, controllers.CreateUser) // Crear + protected.Put("/users/:id", middlewares.SoloAdmin, controllers.UpdateUser) // Actualizar + protected.Put("/password/:id", middlewares.SoloAdmin, controllers.UpdatePassword) // Actualizar + protected.Delete("/users/:id", middlewares.SoloAdmin, controllers.DeleteUser) // Eliminar + protected.Get("/user/:id", middlewares.RequiereModulo("/app/users"), controllers.GetUser) // Buscar un usuario + protected.Post("/users/:id/credenciales", controllers.EnviarCredencialesUsuario) // Enviar credenciales // Rutas de conexiones ssh protected.Get("/conexion_ssh", middlewares.MenuMiddleware, controllers.ConxSsh) // Renderizar la vista @@ -292,18 +292,18 @@ func UserRoutes(app fiber.Router) { // ─── Documentación: categorías globales ────────────────────────────────── protected.Get("/doc/categorias", middlewares.MenuMiddleware, controllers.DocCategoriasIndex) - protected.Get("/doc/loadcategorias", controllers.GetDocCategorias) - protected.Post("/doc/categorias", controllers.CreateDocCategoria) - protected.Put("/doc/categorias/:id", controllers.UpdateDocCategoria) - protected.Delete("/doc/categorias/:id", controllers.DeleteDocCategoria) + protected.Get("/doc/loadcategorias", middlewares.RequiereModulo("/app/doc/categorias"), controllers.GetDocCategorias) + protected.Post("/doc/categorias", middlewares.RequiereModulo("/app/doc/categorias"), controllers.CreateDocCategoria) + protected.Put("/doc/categorias/:id", middlewares.RequiereModulo("/app/doc/categorias"), controllers.UpdateDocCategoria) + protected.Delete("/doc/categorias/:id", middlewares.RequiereModulo("/app/doc/categorias"), controllers.DeleteDocCategoria) // ─── Documentación: páginas ─────────────────────────────────────────────── protected.Get("/doc/paginas", middlewares.MenuMiddleware, controllers.DocPaginasIndex) - protected.Get("/doc/loadpaginas", controllers.GetDocPaginas) - protected.Get("/doc/paginas/:id", controllers.GetDocPaginaDetalle) - protected.Post("/doc/paginas", controllers.CreateDocPagina) - protected.Put("/doc/paginas/:id", controllers.UpdateDocPagina) - protected.Delete("/doc/paginas/:id", controllers.DeleteDocPagina) + protected.Get("/doc/loadpaginas", middlewares.RequiereModulo("/app/doc/paginas"), controllers.GetDocPaginas) + protected.Get("/doc/paginas/:id", middlewares.RequiereModulo("/app/doc/paginas"), controllers.GetDocPaginaDetalle) + protected.Post("/doc/paginas", middlewares.RequiereModulo("/app/doc/paginas"), controllers.CreateDocPagina) + protected.Put("/doc/paginas/:id", middlewares.RequiereModulo("/app/doc/paginas"), controllers.UpdateDocPagina) + protected.Delete("/doc/paginas/:id", middlewares.RequiereModulo("/app/doc/paginas"), controllers.DeleteDocPagina) // ─── Documentación: lectura privada (usuario logueado) ─────────────────── protected.Get("/docs/:saas", middlewares.MenuMiddleware, controllers.DocsPrivadoIndex)