Phase 1: Critical security fixes - 59 permissions, grants, RLS, mass-assignment, ownership, leaks, MIME
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
This commit is contained in:
@@ -86,6 +86,60 @@ async def create_entity_permission(
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
|
||||
|
||||
async def _check_entity_ownership(
|
||||
db: AsyncSession,
|
||||
tenant_id: uuid.UUID,
|
||||
user_id: uuid.UUID,
|
||||
entity_type: str,
|
||||
entity_id: str,
|
||||
is_system_admin: bool,
|
||||
) -> None:
|
||||
"""Verify that the current user owns the entity or is system admin.
|
||||
|
||||
Raises 403 if the user is neither owner nor system admin.
|
||||
"""
|
||||
if is_system_admin:
|
||||
return
|
||||
|
||||
from app.models.entity_permission import ENTITY_MODELS
|
||||
from sqlalchemy import select
|
||||
|
||||
model_info = ENTITY_MODELS.get(entity_type)
|
||||
if model_info is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"detail": f"Unknown entity type: {entity_type}", "code": "not_found"},
|
||||
)
|
||||
|
||||
model = model_info["model"]
|
||||
try:
|
||||
eid = uuid.UUID(entity_id)
|
||||
except ValueError:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail={"detail": "Invalid entity_id", "code": "invalid_id"},
|
||||
) from None
|
||||
|
||||
result = await db.execute(
|
||||
select(model.owner_id).where(
|
||||
model.id == eid,
|
||||
model.tenant_id == tenant_id,
|
||||
)
|
||||
)
|
||||
row = result.first()
|
||||
if row is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"detail": "Entity not found", "code": "not_found"},
|
||||
)
|
||||
owner_id = row[0]
|
||||
if owner_id is not None and owner_id != user_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"detail": "Only the entity owner or system admin can modify permissions", "code": "forbidden"},
|
||||
)
|
||||
|
||||
|
||||
@router.put("/{entity_type}/{entity_id}/{permission_id}")
|
||||
async def update_entity_permission(
|
||||
entity_type: str,
|
||||
@@ -103,6 +157,11 @@ async def update_entity_permission(
|
||||
_PERM_RATE_LIMIT_MAX,
|
||||
_PERM_RATE_LIMIT_WINDOW,
|
||||
)
|
||||
# Ownership check: only owner or system admin can update permissions
|
||||
await _check_entity_ownership(
|
||||
db, tenant_id, user_id, entity_type, entity_id,
|
||||
current_user.get("is_system_admin", False),
|
||||
)
|
||||
try:
|
||||
return await entity_permission_service.update_permission(
|
||||
db,
|
||||
|
||||
@@ -20,11 +20,24 @@ async def get_system_settings(
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: dict = Depends(require_permission("settings:read")),
|
||||
):
|
||||
"""Get system settings for the current tenant."""
|
||||
"""Get system settings for the current tenant.
|
||||
|
||||
Sensitive fields (tax_number, iban, bic) are masked for non-admin users.
|
||||
"""
|
||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||
result = await system_settings_service.get_system_settings(db, tenant_id)
|
||||
if result is None:
|
||||
return SystemSettingsResponse()
|
||||
|
||||
# Mask sensitive fields for non-admin users
|
||||
if not current_user.get("is_system_admin"):
|
||||
if hasattr(result, "tax_number") and result.tax_number:
|
||||
result.tax_number = "********"
|
||||
if hasattr(result, "iban") and result.iban:
|
||||
result.iban = "********"
|
||||
if hasattr(result, "bic") and result.bic:
|
||||
result.bic = "********"
|
||||
|
||||
return result
|
||||
|
||||
|
||||
|
||||
+16
-1
@@ -63,6 +63,14 @@ async def create_user(
|
||||
user_id = uuid.UUID(current_user["user_id"])
|
||||
role_id = _parse_role_id(body.role_id)
|
||||
|
||||
# Mass-Assignment protection: only system admin can create admin users
|
||||
role = body.role
|
||||
if role == "admin" and not current_user.get("is_system_admin"):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"detail": "Only system admin can create admin users", "code": "role_escalation_forbidden"},
|
||||
)
|
||||
|
||||
try:
|
||||
user = await user_service.create_user(
|
||||
db,
|
||||
@@ -70,7 +78,7 @@ async def create_user(
|
||||
body.email,
|
||||
body.name,
|
||||
body.password,
|
||||
body.role,
|
||||
role,
|
||||
role_id,
|
||||
body.is_active,
|
||||
)
|
||||
@@ -190,6 +198,13 @@ async def update_user(
|
||||
detail={"detail": "Cannot modify your own role or active status", "code": "self_modification_forbidden"},
|
||||
)
|
||||
|
||||
# Mass-Assignment protection: only system admin can change roles to admin
|
||||
if body.role == "admin" and not current_user.get("is_system_admin"):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"detail": "Only system admin can assign admin role", "code": "role_escalation_forbidden"},
|
||||
)
|
||||
|
||||
# Determine if role_id was explicitly sent (Pydantic v2)
|
||||
role_id_sent = "role_id" in body.model_fields_set
|
||||
|
||||
|
||||
Reference in New Issue
Block a user