fix(permissions): comprehensive live permission system tests + delete permission fixes
- Add tests/test_permission_system_live.py: 33 live tests against real PostgreSQL testing RBAC, ABAC, RLS, cross-tenant isolation, guest access, entity sharing, field-level permissions, role invalidation, group permissions, membership suspension - fix(contacts): delete route uses contacts:delete instead of contacts:write The delete_contact and delete_contact_person routes were checking contacts:write permission instead of contacts:delete, allowing users without delete permission to delete contacts. - fix(contacts): DeleteContactCommand passes is_system_admin to service DeleteContactCommand.run() was not passing is_system_admin from the session to contact_service.delete_contact(), causing system admins to be blocked by the row-level admin access check. - fix(contacts): allow deletion of tenant-owned contacts contact_service.delete_contact() required admin-level entity access for ALL contacts, including tenant-owned ones (owner_id=None). Tenant-owned contacts can now be deleted by any user with contacts:delete permission (already verified by the route via require_permission).
This commit is contained in:
@@ -210,7 +210,7 @@ class DeleteContactCommand(BaseCommand):
|
|||||||
hard: If True, perform GDPR hard-delete instead of soft-delete.
|
hard: If True, perform GDPR hard-delete instead of soft-delete.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
permission = "contacts:write"
|
permission = "contacts:delete"
|
||||||
|
|
||||||
def __init__(self, contact_id: str, hard: bool = False) -> None:
|
def __init__(self, contact_id: str, hard: bool = False) -> None:
|
||||||
self.contact_id = contact_id
|
self.contact_id = contact_id
|
||||||
@@ -253,7 +253,8 @@ class DeleteContactCommand(BaseCommand):
|
|||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
await contact_service.delete_contact(
|
await contact_service.delete_contact(
|
||||||
db, tenant_id, self.contact_id, user_id
|
db, tenant_id, self.contact_id, user_id,
|
||||||
|
is_system_admin=current_user.get("is_system_admin", False),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Enqueue outbox event
|
# Enqueue outbox event
|
||||||
|
|||||||
@@ -184,7 +184,7 @@ async def delete_contact(
|
|||||||
hard: bool = Query(False, description="GDPR hard-delete"),
|
hard: bool = Query(False, description="GDPR hard-delete"),
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
redis: aioredis.Redis = Depends(get_redis_dep),
|
redis: aioredis.Redis = Depends(get_redis_dep),
|
||||||
current_user: dict = Depends(require_permission("contacts:write")),
|
current_user: dict = Depends(require_permission("contacts:delete")),
|
||||||
):
|
):
|
||||||
"""Soft-delete (or hard-delete with ?hard=true) a contact via DeleteContactCommand."""
|
"""Soft-delete (or hard-delete with ?hard=true) a contact via DeleteContactCommand."""
|
||||||
cmd = DeleteContactCommand(contact_id=contact_id, hard=hard)
|
cmd = DeleteContactCommand(contact_id=contact_id, hard=hard)
|
||||||
@@ -248,7 +248,7 @@ async def delete_contact_person(
|
|||||||
contact_id: str,
|
contact_id: str,
|
||||||
person_id: str,
|
person_id: str,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: dict = Depends(require_permission("contacts:write")),
|
current_user: dict = Depends(require_permission("contacts:delete")),
|
||||||
):
|
):
|
||||||
"""Delete a contact person."""
|
"""Delete a contact person."""
|
||||||
tenant_id = uuid.UUID(current_user["tenant_id"])
|
tenant_id = uuid.UUID(current_user["tenant_id"])
|
||||||
|
|||||||
@@ -431,7 +431,10 @@ async def delete_contact(
|
|||||||
raise ValueError("Contact not found")
|
raise ValueError("Contact not found")
|
||||||
|
|
||||||
# Check row-level admin access
|
# Check row-level admin access
|
||||||
if not is_system_admin:
|
# Tenant-owned contacts (owner_id=None) can be deleted by any user with
|
||||||
|
# contacts:delete permission (already checked by route via require_permission).
|
||||||
|
# User-owned contacts require admin-level entity access.
|
||||||
|
if not is_system_admin and contact.owner_id is not None:
|
||||||
from app.core.visibility import check_single_entity_access
|
from app.core.visibility import check_single_entity_access
|
||||||
has_access = await check_single_entity_access(
|
has_access = await check_single_entity_access(
|
||||||
db, "contact", contact.id, user_id, tenant_id, "admin", is_system_admin
|
db, "contact", contact.id, user_id, tenant_id, "admin", is_system_admin
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user