From c855777cf97930791acaa52ae3f44db43a403664 Mon Sep 17 00:00:00 2001 From: razyd Date: Thu, 8 Oct 2026 09:45:41 +0200 Subject: [PATCH 1/5] test(auth): pin current role access for excludeMantenimiento - Test excludeMantenimiento with 4 roles + missing user + uppercase - Test canOpenModule with 4 roles x 6 modules matrix - Verify 9 routers use excludeMantenimiento middleware --- backend/tests/auth/role-access.test.ts | 145 +++++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 backend/tests/auth/role-access.test.ts diff --git a/backend/tests/auth/role-access.test.ts b/backend/tests/auth/role-access.test.ts new file mode 100644 index 0000000..7197f0b --- /dev/null +++ b/backend/tests/auth/role-access.test.ts @@ -0,0 +1,145 @@ +// tests/auth/role-access.test.ts +// Pin the current role access behaviour for excludeMantenimiento and +// canOpenModule. This test documents what the code does today; B4 will +// change the implementation but keep these tests green. + +import { describe, it, expect, vi } from 'vitest' +import { excludeMantenimiento } from '../../middlewares/roleCheck.js' +import { canOpenModule, type AppModule } from '../../services/auth/module-access.js' +import { readFileSync } from 'fs' +import { resolve } from 'path' + +// Helper to create mock req/res/next +function createMockReqRes(role?: string) { + const req = { + user: role ? { role } : undefined, + } as any + const res = { + status: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), + } as any + const next = vi.fn() + return { req, res, next } +} + +describe('excludeMantenimiento', () => { + it('allows admin', () => { + const { req, res, next } = createMockReqRes('admin') + excludeMantenimiento(req, res, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it('allows recepcionista', () => { + const { req, res, next } = createMockReqRes('recepcionista') + excludeMantenimiento(req, res, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it('allows group-admin', () => { + const { req, res, next } = createMockReqRes('group-admin') + excludeMantenimiento(req, res, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it('blocks mantenimiento with 403', () => { + const { req, res, next } = createMockReqRes('mantenimiento') + excludeMantenimiento(req, res, next) + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(403) + }) + + it('blocks missing req.user with 403', () => { + const { req, res, next } = createMockReqRes(undefined) + excludeMantenimiento(req, res, next) + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(403) + }) + + it('allows ADMIN in uppercase (toLowerCase)', () => { + const { req, res, next } = createMockReqRes('ADMIN') + excludeMantenimiento(req, res, next) + expect(next).toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) +}) + +describe('canOpenModule', () => { + const roles = ['admin', 'recepcionista', 'group-admin', 'mantenimiento'] as const + const modules: AppModule[] = [ + 'parking', + 'maintenance', + 'groups', + 'blacklist', + 'cashier', + 'logbook', + ] + + // Expected results based on current implementation: + // - parking, blacklist, cashier, logbook: allButMaintenanceRole (all except mantenimiento) + // - maintenance: staffRoles (admin, recepcionista, group-admin, mantenimiento) + // - groups: staffRoles (admin, recepcionista, group-admin, mantenimiento) + const expected: Record> = { + admin: { + parking: true, + maintenance: true, + groups: true, + blacklist: true, + cashier: true, + logbook: true, + }, + recepcionista: { + parking: true, + maintenance: true, + groups: true, + blacklist: true, + cashier: true, + logbook: true, + }, + 'group-admin': { + parking: true, + maintenance: true, + groups: true, + blacklist: true, + cashier: true, + logbook: true, + }, + mantenimiento: { + parking: false, + maintenance: true, + groups: true, + blacklist: false, + cashier: false, + logbook: false, + }, + } + + it.each(roles)('role %s', (role) => { + for (const mod of modules) { + expect(canOpenModule(role, mod)).toBe(expected[role][mod]) + } + }) +}) + +describe('router wiring: excludeMantenimiento', () => { + const routersWithExcludeMantenimiento = [ + 'routes/blacklist/blacklist-routes.ts', + 'routes/cashier/cashier-routes.ts', + 'routes/checklist/checklist-routes.ts', + 'routes/conciliation/conciliation.routes.ts', + 'routes/departments/departments-routes.ts', + 'routes/logbook/logbook-routes.ts', + 'routes/parking/bookings.routes.ts', + 'routes/parking/parking.routes.ts', + 'routes/parking/stats.routes.ts', + ] + + it.each(routersWithExcludeMantenimiento)('%s uses excludeMantenimiento', (routerPath) => { + const fullPath = resolve(process.cwd(), routerPath) + const content = readFileSync(fullPath, 'utf-8') + expect(content).toContain('excludeMantenimiento') + expect(content).toMatch(/router\.use\(.*excludeMantenimiento/) + }) +}) From 843485ad9ba82609a22fb62564a700c08fff736f Mon Sep 17 00:00:00 2001 From: razyd Date: Fri, 9 Oct 2026 15:45:45 +0200 Subject: [PATCH 2/5] refactor(auth): replace the maintenance exclusion with an allowlist - excludeMantenimiento now uses allowedRoles = ['admin', 'recepcionista', 'group-admin'] instead of blockedRoles = ['mantenimiento'], so unknown roles are denied by default - module-access.ts: rename allButMaintenanceRole to frontDeskRoles with an explicit allowlist - role-access.test.ts: add 'auditor' role case (403 in middleware, false in all 6 modules) --- backend/middlewares/roleCheck.ts | 12 ++++++------ backend/services/auth/module-access.ts | 10 +++++----- backend/tests/auth/role-access.test.ts | 21 ++++++++++++++++++--- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/backend/middlewares/roleCheck.ts b/backend/middlewares/roleCheck.ts index 96230a2..d723786 100644 --- a/backend/middlewares/roleCheck.ts +++ b/backend/middlewares/roleCheck.ts @@ -159,24 +159,24 @@ export const canAccessMaintenance: RoleCheckMiddleware = (req, res, next) => { } /** - * Excluye al rol mantenimiento de acceder a módulos generales + * Verifica que el usuario tenga uno de los roles de mostrador * Uso: Para rutas de logbook, parking, blacklist, etc. - * El rol mantenimiento SOLO puede acceder a /api/maintenance + * Solo admin, recepcionista y group-admin pueden acceder; el resto (mantenimiento, etc.) no. */ export const excludeMantenimiento: RoleCheckMiddleware = (req, res, next) => { - const blockedRoles = ['mantenimiento'] + const allowedRoles = ['admin', 'recepcionista', 'group-admin'] if (!req.user?.role) { res.status(403).json({ error: 'No se pudo verificar el rol del usuario' }) return } - if (blockedRoles.includes(req.user.role.toLowerCase())) { + if (allowedRoles.includes(req.user.role.toLowerCase())) { + next() + } else { res.status(403).json({ error: 'Acceso denegado. Tu rol solo tiene acceso al módulo de mantenimiento', }) - } else { - next() } } diff --git a/backend/services/auth/module-access.ts b/backend/services/auth/module-access.ts index 13fff2f..70e63b8 100644 --- a/backend/services/auth/module-access.ts +++ b/backend/services/auth/module-access.ts @@ -7,14 +7,14 @@ export type AppModule = 'parking' | 'maintenance' | 'groups' | 'blacklist' | 'cashier' | 'logbook' -const allButMaintenanceRole = (role: string) => role !== 'mantenimiento' +const frontDeskRoles = (role: string) => ['admin', 'recepcionista', 'group-admin'].includes(role) const staffRoles = ['admin', 'recepcionista', 'group-admin', 'mantenimiento'] const MODULE_ACCESS: Record boolean> = { - parking: allButMaintenanceRole, // excludeMantenimiento - blacklist: allButMaintenanceRole, // excludeMantenimiento - cashier: allButMaintenanceRole, // excludeMantenimiento - logbook: allButMaintenanceRole, // excludeMantenimiento + parking: frontDeskRoles, // excludeMantenimiento + blacklist: frontDeskRoles, // excludeMantenimiento + cashier: frontDeskRoles, // excludeMantenimiento + logbook: frontDeskRoles, // excludeMantenimiento maintenance: (role) => staffRoles.includes(role), // canAccessMaintenance groups: (role) => staffRoles.includes(role), // canViewGroups } diff --git a/backend/tests/auth/role-access.test.ts b/backend/tests/auth/role-access.test.ts index 7197f0b..566ff06 100644 --- a/backend/tests/auth/role-access.test.ts +++ b/backend/tests/auth/role-access.test.ts @@ -64,10 +64,17 @@ describe('excludeMantenimiento', () => { expect(next).toHaveBeenCalled() expect(res.status).not.toHaveBeenCalled() }) + + it('blocks unknown role "auditor" with 403', () => { + const { req, res, next } = createMockReqRes('auditor') + excludeMantenimiento(req, res, next) + expect(next).not.toHaveBeenCalled() + expect(res.status).toHaveBeenCalledWith(403) + }) }) describe('canOpenModule', () => { - const roles = ['admin', 'recepcionista', 'group-admin', 'mantenimiento'] as const + const roles = ['admin', 'recepcionista', 'group-admin', 'mantenimiento', 'auditor'] as const const modules: AppModule[] = [ 'parking', 'maintenance', @@ -77,8 +84,8 @@ describe('canOpenModule', () => { 'logbook', ] - // Expected results based on current implementation: - // - parking, blacklist, cashier, logbook: allButMaintenanceRole (all except mantenimiento) + // Expected results based on implementation: + // - parking, blacklist, cashier, logbook: frontDeskRoles (admin, recepcionista, group-admin) // - maintenance: staffRoles (admin, recepcionista, group-admin, mantenimiento) // - groups: staffRoles (admin, recepcionista, group-admin, mantenimiento) const expected: Record> = { @@ -114,6 +121,14 @@ describe('canOpenModule', () => { cashier: false, logbook: false, }, + auditor: { + parking: false, + maintenance: false, + groups: false, + blacklist: false, + cashier: false, + logbook: false, + }, } it.each(roles)('role %s', (role) => { From 0c0cdb5aa85e18a1e29555fdd9e85e6003ddb75d Mon Sep 17 00:00:00 2001 From: razyd Date: Sat, 10 Oct 2026 18:21:45 +0200 Subject: [PATCH 3/5] test(auth): type the role access mocks --- backend/tests/auth/role-access.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/tests/auth/role-access.test.ts b/backend/tests/auth/role-access.test.ts index 566ff06..b9fbd9c 100644 --- a/backend/tests/auth/role-access.test.ts +++ b/backend/tests/auth/role-access.test.ts @@ -4,6 +4,7 @@ // change the implementation but keep these tests green. import { describe, it, expect, vi } from 'vitest' +import type { Request, Response } from 'express' import { excludeMantenimiento } from '../../middlewares/roleCheck.js' import { canOpenModule, type AppModule } from '../../services/auth/module-access.js' import { readFileSync } from 'fs' @@ -13,11 +14,11 @@ import { resolve } from 'path' function createMockReqRes(role?: string) { const req = { user: role ? { role } : undefined, - } as any + } as unknown as Request const res = { status: vi.fn().mockReturnThis(), json: vi.fn().mockReturnThis(), - } as any + } as unknown as Response const next = vi.fn() return { req, res, next } } From 4e827495ef280e9d66accbd30bee02035ad24359 Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 11:00:42 +0200 Subject: [PATCH 4/5] refactor(auth): share the front-desk allowlist and word the 403 for any role - module-access.ts: export FRONT_DESK_ROLES and build frontDeskRoles on it. - roleCheck.ts: excludeMantenimiento reads FRONT_DESK_ROLES instead of its own copy, and its 403 no longer says the role only reaches maintenance. Why: the allowlist lived in two files that had to be kept in step, and after the switch to an allowlist the old message was wrong for any role other than mantenimiento. --- backend/middlewares/roleCheck.ts | 7 +++---- backend/services/auth/module-access.ts | 5 ++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/backend/middlewares/roleCheck.ts b/backend/middlewares/roleCheck.ts index d723786..cb402bb 100644 --- a/backend/middlewares/roleCheck.ts +++ b/backend/middlewares/roleCheck.ts @@ -1,6 +1,7 @@ // middlewares/roleCheck.ts import { Request, Response, NextFunction } from 'express' +import { FRONT_DESK_ROLES } from '../services/auth/module-access.js' type RoleCheckMiddleware = (req: Request, res: Response, next: NextFunction) => void @@ -164,18 +165,16 @@ export const canAccessMaintenance: RoleCheckMiddleware = (req, res, next) => { * Solo admin, recepcionista y group-admin pueden acceder; el resto (mantenimiento, etc.) no. */ export const excludeMantenimiento: RoleCheckMiddleware = (req, res, next) => { - const allowedRoles = ['admin', 'recepcionista', 'group-admin'] - if (!req.user?.role) { res.status(403).json({ error: 'No se pudo verificar el rol del usuario' }) return } - if (allowedRoles.includes(req.user.role.toLowerCase())) { + if (FRONT_DESK_ROLES.includes(req.user.role.toLowerCase())) { next() } else { res.status(403).json({ - error: 'Acceso denegado. Tu rol solo tiene acceso al módulo de mantenimiento', + error: 'Acceso denegado. Tu rol no tiene acceso a este módulo', }) } } diff --git a/backend/services/auth/module-access.ts b/backend/services/auth/module-access.ts index 70e63b8..d688349 100644 --- a/backend/services/auth/module-access.ts +++ b/backend/services/auth/module-access.ts @@ -7,7 +7,10 @@ export type AppModule = 'parking' | 'maintenance' | 'groups' | 'blacklist' | 'cashier' | 'logbook' -const frontDeskRoles = (role: string) => ['admin', 'recepcionista', 'group-admin'].includes(role) +// Front-desk modules: an allowlist, so a new role gets no access until someone decides it +export const FRONT_DESK_ROLES: readonly string[] = ['admin', 'recepcionista', 'group-admin'] + +const frontDeskRoles = (role: string) => FRONT_DESK_ROLES.includes(role) const staffRoles = ['admin', 'recepcionista', 'group-admin', 'mantenimiento'] const MODULE_ACCESS: Record boolean> = { From 4c1023a5caf243b3265c0be3f4fc7ff9c2a55fbd Mon Sep 17 00:00:00 2001 From: razyd Date: Sun, 11 Oct 2026 11:00:43 +0200 Subject: [PATCH 5/5] docs(todo): drop the excludeMantenimiento allowlist, now done - Remove the "excludeMantenimiento is an exclusion list" entry. Why: the guard and module-access.ts share FRONT_DESK_ROLES, an allowlist; role-access.test.ts pins the four roles and denies an unknown one. --- docs/TODO.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 473ba85..539dd3f 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -286,13 +286,6 @@ preview. de Python. Enlazar la demo y el repo. Es el gancho para promocionar el proyecto. Se escribe en el repo `portfolio-bp`, no aquí. _Decidido el 2026-10-05._ -- [ ] **`excludeMantenimiento` es una lista de exclusión** — parking, lista - negra, caja, partes, conciliación y departamentos dejan pasar cualquier - rol salvo `mantenimiento`, y `services/auth/module-access.ts` lo replica. - Hoy solo hay roles conocidos (el rol va en el token firmado y sale de - `roles`), pero un rol nuevo tendría acceso sin que nadie lo decida. Pasar - a lista de permitidos. _Comprobado el 2026-10-05 al retirar - `demo-admin`._ - [ ] **`20260512_disable_demo_user.sql` desactivaría la cuenta demo** si se volviera a ejecutar (`UPDATE users SET is_active = 0 WHERE username = 'demo'`). `setup:local` no la ejecuta (es anterior al baseline); no