fix(security): improve tenant ID resolution and error handling for hub_admin access
This commit is contained in:
@@ -721,7 +721,6 @@ async def get_all_movements(
|
||||
logger.warning(f"Email sending failed: {str(email_error)} - continuing with report generation")
|
||||
|
||||
return movements
|
||||
return movements
|
||||
except ValueError as e:
|
||||
logger.warning(f"Validation error fetching all movements: {str(e)}")
|
||||
raise HTTPException(
|
||||
@@ -761,11 +760,18 @@ def generate_invoice_report_async(
|
||||
# validate_access_to_resource returns the integer tenant_id from DB
|
||||
tenant_id = validate_access_to_resource(db, company_id, current_user, ["report.process"])
|
||||
|
||||
# Guardia explícita: tenant_id debe ser un entero positivo antes del dispatch a Celery.
|
||||
# Un valor inválido aquí generaría un reporte sin filtro de tenant o un crash en la tarea.
|
||||
if not isinstance(tenant_id, int) or tenant_id <= 0:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="No se pudo determinar el tenant para esta empresa. Verifica que la empresa exista.",
|
||||
)
|
||||
|
||||
# Serialize filters to dict for Celery
|
||||
filter_data = filters.model_dump()
|
||||
user_email = current_user.get('email')
|
||||
|
||||
# tenant_id ya validado por validate_access_to_resource
|
||||
task = track_and_dispatch(
|
||||
db=db,
|
||||
task=generate_invoice_movements_async,
|
||||
|
||||
@@ -580,7 +580,7 @@ class UserService:
|
||||
con ``X-Tenant-Override``; activos desde users-with-info del Hub si hay token;
|
||||
inactivos y fallback de conteos en BD local.
|
||||
"""
|
||||
max_users_allowed = 0
|
||||
max_users_allowed: Optional[int] = None # None = sin cuota (hub_admin ilimitado)
|
||||
hub_max_ok = False
|
||||
active_users = 0
|
||||
active_from_hub = False
|
||||
@@ -600,8 +600,8 @@ class UserService:
|
||||
lic_body = lic_resp.json()
|
||||
if lic_body.get("valid"):
|
||||
raw_max = lic_body.get("max_users")
|
||||
# max_users=null → hub_admin sin cuota; usamos 0 como sentinel
|
||||
max_users_allowed = int(raw_max) if raw_max is not None else 0
|
||||
# max_users=null → hub_admin sin cuota; None indica ilimitado
|
||||
max_users_allowed = int(raw_max) if raw_max is not None else None
|
||||
hub_max_ok = True
|
||||
|
||||
users_resp = client.get(
|
||||
@@ -647,9 +647,14 @@ class UserService:
|
||||
)
|
||||
|
||||
total_users = active_users + inactive_users
|
||||
users_available = max(0, max_users_allowed - active_users)
|
||||
# Cuando max_users_allowed es None la cuota es ilimitada (hub_admin)
|
||||
users_available = (
|
||||
max(0, max_users_allowed - active_users)
|
||||
if max_users_allowed is not None
|
||||
else None
|
||||
)
|
||||
usage_percentage = (
|
||||
(active_users / max_users_allowed * 100) if max_users_allowed > 0 else 0
|
||||
(active_users / max_users_allowed * 100) if max_users_allowed else 0.0
|
||||
)
|
||||
|
||||
return {
|
||||
|
||||
@@ -496,14 +496,23 @@ def resolve_tenant_id_required(
|
||||
|
||||
if is_hub_admin(current_user):
|
||||
if db is not None and company_id is not None:
|
||||
from sqlalchemy.exc import SQLAlchemyError as _SAError
|
||||
try:
|
||||
from api.v1.modules.a76.general_catalogs.company.models import Company
|
||||
company = db.query(Company).filter(Company.id == company_id).first()
|
||||
if company and company.tenant_id:
|
||||
return int(company.tenant_id)
|
||||
except Exception:
|
||||
pass
|
||||
# Sin company_id disponible: sentinel None → el servicio no filtra por tenant
|
||||
except _SAError as exc:
|
||||
# Un error de BD no debe escalar silenciosamente a acceso global
|
||||
logger.error(
|
||||
"Error de BD al resolver tenant para hub_admin company_id=%s: %s",
|
||||
company_id,
|
||||
exc,
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=500, detail="Error al resolver el tenant_id"
|
||||
)
|
||||
# Sin company_id disponible → sentinel None para acceso global sin filtro de tenant
|
||||
return None
|
||||
|
||||
raise HTTPException(status_code=400, detail="Tenant ID not found in user data")
|
||||
@@ -661,8 +670,11 @@ def validate_company_access(
|
||||
)
|
||||
|
||||
if not company:
|
||||
print(f"DEBUG: validate_company_access: No se encontró la compañía {company_id} para el tenant {tenant_id}")
|
||||
logger.warning(f"validate_company_access: No se encontró la compañía {company_id} para el tenant {tenant_id}")
|
||||
logger.warning(
|
||||
"validate_company_access: No se encontró la compañía %s para el tenant %s",
|
||||
company_id,
|
||||
tenant_id,
|
||||
)
|
||||
|
||||
return company is not None
|
||||
except Exception as e:
|
||||
@@ -707,18 +719,21 @@ def validate_access_to_resource(
|
||||
|
||||
if not is_keycloak_admin and not is_me_endpoint:
|
||||
if not validate_company_access(db, company_id, current_user):
|
||||
print(f"DEBUG: Acceso denegado a compañía {company_id}")
|
||||
raise HTTPException(status_code=403, detail="Access denied to this company")
|
||||
|
||||
# Si no hay tenant_id, intentamos recuperarlo de la empresa
|
||||
if not tenant_id:
|
||||
from sqlalchemy.exc import SQLAlchemyError as _SAError
|
||||
try:
|
||||
from api.v1.modules.a76.general_catalogs.company.models import Company
|
||||
company = db.query(Company).filter(Company.id == company_id).first()
|
||||
if company:
|
||||
tenant_id = company.tenant_id
|
||||
except:
|
||||
pass
|
||||
except _SAError as exc:
|
||||
logger.error(
|
||||
"Error de BD al resolver tenant company_id=%s: %s", company_id, exc
|
||||
)
|
||||
raise HTTPException(status_code=500, detail="Error al resolver el tenant_id")
|
||||
|
||||
# Si aún no hay tenant_id y no es admin, error 400
|
||||
if not tenant_id and not is_keycloak_admin and not is_me_endpoint:
|
||||
@@ -727,7 +742,14 @@ def validate_access_to_resource(
|
||||
# Verificar permisos locales
|
||||
if required_permissions:
|
||||
if is_keycloak_admin:
|
||||
return tenant_id or 1
|
||||
# hub_admin siempre debe tener tenant_id resuelto cuando se exigen permisos;
|
||||
# retornar 1 silenciosamente sería acceso al tenant equivocado
|
||||
if tenant_id is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="No se pudo resolver el tenant_id para la empresa especificada",
|
||||
)
|
||||
return int(tenant_id)
|
||||
|
||||
from api.v1.modules.core.permissions.service import PermissionService
|
||||
user_id = current_user.get("sub") or current_user.get("id")
|
||||
@@ -752,12 +774,16 @@ def validate_access_to_resource(
|
||||
has_access = permission_service.has_any_permission(user_id, company_id, required_permissions)
|
||||
|
||||
if has_access:
|
||||
print(f"DEBUG: Auto-bootstrap exitoso para {user_id} en empresa {company_id}")
|
||||
logger.info(
|
||||
"Auto-bootstrap exitoso para user_id=%s company_id=%s", user_id, company_id
|
||||
)
|
||||
except Exception as e:
|
||||
print(f"DEBUG: Error en auto-bootstrap de seguridad: {e}")
|
||||
logger.warning(
|
||||
"Error en auto-bootstrap de seguridad user_id=%s company_id=%s: %s",
|
||||
user_id, company_id, e,
|
||||
)
|
||||
|
||||
if not has_access:
|
||||
print(f"DEBUG: Permiso denegado. Faltan: {required_permissions}")
|
||||
raise HTTPException(status_code=403, detail="Permission denied")
|
||||
|
||||
return tenant_id or 1
|
||||
|
||||
Reference in New Issue
Block a user