feat(security): implement permission checks in tenant CRUD routes and enhance API error handling
This commit is contained in:
@@ -15,8 +15,7 @@ ServiceType = TypeVar("ServiceType")
|
||||
|
||||
|
||||
class TenantCRUDRoutes(
|
||||
Generic[CreateSchemaType, UpdateSchemaType,
|
||||
ResponseSchemaType, ServiceType]
|
||||
Generic[CreateSchemaType, UpdateSchemaType, ResponseSchemaType, ServiceType]
|
||||
):
|
||||
"""
|
||||
Generic CRUD routes factory for tenant-scoped resources
|
||||
@@ -88,6 +87,13 @@ class TenantCRUDRoutes(
|
||||
enable_filters: bool = False, # Enable custom filters in list endpoint
|
||||
default_page_size: int = 50,
|
||||
max_page_size: int = 100,
|
||||
# Permissions for each operation
|
||||
list_permissions: Optional[list[str]] = None,
|
||||
get_permissions: Optional[list[str]] = None,
|
||||
create_permissions: Optional[list[str]] = None,
|
||||
update_permissions: Optional[list[str]] = None,
|
||||
delete_permissions: Optional[list[str]] = None,
|
||||
require_all: bool = True, # If True, requires ALL permissions; if False, requires ANY
|
||||
):
|
||||
self.service = service
|
||||
self.create_schema = create_schema
|
||||
@@ -104,6 +110,12 @@ class TenantCRUDRoutes(
|
||||
self.enable_filters = enable_filters
|
||||
self.default_page_size = default_page_size
|
||||
self.max_page_size = max_page_size
|
||||
self.list_permissions = list_permissions
|
||||
self.get_permissions = get_permissions
|
||||
self.create_permissions = create_permissions
|
||||
self.update_permissions = update_permissions
|
||||
self.delete_permissions = delete_permissions
|
||||
self.require_all = require_all
|
||||
|
||||
self.router = APIRouter(prefix=prefix, tags=tags)
|
||||
self._register_routes()
|
||||
@@ -130,18 +142,22 @@ class TenantCRUDRoutes(
|
||||
le=self.max_page_size,
|
||||
description="Page size",
|
||||
),
|
||||
status: Optional[str] = Query(
|
||||
None, description="Filter by status"),
|
||||
status: Optional[str] = Query(None, description="Filter by status"),
|
||||
operation_type: Optional[str] = Query(
|
||||
None, description="Filter by operation type"),
|
||||
None, description="Filter by operation type"
|
||||
),
|
||||
invoice_type: Optional[str] = Query(
|
||||
None, description="Filter by invoice type"),
|
||||
None, description="Filter by invoice type"
|
||||
),
|
||||
db: Session = Depends(self.db_dependency),
|
||||
current_user: Dict[str, Any] = Depends(
|
||||
self.auth_dependency),
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.list_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
|
||||
skip = (page - 1) * page_size
|
||||
@@ -184,11 +200,14 @@ class TenantCRUDRoutes(
|
||||
description="Page size",
|
||||
),
|
||||
db: Session = Depends(self.db_dependency),
|
||||
current_user: Dict[str, Any] = Depends(
|
||||
self.auth_dependency),
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.list_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
|
||||
skip = (page - 1) * page_size
|
||||
@@ -225,7 +244,8 @@ class TenantCRUDRoutes(
|
||||
):
|
||||
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db, company_id, current_user, self.get_permissions, self.require_all
|
||||
)
|
||||
parent_id = path_params.get(self.parent_id_name)
|
||||
|
||||
# Try method with 4 params (pedimento_id, tenant_id, company_id)
|
||||
@@ -239,8 +259,7 @@ class TenantCRUDRoutes(
|
||||
db, parent_id, tenant_id, company_id
|
||||
)
|
||||
else:
|
||||
resource = self.service.get(
|
||||
db, parent_id, tenant_id, company_id)
|
||||
resource = self.service.get(db, parent_id, tenant_id, company_id)
|
||||
|
||||
if not resource:
|
||||
raise HTTPException(
|
||||
@@ -265,7 +284,8 @@ class TenantCRUDRoutes(
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db, company_id, current_user, self.get_permissions, self.require_all
|
||||
)
|
||||
|
||||
resource = self.service.get_by_id(
|
||||
db, resource_id, tenant_id, company_id
|
||||
@@ -298,7 +318,12 @@ class TenantCRUDRoutes(
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.create_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
|
||||
# For child resources, parent_id validation would go here
|
||||
try:
|
||||
@@ -310,6 +335,7 @@ class TenantCRUDRoutes(
|
||||
except Exception as e:
|
||||
# Re-lanzar otros errores
|
||||
raise
|
||||
|
||||
else:
|
||||
# Parent resource - no parent_id needed
|
||||
|
||||
@@ -330,7 +356,12 @@ class TenantCRUDRoutes(
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.create_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
try:
|
||||
resource = self.service.create(db, data, tenant_id, company_id)
|
||||
return resource
|
||||
@@ -364,7 +395,12 @@ class TenantCRUDRoutes(
|
||||
**path_params,
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.update_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
parent_id = path_params.get(self.parent_id_name)
|
||||
|
||||
try:
|
||||
@@ -404,7 +440,12 @@ class TenantCRUDRoutes(
|
||||
):
|
||||
f"""Update {self.resource_name}"""
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.update_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
|
||||
try:
|
||||
resource = self.service.update(
|
||||
@@ -438,11 +479,15 @@ class TenantCRUDRoutes(
|
||||
**path_params,
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.delete_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
parent_id = path_params.get(self.parent_id_name)
|
||||
|
||||
success = self.service.delete(
|
||||
db, parent_id, tenant_id, company_id)
|
||||
success = self.service.delete(db, parent_id, tenant_id, company_id)
|
||||
|
||||
if not success:
|
||||
raise HTTPException(
|
||||
@@ -467,10 +512,14 @@ class TenantCRUDRoutes(
|
||||
current_user: Dict[str, Any] = Depends(self.auth_dependency),
|
||||
):
|
||||
tenant_id = validate_access_to_resource(
|
||||
db, company_id, current_user)
|
||||
db,
|
||||
company_id,
|
||||
current_user,
|
||||
self.delete_permissions,
|
||||
self.require_all,
|
||||
)
|
||||
|
||||
success = self.service.delete(
|
||||
db, resource_id, tenant_id, company_id)
|
||||
success = self.service.delete(db, resource_id, tenant_id, company_id)
|
||||
|
||||
if not success:
|
||||
raise HTTPException(
|
||||
|
||||
@@ -20,9 +20,14 @@ invoice_crud = TenantCRUDRoutes(
|
||||
tags=[],
|
||||
resource_name="Invoice",
|
||||
id_name="invoice_id",
|
||||
id_type=int,
|
||||
id_type=int,
|
||||
enable_list=True, # Enable list endpoint with pagination
|
||||
enable_filters=True, # Enable filters for status, operation_type, etc.
|
||||
list_permissions=[],
|
||||
get_permissions=[],
|
||||
create_permissions=[],
|
||||
update_permissions=[],
|
||||
delete_permissions=[],
|
||||
default_page_size=50,
|
||||
max_page_size=200,
|
||||
)
|
||||
|
||||
@@ -11,34 +11,6 @@ from core.database import get_core_db
|
||||
from core.security import get_current_user # Asumiendo que existe esta función
|
||||
from .service import PermissionService
|
||||
|
||||
|
||||
# Dependencia para obtener el ID del cliente del header o contexto
|
||||
async def get_client_id(
|
||||
x_client_id: Optional[str] = Header(None, alias="X-Client-ID")
|
||||
) -> int:
|
||||
"""
|
||||
Obtiene el ID del cliente desde el header de la petición.
|
||||
|
||||
En producción, esto podría obtenerse de:
|
||||
- Un header HTTP (X-Client-ID)
|
||||
- Un subdomain (cliente1.miapp.com)
|
||||
- El token JWT del usuario
|
||||
- La sesión del usuario
|
||||
"""
|
||||
if not x_client_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Client ID is required. Provide X-Client-ID header.",
|
||||
)
|
||||
|
||||
try:
|
||||
return int(x_client_id)
|
||||
except ValueError:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid client ID format"
|
||||
)
|
||||
|
||||
|
||||
# Dependencia para obtener el servicio de permisos
|
||||
def get_permission_service(db: Session = Depends(get_core_db)) -> PermissionService:
|
||||
"""
|
||||
@@ -72,9 +44,9 @@ class PermissionChecker:
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
client_id: int = Depends(get_client_id),
|
||||
permission_service: PermissionService = Depends(get_permission_service),
|
||||
client_id: int,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
permission_service: PermissionService = Depends(get_permission_service),
|
||||
):
|
||||
"""
|
||||
Verifica que el usuario tenga los permisos requeridos.
|
||||
@@ -130,8 +102,8 @@ class RequirePermission:
|
||||
|
||||
async def __call__(
|
||||
self,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
client_id: int = Depends(get_client_id),
|
||||
client_id: int,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
permission_service: PermissionService = Depends(get_permission_service),
|
||||
):
|
||||
user_id = current_user.get("sub") or current_user.get("id")
|
||||
@@ -212,8 +184,8 @@ def require_permissions(*permissions: str, require_all: bool = True):
|
||||
|
||||
# Función helper para obtener permisos del usuario actual
|
||||
async def get_current_user_permissions(
|
||||
current_user: dict = Depends(get_current_user),
|
||||
client_id: int = Depends(get_client_id),
|
||||
client_id: int,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
permission_service: PermissionService = Depends(get_permission_service),
|
||||
) -> set:
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user