fix(permisos): compara la ruta completa y avisa de los menús que dan 404
Auditoría del sistema de permisos a partir del 404 en /app/doc/paginas. 1. La comparación de permisos usaba solo el último segmento de la URL (lo que va después del último '/'). Rompía en las dos direcciones: con permiso sobre /app/doc/categorias se entraba a cualquier otra ruta terminada en "categorias", y a la vez el permiso parecía no aplicarse donde sí correspondía porque el segmento coincidía por casualidad. Ahora se compara la ruta completa, cortando en el separador para que /app/doc/paginas no habilite /app/doc/paginas-privadas. 2. El submódulo de Documentación nunca se sembró, aunque las rutas y las vistas existen desde mayo. Había que crear el ítem del menú a mano, y una URL mal tipeada ahí se ve exactamente igual que un permiso mal asignado: un 404. Ahora se siembra con la URL correcta. 3. El submódulo "Statuspage" apuntaba a /app/statuspage, que no existe en ninguna parte — la página real es la pública /status. Se corrige el seed y también el registro ya creado en la base. 4. Chequeo en el arranque que recorre los submódulos de la base y avisa cuáles apuntan a una URL sin ruta. Cubre los creados a mano, que es justo donde el compilador y los tests no llegan. 5. Test que cruza las URLs sembradas contra las rutas registradas: fue el que encontró lo de statuspage. Queda pendiente y es más grande: de 454 rutas protegidas solo 54 pasan por MenuMiddleware. El permiso gatea la página HTML pero no los endpoints de datos — cualquier usuario autenticado puede llamar a GET /app/doc/loadpaginas o DELETE /app/doc/paginas/:id sin tener el submódulo asignado. Se reporta antes de tocarlo porque cerrarlo de golpe puede dejar gente afuera. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Lizandro Guarnizo
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
ad3458bb71
commit
105ab44759
+34
-19
@@ -87,25 +87,15 @@ func MenuMiddleware(c *fiber.Ctx) error {
|
||||
return c.Next()
|
||||
}
|
||||
|
||||
// Verificar si la URL de la solicitud está en la lista de URLs permitidas
|
||||
requestURL := c.Path()
|
||||
|
||||
// Obtener el índice del último '/' en la URL de la solicitud
|
||||
lastSlashIndex := strings.LastIndex(requestURL, "/")
|
||||
if lastSlashIndex != -1 {
|
||||
// Obtener solo la parte de la URL después del último '/'
|
||||
requestURL = requestURL[lastSlashIndex+1:]
|
||||
}
|
||||
|
||||
// Verificar si la URL modificada está en la lista de URLs permitidas
|
||||
urlAllowed := false
|
||||
for _, url := range urls {
|
||||
// Comparar solo la parte de la URL después del último '/'
|
||||
if requestURL == url[strings.LastIndex(url, "/")+1:] {
|
||||
urlAllowed = true
|
||||
break
|
||||
}
|
||||
}
|
||||
// Se compara la ruta COMPLETA contra las URLs permitidas.
|
||||
//
|
||||
// Antes se comparaba solo el último segmento (lo que va después del último
|
||||
// '/'), y eso rompía en las dos direcciones: dos rutas distintas que
|
||||
// terminan igual quedaban indistinguibles — con permiso sobre
|
||||
// /app/doc/categorias se entraba a cualquier otra ruta terminada en
|
||||
// "categorias" — y a la vez el permiso parecía no aplicarse donde sí
|
||||
// debía, porque el segmento coincidía por casualidad.
|
||||
urlAllowed := PuedeVerRuta(c.Path(), urls)
|
||||
|
||||
// Si la URL no está permitida, devolver un error
|
||||
if !urlAllowed {
|
||||
@@ -118,3 +108,28 @@ func MenuMiddleware(c *fiber.Ctx) error {
|
||||
// Continuar con la siguiente función de middleware o manejador
|
||||
return c.Next()
|
||||
}
|
||||
|
||||
// PuedeVerRuta decide si una ruta está cubierta por alguna de las URLs que el
|
||||
// rol tiene asignadas.
|
||||
//
|
||||
// Coincide la ruta exacta o cualquier ruta por debajo de ella: quien tiene
|
||||
// /app/doc/paginas también puede entrar a /app/doc/paginas/7, que es la misma
|
||||
// pantalla con un detalle. No coincide /app/doc/paginas-privadas, porque el
|
||||
// corte se hace en el separador y no en el prefijo de texto — si no, un
|
||||
// permiso abriría rutas vecinas que solo comparten el comienzo del nombre.
|
||||
func PuedeVerRuta(ruta string, permitidas []string) bool {
|
||||
ruta = strings.TrimRight(ruta, "/")
|
||||
if ruta == "" {
|
||||
ruta = "/"
|
||||
}
|
||||
for _, u := range permitidas {
|
||||
u = strings.TrimRight(strings.TrimSpace(u), "/")
|
||||
if u == "" {
|
||||
continue
|
||||
}
|
||||
if ruta == u || strings.HasPrefix(ruta, u+"/") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
package middlewares
|
||||
|
||||
import "testing"
|
||||
|
||||
// El permiso comparaba solo el último segmento de la URL, así que dos rutas
|
||||
// distintas terminadas igual eran indistinguibles. Estos casos fijan que la
|
||||
// comparación es sobre la ruta completa y corta en el separador.
|
||||
func TestPuedeVerRuta(t *testing.T) {
|
||||
permisos := []string{"/app/doc/paginas", "/app/clientes", "/status"}
|
||||
|
||||
casos := []struct {
|
||||
ruta string
|
||||
esperado bool
|
||||
porque string
|
||||
}{
|
||||
{"/app/doc/paginas", true, "coincidencia exacta"},
|
||||
{"/app/doc/paginas/7", true, "el detalle cuelga de la pantalla permitida"},
|
||||
{"/app/doc/paginas/", true, "la barra final no cambia nada"},
|
||||
{"/app/clientes", true, "otra pantalla permitida"},
|
||||
{"/status", true, "una ruta pública también puede ser un ítem del menú"},
|
||||
|
||||
{"/app/doc/categorias", false, "otra pantalla del mismo módulo"},
|
||||
{"/app/umind", false, "sin permiso"},
|
||||
{"/app/doc", false, "el padre no se hereda del hijo"},
|
||||
|
||||
// El bug original: comparaba solo lo que va después del último '/'.
|
||||
{"/app/otro/paginas", false, "termina igual pero es otra ruta"},
|
||||
// El corte va en el separador, no en el prefijo de texto.
|
||||
{"/app/doc/paginas-privadas", false, "solo comparte el comienzo del nombre"},
|
||||
{"/app/clientes-vip", false, "prefijo parecido, ruta distinta"},
|
||||
}
|
||||
|
||||
for _, cas := range casos {
|
||||
if got := PuedeVerRuta(cas.ruta, permisos); got != cas.esperado {
|
||||
t.Errorf("PuedeVerRuta(%q) = %v, esperaba %v — %s", cas.ruta, got, cas.esperado, cas.porque)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Sin permisos no se entra a ningún lado (los administradores se resuelven
|
||||
// antes, con user.IsAdmin).
|
||||
func TestPuedeVerRutaSinPermisos(t *testing.T) {
|
||||
if PuedeVerRuta("/app/clientes", nil) {
|
||||
t.Error("un rol sin submódulos no debería acceder a nada")
|
||||
}
|
||||
if PuedeVerRuta("/app/clientes", []string{"", " "}) {
|
||||
t.Error("una URL vacía no debería habilitar nada")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,90 @@
|
||||
package routes
|
||||
|
||||
import (
|
||||
"os"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gofiber/fiber/v2"
|
||||
)
|
||||
|
||||
// Un submódulo del menú es una URL escrita a mano en el seed. Si no coincide
|
||||
// con ninguna ruta registrada, el ítem aparece en el menú, se le puede asignar
|
||||
// permiso a un rol, y al hacer clic da 404 — que es exactamente el síntoma
|
||||
// reportado en /app/doc/paginas. El compilador no ve nada de esto porque son
|
||||
// cadenas sueltas en dos archivos distintos.
|
||||
func TestUrlsDeSubmodulosTienenRuta(t *testing.T) {
|
||||
rutas := rutasGETRegistradas(t)
|
||||
|
||||
urls, err := urlsSembradas()
|
||||
if err != nil {
|
||||
t.Skipf("no se pudo leer el seed: %v", err)
|
||||
}
|
||||
if len(urls) == 0 {
|
||||
t.Fatal("no se encontró ninguna URL de submódulo en el seed")
|
||||
}
|
||||
|
||||
var rotas []string
|
||||
for _, u := range urls {
|
||||
if !rutas[u] {
|
||||
rotas = append(rotas, u)
|
||||
}
|
||||
}
|
||||
sort.Strings(rotas)
|
||||
for _, u := range rotas {
|
||||
t.Errorf("el submódulo %q no tiene ruta registrada: el menú lo muestra y da 404 al entrar", u)
|
||||
}
|
||||
}
|
||||
|
||||
// rutasGETRegistradas devuelve el conjunto de rutas GET que sirven una página.
|
||||
func rutasGETRegistradas(t *testing.T) map[string]bool {
|
||||
t.Helper()
|
||||
// LoadRoutes monta todos los grupos: un submódulo puede apuntar a una ruta
|
||||
// pública (ej. /status) y no solo a /app/*.
|
||||
app := fiber.New()
|
||||
LoadRoutes(app)
|
||||
|
||||
out := map[string]bool{}
|
||||
for _, capa := range app.Stack() {
|
||||
for _, r := range capa {
|
||||
if r.Method == "GET" {
|
||||
out[r.Path] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Solo las tuplas {título, descripción, url} de los slices de entries y las
|
||||
// asignaciones Url:. Buscar cualquier "/app/..." suelto daba falsos positivos
|
||||
// con las líneas que corrigen una URL vieja en la base.
|
||||
var reURLSubmodulo = regexp.MustCompile(`\{"[^"]*",\s*"[^"]*",\s*"(/[a-z0-9/_-]+)"\}|Url:\s*"(/[a-z0-9/_-]+)"`)
|
||||
|
||||
// urlsSembradas saca las URLs de submódulo del seed. Se lee el fuente porque
|
||||
// los seeds son literales en el código, no datos que se puedan consultar.
|
||||
func urlsSembradas() ([]string, error) {
|
||||
b, err := os.ReadFile("../../migrations/migrate.go")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
texto := string(b)
|
||||
|
||||
vistas := map[string]bool{}
|
||||
var out []string
|
||||
for _, m := range reURLSubmodulo.FindAllStringSubmatch(texto, -1) {
|
||||
u := m[1]
|
||||
if u == "" {
|
||||
u = m[2]
|
||||
}
|
||||
// Solo interesan las que se usan como Url de un submódulo; las de
|
||||
// redirección o comparación quedan cubiertas igual y no molestan.
|
||||
if vistas[u] || strings.Contains(u, "//") {
|
||||
continue
|
||||
}
|
||||
vistas[u] = true
|
||||
out = append(out, u)
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
Reference in New Issue
Block a user