movidas openspec a /docs
This commit is contained in:
parent
0ef0c9951c
commit
aae6b6096e
9 changed files with 486 additions and 0 deletions
112
docs/openspec/changes/security-audit-hardening/design.md
Normal file
112
docs/openspec/changes/security-audit-hardening/design.md
Normal file
|
|
@ -0,0 +1,112 @@
|
|||
## Context
|
||||
|
||||
`website_sale_aplicoop` expone el portal de compra colaborativa (Eskaera) a usuarios de portal, y
|
||||
la API externa de Odoo (XML-RPC/JSON-RPC) es accesible aunque el puerto de la BD esté firewallado.
|
||||
La recon del código confirmó cinco hallazgos (H1–H5) y varios puntos de config de infra. Producción
|
||||
corre tras nginx/traefik con TLS; el `docker-compose.yml` del repo es lab/dev y no refleja prod.
|
||||
|
||||
Los controladores de eskaera ya usan `sudo()` para sus escrituras y ya disponen de gatekeepers de
|
||||
pertenencia (`_validate_user_group_access`, `_get_consumer_group_for_user` en
|
||||
`controllers/website_sale_validators.py`); el problema es que las ACL/record rules son demasiado
|
||||
permisivas y que algunos endpoints no invocan esos gatekeepers.
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
- Cerrar H1–H5 con mínimo cambio de comportamiento observable para el usuario legítimo, respaldado
|
||||
por tests que impidan regresión.
|
||||
- Entregar un proceso de auditoría repetible (checklist, runbook, scripts, findings register).
|
||||
- Documentar y aplicar hardening de infra (odoo.conf, proxy, fail2ban, secretos/BD).
|
||||
|
||||
**Non-Goals:**
|
||||
- No se corrigen hallazgos nuevos de la auditoría activa en este change (se registran).
|
||||
- No se toca `ocb/` ni addons OCA originales.
|
||||
- No se rediseña la UX del portal; los cambios de endpoint son de transporte/seguridad.
|
||||
|
||||
## Decisions
|
||||
|
||||
**D1 — ACL de `group.order`/`group.order.slot` (H1): reemplazar la fila de `group_id` vacío por
|
||||
ACLs explícitas de mínimo privilegio.** La fila vacía concede write+create a todos. Se sustituye
|
||||
por: lectura interna (`base.group_user`, `1,0,0,0`, que empareja con la record rule interna ya
|
||||
existente), lectura de portal explícita donde haga falta, y escritura solo para
|
||||
`group_group_order_manager`. Alternativa descartada: añadir record rules que bloqueen write/create a
|
||||
portal — más frágil que quitar el permiso en la ACL, porque una ACL permisiva sin regla restrictiva
|
||||
ya abre el acceso. Verificar si el portal lee `group.order.slot` por ACL o por `sudo()` en el
|
||||
controlador; si es por `sudo()`, no añadir ACL de portal para el slot.
|
||||
|
||||
**D2 — `product.supplierinfo` (H2): acotar el acceso, no eliminarlo.** El portal SÍ usa
|
||||
`supplierinfo`: la web pinta el **origen** del producto y el **proveedor principal** a partir de él
|
||||
(vía `product.seller_ids`), además de que el pricing se computa server-side en
|
||||
`controllers/website_sale_pricing.py` vía `sudo()`. Por tanto no se puede quitar la ACL de portal
|
||||
sin romper ese render. Dos opciones, en orden de preferencia:
|
||||
- **(preferida) Preparar origen + proveedor principal en el controlador** vía `sudo()` y pasarlos ya
|
||||
resueltos a la plantilla (alineado con la regla del repo "sin lógica en QWeb"). Entonces el portal
|
||||
deja de necesitar ACL directa sobre `product.supplierinfo` → se elimina la ACL de portal (fila 6) y
|
||||
la record rule `rule_product_supplierinfo_portal_read`. Es lo más seguro: no expone ningún coste.
|
||||
- **(fallback) Acotar la record rule** para que el portal solo lea `supplierinfo` de los productos de
|
||||
los grupos a los que pertenece, sustituyendo `domain=[(1,'=',1)]` por un dominio filtrado, y sin
|
||||
exponer campos de coste/precio (restringir con `groups=` en el campo o exponer campo derivado).
|
||||
|
||||
El punto clave: hay que verificar en las plantillas/controladores qué campos de `supplierinfo` se
|
||||
leen realmente (origen, nombre del proveedor) antes de elegir; el objetivo es no exponer
|
||||
`price`/coste de todos los proveedores a cualquier usuario de portal.
|
||||
|
||||
**D3 — CSRF (H3): convertir a `type="json"`.** Los endpoints de estado (`save-order`, `confirm`,
|
||||
`clear-cart`, `save-cart`) pasan de `type="http"` + `csrf=False` a `type="json"`. Un `type="json"`
|
||||
exige `Content-Type: application/json`, que un formulario HTML cross-site no puede fijar, de modo
|
||||
que neutraliza el CSRF por formulario sin gestionar tokens manualmente. Plantilla de referencia:
|
||||
`confirm_order_from_portal` (ya `type="json"`). Implica ajustar el JS (solo transporte: fetch con
|
||||
envelope JSON-RPC y lectura de `result`) y el Python (devolver `dict` en vez de `Response`).
|
||||
Alternativa descartada: mantener `type="http"` y validar token CSRF — más código y más fácil de
|
||||
olvidar en endpoints futuros.
|
||||
|
||||
**D4 — Autorización horizontal (H4): invocar los gatekeepers existentes.** En cada endpoint que hace
|
||||
`group.order.sudo().browse(order_id)` y solo comprueba `exists()`/`state`, añadir la comprobación de
|
||||
pertenencia tras ese check. Usar `_get_consumer_group_for_user` (devuelve `False`) en los que deban
|
||||
retornar vacío/redirect silencioso (`load_eskaera_page`, `load_products_ajax`) y
|
||||
`_validate_user_group_access` (lanza) en los que deban fallar duro. Mantener el bypass para usuarios
|
||||
internos (`current_user.share == False`) igual que `eskaera_shop`.
|
||||
|
||||
**D5 — Plantilla (H5): salida JSON segura.** Sustituir `t-raw` en `<script>` de
|
||||
`load_from_history_templates.xml` por `<script type="application/json">` leído por el JS, evitando
|
||||
inyección raw en contexto de script.
|
||||
|
||||
**D6 — Proceso de auditoría: docs + scripts parametrizados.** Checklist, runbook y findings register
|
||||
en `docs/`; scripts en `scripts/security/` que toman URL y credenciales por parámetro/env (sin
|
||||
secretos versionados) y son no destructivos. El harness de fuerza bruta y el fuzzing corren contra
|
||||
staging o en ventana, nunca contra cuentas reales.
|
||||
|
||||
**D7 — Hardening de infra: requisitos documentados + cambio versionable puntual.** La config de
|
||||
`odoo.conf`, nginx/traefik y fail2ban vive en el servidor (no versionable aquí); se documenta como
|
||||
requisitos verificables en el checklist. El único cambio versionable de este bloque es añadir
|
||||
`groups_id` a la server action de mandatos SEPA en `account_banking_mandate_batch`.
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- [Convertir endpoints a `type="json"` rompe el carrito/checkout] → Tests de endpoint + verificación
|
||||
end-to-end del flujo de portal antes de mergear; cambios de JS limitados a transporte.
|
||||
- [Recortar `supplierinfo` rompe la visualización de precios] → Confirmar que el pricing se computa
|
||||
vía `sudo()`; test que verifica que el shop sigue mostrando precios.
|
||||
- [Endurecer ACL bloquea a un usuario interno legítimo que dependía del permiso global] → La record
|
||||
rule interna de lectura ya existe; el test cubre lectura interna y escritura de manager.
|
||||
- [Auditoría activa afecta a producción] → Snapshot previo, staging preferente, ventana de
|
||||
mantenimiento, cuentas de test dedicadas, monitorización en vivo.
|
||||
- [`proxy_mode=True` mal configurado falsea la IP de origen] → Verificar cabeceras `X-Forwarded-For`
|
||||
en nginx y comprobar en la Fase 1 que fail2ban ve la IP real de Kali.
|
||||
|
||||
## Migration Plan
|
||||
|
||||
1. Rama aparte para los fixes de código; la auditoría activa se ejecuta contra staging.
|
||||
2. Aplicar D1–D5 en `website_sale_aplicoop` + tests; bump de `__manifest__.py`.
|
||||
3. Actualizar el addon: `docker-compose run odoo odoo -d odoo --stop-after-init -u website_sale_aplicoop`.
|
||||
Rollback: revertir la rama y re-actualizar el addon (las ACL/record rules se recargan al -u).
|
||||
4. Aplicar D7 (config de servidor) en ventana; rollback = restaurar los ficheros de config previos y
|
||||
recargar servicios.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- ¿El portal lee `group.order.slot` y `product.supplierinfo` directamente en algún punto, o todo va
|
||||
por `sudo()`? Resolver leyendo los controladores antes de eliminar ACLs (afecta a D1/D2).
|
||||
- ¿Se usa traefik o nginx en prod? Ajusta la sintaxis de rate-limit/cabeceras del runbook (no el
|
||||
requisito).
|
||||
- ¿Externalizar secretos a `.env`/secrets del orquestador en este change o en uno posterior de infra?
|
||||
Loading…
Add table
Add a link
Reference in a new issue