Jair Cedillo
a1793de2f2
tests/test_uploads_alcance.py llegó en el rebase del carril EFC (5c4df59) sin el código
de producción que lo acompañaba: venía de la rama del expediente paralelo que se
descartó. El módulo no importaba —`validar_extension` no existe— así que la suite
nunca corrió y el hueco quedó invisible.
El hueco: ambos endpoints solo comprobaban que la key empezara con
tenants/{tid}/companies/{cid}/. Con el permiso de módulo crm.access eso alcanzaba para
firmar o descargar CUALQUIER objeto de la company, incluidos certificates/*.key — la
llave privada del CSD con la que se sellan los CFDI.
- _validar_alcance: además del aislamiento por tenant/company, la key tiene que ser un
documento del CRM (crm-docs/ o expedientes/{n}/documents/). Se aplica a los dos
endpoints: el que entrega bytes no puede ser más laxo que el que firma una URL.
- validar_extension: allowlist de extensiones en la subida, no lista de vetados.
Verificado que el frontend solo pasa file_key de documentos a estos endpoints
(uploads.ts, sus dos únicos llamadores). Se agregan 5 casos para /uploads/download,
que la prueba original no cubría.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
123 lines
4.4 KiB
Python
123 lines
4.4 KiB
Python
"""Alcance de ``GET /uploads/url``: qué objetos puede firmar este endpoint y cuáles no.
|
|
|
|
**Cierra una fuga real.** Antes bastaba con que la key empezara por
|
|
``tenants/{tid}/companies/{cid}/`` para firmar una URL de lectura, lo que permitía firmar
|
|
**cualquier** objeto de esa company —incluidos los certificados de la FIEL— con solo el permiso de
|
|
módulo ``crm.access``. El alcance de este endpoint es «los archivos que el CRM subió», no «todo el
|
|
almacén de la company».
|
|
"""
|
|
|
|
import pytest
|
|
from fastapi import HTTPException
|
|
|
|
from api.v1.modules.crm.uploads.routes import download_file, get_upload_url, validar_extension
|
|
from tests.conftest import COMPANY_ID, TENANT_ID
|
|
|
|
USUARIO = {"tenant_id": TENANT_ID, "sub": "user-1"}
|
|
PREFIJO = f"tenants/{TENANT_ID}/companies/{COMPANY_ID}/"
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _sin_s3(monkeypatch):
|
|
import api.v1.modules.crm.uploads.routes as uploads
|
|
|
|
monkeypatch.setattr(uploads, "presigned_get_url", lambda key, **k: f"https://firmada/{key}")
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"sufijo",
|
|
[
|
|
"certificates/fiel_20260101.key", # llave privada de la FIEL
|
|
"certificates/fiel_20260101.cer",
|
|
"invoices/9/cove/cove.xml",
|
|
"imports/csv/invoice/job-1.csv",
|
|
"branding/logo.png",
|
|
"doda/1/report/doda_report.pdf",
|
|
"signatures/1/photo_x.png",
|
|
],
|
|
)
|
|
def test_no_se_puede_firmar_nada_fuera_de_los_documentos_del_crm(sufijo):
|
|
with pytest.raises(HTTPException) as exc:
|
|
get_upload_url(PREFIJO + sufijo, COMPANY_ID, USUARIO)
|
|
assert exc.value.status_code == 403
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"sufijo",
|
|
[
|
|
"crm-docs/abc123/contrato.pdf",
|
|
"expedientes/1/documents/abc123_guia.pdf",
|
|
],
|
|
)
|
|
def test_los_documentos_del_crm_si_se_pueden_firmar(sufijo):
|
|
resp = get_upload_url(PREFIJO + sufijo, COMPANY_ID, USUARIO)
|
|
assert resp["url"].endswith(sufijo)
|
|
|
|
|
|
def test_no_se_puede_firmar_nada_de_otro_tenant_ni_de_otra_company():
|
|
"""El aislamiento previo sigue en pie: es una guarda adicional, no un reemplazo."""
|
|
for key in (
|
|
"tenants/999/companies/1/crm-docs/a/b.pdf",
|
|
f"tenants/{TENANT_ID}/companies/999/crm-docs/a/b.pdf",
|
|
):
|
|
with pytest.raises(HTTPException) as exc:
|
|
get_upload_url(key, COMPANY_ID, USUARIO)
|
|
assert exc.value.status_code == 403
|
|
|
|
|
|
def test_una_key_que_solo_CONTIENE_el_prefijo_no_pasa():
|
|
"""La comprobación es de prefijo, no de subcadena: ``startswith`` y no ``in``."""
|
|
with pytest.raises(HTTPException) as exc:
|
|
get_upload_url(f"otro/{PREFIJO}crm-docs/a/b.pdf", COMPANY_ID, USUARIO)
|
|
assert exc.value.status_code == 403
|
|
|
|
|
|
def test_la_allowlist_de_extensiones_rechaza_lo_ejecutable():
|
|
for nombre in ("virus.exe", "script.sh", "macro.bat", "lib.dll", "sin_extension"):
|
|
with pytest.raises(HTTPException) as exc:
|
|
validar_extension(nombre)
|
|
assert exc.value.status_code == 422
|
|
assert exc.value.detail == "Ese tipo de archivo no está permitido."
|
|
|
|
|
|
def test_la_allowlist_acepta_los_formatos_de_documento():
|
|
for nombre in ("guia.pdf", "factura.XML", "foto.JPG", "hoja.xlsx", "carta.docx", "paquete.zip"):
|
|
validar_extension(nombre) # no lanza
|
|
|
|
|
|
# ── /uploads/download: el mismo alcance ──────────────────────────────────────
|
|
#
|
|
# El endpoint que entrega los BYTES no puede ser más permisivo que el que solo firma una URL:
|
|
# sería la puerta grande justo al lado de la que se cerró.
|
|
|
|
@pytest.fixture()
|
|
def _sin_descarga(monkeypatch):
|
|
import api.v1.modules.crm.uploads.routes as uploads
|
|
|
|
monkeypatch.setattr(uploads, "get_object_bytes", lambda key: b"contenido")
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"sufijo",
|
|
[
|
|
"certificates/fiel_20260101.key",
|
|
"invoices/9/cove/cove.xml",
|
|
"branding/logo.png",
|
|
],
|
|
)
|
|
def test_la_descarga_tampoco_alcanza_fuera_de_los_documentos(sufijo, _sin_descarga):
|
|
with pytest.raises(HTTPException) as exc:
|
|
download_file(PREFIJO + sufijo, COMPANY_ID, USUARIO)
|
|
assert exc.value.status_code == 403
|
|
|
|
|
|
def test_la_descarga_si_entrega_un_documento_del_crm(_sin_descarga):
|
|
resp = download_file(PREFIJO + "crm-docs/abc123/contrato.pdf", COMPANY_ID, USUARIO)
|
|
assert resp.body == b"contenido"
|
|
|
|
|
|
def test_la_descarga_no_alcanza_otro_tenant(_sin_descarga):
|
|
with pytest.raises(HTTPException) as exc:
|
|
download_file("tenants/999/companies/1/crm-docs/a/b.pdf", COMPANY_ID, USUARIO)
|
|
assert exc.value.status_code == 403
|