feat(permissions): ABAC integration, principals caching, cache version validation
- Integrate ABAC policies into apply_visibility_filter() (allow/deny with priority) - Add field whitelist (ABAC_ALLOWED_FIELDS) for build_sql_condition() security - Add request-level ContextVar for user principals (group_ids, role_id) - Set principals in deps.py (session + bearer auth) - Use ContextVar in visibility.py and permission_resolver.py (N+1 fix) - Add version validation to get_cached_visible_ids() (cache strategy unification) - Deactivate delegation route (parked — not integrated into resolve_permissions) - Add 7 ABAC integration tests All 70 tests pass (7 ABAC + 63 existing). No regressions.
This commit is contained in:
@@ -0,0 +1,42 @@
|
|||||||
|
"""Request-level caching for user principals (group_ids + role_id).
|
||||||
|
|
||||||
|
Avoids N+1 queries by loading principals once per request in deps.py
|
||||||
|
and reusing them in visibility.py and permission_resolver.py.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
from contextvars import ContextVar
|
||||||
|
from dataclasses import dataclass
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class UserPrincipals:
|
||||||
|
"""Cached user principals for the current request."""
|
||||||
|
user_id: uuid.UUID
|
||||||
|
tenant_id: uuid.UUID
|
||||||
|
group_ids: list[uuid.UUID]
|
||||||
|
role_id: uuid.UUID | None
|
||||||
|
|
||||||
|
|
||||||
|
# ContextVar — async-safe, isolated per request
|
||||||
|
_principals_ctx: ContextVar[UserPrincipals | None] = ContextVar(
|
||||||
|
"user_principals", default=None
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def set_principals(principals: UserPrincipals) -> None:
|
||||||
|
"""Set principals for the current request context."""
|
||||||
|
_principals_ctx.set(principals)
|
||||||
|
|
||||||
|
|
||||||
|
def get_principals() -> UserPrincipals | None:
|
||||||
|
"""Get principals for the current request context, or None if not set."""
|
||||||
|
return _principals_ctx.get()
|
||||||
|
|
||||||
|
|
||||||
|
def clear_principals() -> None:
|
||||||
|
"""Clear principals at end of request (optional — ContextVar is
|
||||||
|
isolated per task, but explicit cleanup is good practice)."""
|
||||||
|
_principals_ctx.set(None)
|
||||||
+78
-2
@@ -29,7 +29,7 @@ import logging
|
|||||||
import uuid
|
import uuid
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from sqlalchemy import and_, exists, or_, select, text
|
from sqlalchemy import and_, exists, not_, or_, select, text
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
from sqlalchemy.orm import DeclarativeBase
|
from sqlalchemy.orm import DeclarativeBase
|
||||||
|
|
||||||
@@ -51,7 +51,17 @@ async def _get_user_principals(
|
|||||||
user_id: uuid.UUID,
|
user_id: uuid.UUID,
|
||||||
tenant_id: uuid.UUID,
|
tenant_id: uuid.UUID,
|
||||||
) -> tuple[list[uuid.UUID], uuid.UUID | None]:
|
) -> tuple[list[uuid.UUID], uuid.UUID | None]:
|
||||||
"""Get user's group IDs and role ID for permission resolution."""
|
"""Get user's group IDs and role ID for permission resolution.
|
||||||
|
|
||||||
|
Uses request-level ContextVar cache when available (set in deps.py).
|
||||||
|
Falls back to DB query when ContextVar is not set (e.g. worker, tests).
|
||||||
|
"""
|
||||||
|
from app.core.principals import get_principals
|
||||||
|
cached = get_principals()
|
||||||
|
if cached is not None and cached.user_id == user_id and cached.tenant_id == tenant_id:
|
||||||
|
return cached.group_ids, cached.role_id
|
||||||
|
|
||||||
|
# Fallback: load from DB (worker, tests, non-request context)
|
||||||
groups_q = await db.execute(
|
groups_q = await db.execute(
|
||||||
select(UserGroup.group_id)
|
select(UserGroup.group_id)
|
||||||
.where(UserGroup.user_id == user_id)
|
.where(UserGroup.user_id == user_id)
|
||||||
@@ -157,6 +167,72 @@ async def apply_visibility_filter(
|
|||||||
shared_exists, # Shared via entity_permissions
|
shared_exists, # Shared via entity_permissions
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# ─── ABAC Policy Layer ──────────────────────────────────────────
|
||||||
|
# Load enabled entity policies for this entity_type + tenant and
|
||||||
|
# apply them as an additional visibility layer:
|
||||||
|
# (owner OR tenant-owned OR shared OR allow-matched) AND NOT (deny-matched)
|
||||||
|
from app.models.entity_policy import EntityPolicy
|
||||||
|
from app.services.policy_service import build_sql_condition
|
||||||
|
|
||||||
|
policy_stmt = (
|
||||||
|
select(EntityPolicy)
|
||||||
|
.where(EntityPolicy.tenant_id == tenant_id)
|
||||||
|
.where(EntityPolicy.entity_type == entity_type)
|
||||||
|
.where(EntityPolicy.enabled == True) # noqa: E712
|
||||||
|
.order_by(EntityPolicy.priority.desc())
|
||||||
|
)
|
||||||
|
policy_result = await db.execute(policy_stmt)
|
||||||
|
all_policies = policy_result.scalars().all()
|
||||||
|
|
||||||
|
# Filter policies applicable to this user (user / group / role principals)
|
||||||
|
applicable_policies: list[EntityPolicy] = []
|
||||||
|
for policy in all_policies:
|
||||||
|
if policy.principal_type == "user" and policy.principal_id == user_id:
|
||||||
|
applicable_policies.append(policy)
|
||||||
|
elif policy.principal_type == "group" and policy.principal_id in group_ids:
|
||||||
|
applicable_policies.append(policy)
|
||||||
|
elif (
|
||||||
|
policy.principal_type == "role"
|
||||||
|
and role_id is not None
|
||||||
|
and policy.principal_id == role_id
|
||||||
|
):
|
||||||
|
applicable_policies.append(policy)
|
||||||
|
|
||||||
|
if applicable_policies:
|
||||||
|
allow_clauses: list[Any] = []
|
||||||
|
deny_clauses: list[Any] = []
|
||||||
|
|
||||||
|
for policy in applicable_policies:
|
||||||
|
if not policy.conditions:
|
||||||
|
# Policy without conditions matches all rows
|
||||||
|
if policy.effect == "allow":
|
||||||
|
allow_clauses.append(text("1 = 1"))
|
||||||
|
else:
|
||||||
|
deny_clauses.append(text("1 = 1"))
|
||||||
|
continue
|
||||||
|
|
||||||
|
condition = build_sql_condition(
|
||||||
|
policy.conditions, model, entity_type=entity_type
|
||||||
|
)
|
||||||
|
if condition is None:
|
||||||
|
continue
|
||||||
|
|
||||||
|
if policy.effect == "allow":
|
||||||
|
allow_clauses.append(condition)
|
||||||
|
else:
|
||||||
|
deny_clauses.append(condition)
|
||||||
|
|
||||||
|
# allow: OR-join — expands visibility beyond owner/shared
|
||||||
|
if allow_clauses:
|
||||||
|
visibility_condition = or_(visibility_condition, *allow_clauses)
|
||||||
|
|
||||||
|
# deny: NOT — excludes matching rows (deny takes precedence)
|
||||||
|
if deny_clauses:
|
||||||
|
visibility_condition = and_(
|
||||||
|
visibility_condition,
|
||||||
|
not_(or_(*deny_clauses)),
|
||||||
|
)
|
||||||
|
|
||||||
return query.where(visibility_condition)
|
return query.where(visibility_condition)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+31
-3
@@ -96,20 +96,31 @@ async def get_current_user(
|
|||||||
is_admin = session_data.get("is_system_admin", False)
|
is_admin = session_data.get("is_system_admin", False)
|
||||||
await set_user_context(db, user_id, group_ids, is_admin)
|
await set_user_context(db, user_id, group_ids, is_admin)
|
||||||
|
|
||||||
# Check membership status (P1.7: suspended membership should not be usable)
|
# Check membership status and load role_id (P1.7: suspended membership should not be usable)
|
||||||
from app.models.user import UserTenant
|
from app.models.user import UserTenant
|
||||||
membership_q = await db.execute(
|
membership_q = await db.execute(
|
||||||
select(UserTenant.status)
|
select(UserTenant.status, UserTenant.role_id)
|
||||||
.where(UserTenant.user_id == user_id)
|
.where(UserTenant.user_id == user_id)
|
||||||
.where(UserTenant.tenant_id == tenant_id)
|
.where(UserTenant.tenant_id == tenant_id)
|
||||||
)
|
)
|
||||||
membership_status = membership_q.scalar_one_or_none()
|
membership_row = membership_q.first()
|
||||||
|
membership_status = membership_row[0] if membership_row else None
|
||||||
|
role_id = membership_row[1] if membership_row else None
|
||||||
if membership_status is not None and membership_status != "active":
|
if membership_status is not None and membership_status != "active":
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
detail={"detail": f"Mitgliedschaft ist {membership_status}, Zugriff verweigert", "code": "membership_suspended"},
|
detail={"detail": f"Mitgliedschaft ist {membership_status}, Zugriff verweigert", "code": "membership_suspended"},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Cache user principals for this request — avoids N+1 queries in visibility.py
|
||||||
|
from app.core.principals import UserPrincipals, set_principals
|
||||||
|
set_principals(UserPrincipals(
|
||||||
|
user_id=user_id,
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
group_ids=group_ids,
|
||||||
|
role_id=role_id,
|
||||||
|
))
|
||||||
|
|
||||||
# Load resolved permissions from cache (or DB on miss)
|
# Load resolved permissions from cache (or DB on miss)
|
||||||
from app.core.permissions import get_cached_permissions
|
from app.core.permissions import get_cached_permissions
|
||||||
|
|
||||||
@@ -165,6 +176,23 @@ async def get_current_user_bearer(
|
|||||||
is_admin = user_data.get("is_system_admin", False)
|
is_admin = user_data.get("is_system_admin", False)
|
||||||
await set_user_context(db, user_id, group_ids, is_admin)
|
await set_user_context(db, user_id, group_ids, is_admin)
|
||||||
|
|
||||||
|
# Load role_id and cache user principals for this request
|
||||||
|
from app.models.user import UserTenant
|
||||||
|
membership_q = await db.execute(
|
||||||
|
select(UserTenant.role_id)
|
||||||
|
.where(UserTenant.user_id == user_id)
|
||||||
|
.where(UserTenant.tenant_id == tenant_id)
|
||||||
|
)
|
||||||
|
role_id = membership_q.scalar_one_or_none()
|
||||||
|
|
||||||
|
from app.core.principals import UserPrincipals, set_principals
|
||||||
|
set_principals(UserPrincipals(
|
||||||
|
user_id=user_id,
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
group_ids=group_ids,
|
||||||
|
role_id=role_id,
|
||||||
|
))
|
||||||
|
|
||||||
# Load resolved permissions
|
# Load resolved permissions
|
||||||
from app.core.permissions import get_cached_permissions
|
from app.core.permissions import get_cached_permissions
|
||||||
redis = get_redis()
|
redis = get_redis()
|
||||||
|
|||||||
+2
-2
@@ -66,7 +66,7 @@ from app.routes import (
|
|||||||
backups,
|
backups,
|
||||||
owner_transfer,
|
owner_transfer,
|
||||||
permission_templates,
|
permission_templates,
|
||||||
delegations,
|
# delegations, # ⏸ Parked — not integrated into resolve_permissions()
|
||||||
policies,
|
policies,
|
||||||
guests,
|
guests,
|
||||||
outbox,
|
outbox,
|
||||||
@@ -444,7 +444,7 @@ def create_app() -> FastAPI:
|
|||||||
app.include_router(saved_views.router)
|
app.include_router(saved_views.router)
|
||||||
app.include_router(webhooks.router)
|
app.include_router(webhooks.router)
|
||||||
app.include_router(permission_templates.router)
|
app.include_router(permission_templates.router)
|
||||||
app.include_router(delegations.router)
|
# app.include_router(delegations.router) # ⏸ Parked — not integrated into resolve_permissions()
|
||||||
app.include_router(policies.router)
|
app.include_router(policies.router)
|
||||||
app.include_router(errors.router)
|
app.include_router(errors.router)
|
||||||
app.include_router(guests.router) # ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest
|
app.include_router(guests.router) # ⚠️ Guest-System umgebaut — Guests sind jetzt reguläre User mit role=guest
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ Extracted from entity_permission_service.py for modularity.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import logging
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
import redis.asyncio as aioredis
|
import redis.asyncio as aioredis
|
||||||
@@ -13,6 +14,8 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
|||||||
|
|
||||||
from app.services.permission_resolver import get_visible_ids
|
from app.services.permission_resolver import get_visible_ids
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
CACHE_TTL = 300 # 5 minutes
|
CACHE_TTL = 300 # 5 minutes
|
||||||
CACHE_PREFIX = "ep_vis" # entity permission visibility
|
CACHE_PREFIX = "ep_vis" # entity permission visibility
|
||||||
|
|
||||||
@@ -40,25 +43,61 @@ async def get_cached_visible_ids(
|
|||||||
"""Get visible IDs from Redis cache or resolve from DB.
|
"""Get visible IDs from Redis cache or resolve from DB.
|
||||||
|
|
||||||
Cache key: ep_vis:{user_id}:{tenant_id}:{entity_type}
|
Cache key: ep_vis:{user_id}:{tenant_id}:{entity_type}
|
||||||
Cache value: JSON {visible_ids: [str], access_map: {str: str}}
|
Cache value: JSON {visible_ids: [str], access_map: {str: str}, version: int}
|
||||||
TTL: 5 minutes
|
TTL: 5 minutes
|
||||||
|
|
||||||
|
Validates cached permission_version against current DB version.
|
||||||
|
If they differ, the cache entry is stale and will be re-resolved.
|
||||||
"""
|
"""
|
||||||
cache_key = f"{CACHE_PREFIX}:{user_id}:{tenant_id}:{entity_type}"
|
cache_key = f"{CACHE_PREFIX}:{user_id}:{tenant_id}:{entity_type}"
|
||||||
|
|
||||||
raw = await redis.get(cache_key)
|
raw = await redis.get(cache_key)
|
||||||
if raw is not None:
|
if raw is not None:
|
||||||
data = json.loads(raw)
|
data = json.loads(raw)
|
||||||
visible = {uuid.UUID(eid) for eid in data.get("visible_ids", [])}
|
cached_version = data.get("version", -1)
|
||||||
access_map = {uuid.UUID(eid): level for eid, level in data.get("access_map", {}).items()}
|
|
||||||
return visible, access_map
|
|
||||||
|
|
||||||
# Cache miss — resolve from DB
|
# Validate cached version against current DB version
|
||||||
|
try:
|
||||||
|
from app.core.permissions import _get_current_permission_version
|
||||||
|
current_version = await _get_current_permission_version(db, user_id, tenant_id)
|
||||||
|
except Exception:
|
||||||
|
logger.warning(
|
||||||
|
"Failed to query permission_version for visible_ids cache validation "
|
||||||
|
"(user=%s, tenant=%s) — invalidating cache and re-resolving",
|
||||||
|
user_id, tenant_id,
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
await redis.delete(cache_key)
|
||||||
|
# Fall through to re-resolution below
|
||||||
|
else:
|
||||||
|
if cached_version == current_version:
|
||||||
|
visible = {uuid.UUID(eid) for eid in data.get("visible_ids", [])}
|
||||||
|
access_map = {uuid.UUID(eid): level for eid, level in data.get("access_map", {}).items()}
|
||||||
|
return visible, access_map
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
"visible_ids cache version mismatch for user=%s tenant=%s entity_type=%s "
|
||||||
|
"(cached=%s, current=%s) — re-resolving",
|
||||||
|
user_id, tenant_id, entity_type, cached_version, current_version,
|
||||||
|
)
|
||||||
|
await redis.delete(cache_key)
|
||||||
|
|
||||||
|
# Cache miss, stale, or version mismatch — resolve from DB
|
||||||
visible, access_map = await get_visible_ids(db, tenant_id, user_id, entity_type)
|
visible, access_map = await get_visible_ids(db, tenant_id, user_id, entity_type)
|
||||||
|
|
||||||
|
# Get current permission version for cache stamping
|
||||||
|
try:
|
||||||
|
from app.core.permissions import _get_current_permission_version
|
||||||
|
current_version = await _get_current_permission_version(db, user_id, tenant_id)
|
||||||
|
except Exception:
|
||||||
|
logger.warning("Failed to get permission_version for cache stamping — using 0")
|
||||||
|
current_version = 0
|
||||||
|
|
||||||
# Store in cache
|
# Store in cache
|
||||||
cache_data = {
|
cache_data = {
|
||||||
"visible_ids": [str(eid) for eid in visible],
|
"visible_ids": [str(eid) for eid in visible],
|
||||||
"access_map": {str(eid): level for eid, level in access_map.items()},
|
"access_map": {str(eid): level for eid, level in access_map.items()},
|
||||||
|
"version": current_version,
|
||||||
}
|
}
|
||||||
await redis.setex(cache_key, CACHE_TTL, json.dumps(cache_data))
|
await redis.setex(cache_key, CACHE_TTL, json.dumps(cache_data))
|
||||||
|
|
||||||
|
|||||||
@@ -74,20 +74,27 @@ async def get_effective_access(
|
|||||||
if owner_id == user_id:
|
if owner_id == user_id:
|
||||||
return "owner"
|
return "owner"
|
||||||
|
|
||||||
# Get user's groups and role
|
# Get user's groups and role — use ContextVar cache when available
|
||||||
groups_q = await db.execute(
|
from app.core.principals import get_principals
|
||||||
select(UserGroup.group_id)
|
cached = get_principals()
|
||||||
.where(UserGroup.user_id == user_id)
|
if cached is not None and cached.user_id == user_id and cached.tenant_id == tenant_id:
|
||||||
.where(UserGroup.tenant_id == tenant_id)
|
group_ids = cached.group_ids
|
||||||
)
|
role_id = cached.role_id
|
||||||
group_ids = [row[0] for row in groups_q]
|
else:
|
||||||
|
# Fallback: load from DB (worker, tests, non-request context)
|
||||||
|
groups_q = await db.execute(
|
||||||
|
select(UserGroup.group_id)
|
||||||
|
.where(UserGroup.user_id == user_id)
|
||||||
|
.where(UserGroup.tenant_id == tenant_id)
|
||||||
|
)
|
||||||
|
group_ids = [row[0] for row in groups_q]
|
||||||
|
|
||||||
role_q = await db.execute(
|
role_q = await db.execute(
|
||||||
select(UserTenant.role_id)
|
select(UserTenant.role_id)
|
||||||
.where(UserTenant.user_id == user_id)
|
.where(UserTenant.user_id == user_id)
|
||||||
.where(UserTenant.tenant_id == tenant_id)
|
.where(UserTenant.tenant_id == tenant_id)
|
||||||
)
|
)
|
||||||
role_id = role_q.scalar_one_or_none()
|
role_id = role_q.scalar_one_or_none()
|
||||||
|
|
||||||
# Build principal conditions
|
# Build principal conditions
|
||||||
principal_conditions = [
|
principal_conditions = [
|
||||||
@@ -178,20 +185,27 @@ async def get_visible_ids(
|
|||||||
all_ids = {row[0] for row in all_q}
|
all_ids = {row[0] for row in all_q}
|
||||||
return all_ids, {eid: "delete" for eid in all_ids}
|
return all_ids, {eid: "delete" for eid in all_ids}
|
||||||
|
|
||||||
# Get user's groups and role
|
# Get user's groups and role — use ContextVar cache when available
|
||||||
groups_q = await db.execute(
|
from app.core.principals import get_principals
|
||||||
select(UserGroup.group_id)
|
cached = get_principals()
|
||||||
.where(UserGroup.user_id == user_id)
|
if cached is not None and cached.user_id == user_id and cached.tenant_id == tenant_id:
|
||||||
.where(UserGroup.tenant_id == tenant_id)
|
group_ids = cached.group_ids
|
||||||
)
|
role_id = cached.role_id
|
||||||
group_ids = [row[0] for row in groups_q]
|
else:
|
||||||
|
# Fallback: load from DB (worker, tests, non-request context)
|
||||||
|
groups_q = await db.execute(
|
||||||
|
select(UserGroup.group_id)
|
||||||
|
.where(UserGroup.user_id == user_id)
|
||||||
|
.where(UserGroup.tenant_id == tenant_id)
|
||||||
|
)
|
||||||
|
group_ids = [row[0] for row in groups_q]
|
||||||
|
|
||||||
role_q = await db.execute(
|
role_q = await db.execute(
|
||||||
select(UserTenant.role_id)
|
select(UserTenant.role_id)
|
||||||
.where(UserTenant.user_id == user_id)
|
.where(UserTenant.user_id == user_id)
|
||||||
.where(UserTenant.tenant_id == tenant_id)
|
.where(UserTenant.tenant_id == tenant_id)
|
||||||
)
|
)
|
||||||
role_id = role_q.scalar_one_or_none()
|
role_id = role_q.scalar_one_or_none()
|
||||||
|
|
||||||
# 1. Owned entities
|
# 1. Owned entities
|
||||||
model = _get_entity_model(entity_type)
|
model = _get_entity_model(entity_type)
|
||||||
|
|||||||
@@ -30,6 +30,21 @@ _SUPPORTED_OPS = {
|
|||||||
"contains", "starts_with", "is_null", "is_not_null",
|
"contains", "starts_with", "is_null", "is_not_null",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# ─── ABAC Field Whitelist ──────────────────────────────────────────
|
||||||
|
# Only these fields may be referenced in policy conditions per entity
|
||||||
|
# type. Prevents policies from filtering on sensitive columns such as
|
||||||
|
# tenant_id, password_hash, etc.
|
||||||
|
ABAC_ALLOWED_FIELDS: dict[str, set[str]] = {
|
||||||
|
"contact": {"status", "type", "country", "tags", "created_at", "updated_at", "owner_id"},
|
||||||
|
"file": {"status", "size", "mime_type", "created_at"},
|
||||||
|
"task": {"status", "priority", "due_date", "created_at"},
|
||||||
|
"calendar_event": {"status", "start_time", "end_time", "created_at"},
|
||||||
|
"mailbox": {"status", "created_at"},
|
||||||
|
"address": {"country", "city", "created_at", "updated_at"},
|
||||||
|
"contact_folder": {"name", "created_at"},
|
||||||
|
"workflow": {"status", "created_at"},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def _serialize_policy(p: EntityPolicy) -> dict:
|
def _serialize_policy(p: EntityPolicy) -> dict:
|
||||||
return {
|
return {
|
||||||
@@ -51,6 +66,7 @@ def _serialize_policy(p: EntityPolicy) -> dict:
|
|||||||
def build_sql_condition(
|
def build_sql_condition(
|
||||||
conditions: dict[str, Any],
|
conditions: dict[str, Any],
|
||||||
model: type,
|
model: type,
|
||||||
|
entity_type: str | None = None,
|
||||||
) -> Any:
|
) -> Any:
|
||||||
"""Recursively translate a JSONB conditions block into a SQLAlchemy filter expression.
|
"""Recursively translate a JSONB conditions block into a SQLAlchemy filter expression.
|
||||||
|
|
||||||
@@ -65,6 +81,11 @@ def build_sql_condition(
|
|||||||
}
|
}
|
||||||
|
|
||||||
Nested conditions are supported via rules that contain a nested conditions block.
|
Nested conditions are supported via rules that contain a nested conditions block.
|
||||||
|
|
||||||
|
When *entity_type* is provided, each rule's ``field`` is validated against
|
||||||
|
the ABAC_ALLOWED_FIELDS whitelist for that entity type. Unknown fields are
|
||||||
|
logged and skipped (not crashed) to prevent policies from filtering on
|
||||||
|
sensitive columns such as ``tenant_id`` or ``password_hash``.
|
||||||
"""
|
"""
|
||||||
if not conditions or "operator" not in conditions or "rules" not in conditions:
|
if not conditions or "operator" not in conditions or "rules" not in conditions:
|
||||||
return None
|
return None
|
||||||
@@ -80,7 +101,7 @@ def build_sql_condition(
|
|||||||
for rule in rules:
|
for rule in rules:
|
||||||
# Nested conditions block
|
# Nested conditions block
|
||||||
if "operator" in rule and "rules" in rule:
|
if "operator" in rule and "rules" in rule:
|
||||||
nested = build_sql_condition(rule, model)
|
nested = build_sql_condition(rule, model, entity_type=entity_type)
|
||||||
if nested is not None:
|
if nested is not None:
|
||||||
clauses.append(nested)
|
clauses.append(nested)
|
||||||
continue
|
continue
|
||||||
@@ -96,6 +117,17 @@ def build_sql_condition(
|
|||||||
logger.warning(f"Unsupported condition operator: {op}")
|
logger.warning(f"Unsupported condition operator: {op}")
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# ── Field whitelist check ──────────────────────────────────
|
||||||
|
# Prevent policies from filtering on sensitive columns
|
||||||
|
if entity_type is not None:
|
||||||
|
allowed = ABAC_ALLOWED_FIELDS.get(entity_type)
|
||||||
|
if allowed is not None and field_name not in allowed:
|
||||||
|
logger.warning(
|
||||||
|
f"ABAC field '{field_name}' not in whitelist for "
|
||||||
|
f"entity_type '{entity_type}' — skipping rule"
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
|
||||||
# Get the model attribute
|
# Get the model attribute
|
||||||
attr: InstrumentedAttribute | None = getattr(model, field_name, None)
|
attr: InstrumentedAttribute | None = getattr(model, field_name, None)
|
||||||
if attr is None:
|
if attr is None:
|
||||||
@@ -292,7 +324,7 @@ async def apply_policy_filter(
|
|||||||
deny_clauses.append(True)
|
deny_clauses.append(True)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
condition = build_sql_condition(policy.conditions, model)
|
condition = build_sql_condition(policy.conditions, model, entity_type=entity_type)
|
||||||
if condition is None:
|
if condition is None:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,397 @@
|
|||||||
|
"""Tests for ABAC (Attribute-Based Access Control) integration.
|
||||||
|
|
||||||
|
These tests verify that ABAC policies are correctly applied in the
|
||||||
|
visibility filter alongside the existing owner/shared visibility logic.
|
||||||
|
|
||||||
|
Requires a PostgreSQL test database — uses the same conftest.py fixtures
|
||||||
|
as the rest of the test suite (db_session, seed_tenant_and_users).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import uuid
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from app.core.visibility import apply_visibility_filter
|
||||||
|
from app.models.contact import Contact
|
||||||
|
from app.models.entity_policy import EntityPolicy
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.asyncio
|
||||||
|
|
||||||
|
|
||||||
|
# ─── Helpers ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def _make_contact(
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
name: str,
|
||||||
|
status: str = "lead",
|
||||||
|
country: str | None = None,
|
||||||
|
owner_id: uuid.UUID | None = None,
|
||||||
|
created_by: uuid.UUID | None = None,
|
||||||
|
) -> Contact:
|
||||||
|
"""Create a Contact instance for testing."""
|
||||||
|
return Contact(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
type="company",
|
||||||
|
name=name,
|
||||||
|
displayname=name,
|
||||||
|
status=status,
|
||||||
|
country=country,
|
||||||
|
owner_id=owner_id,
|
||||||
|
created_by=created_by,
|
||||||
|
updated_by=created_by,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _make_policy(
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
name: str,
|
||||||
|
entity_type: str,
|
||||||
|
principal_type: str,
|
||||||
|
principal_id: uuid.UUID,
|
||||||
|
effect: str = "allow",
|
||||||
|
conditions: dict | None = None,
|
||||||
|
priority: int = 0,
|
||||||
|
) -> EntityPolicy:
|
||||||
|
"""Create an EntityPolicy instance for testing."""
|
||||||
|
return EntityPolicy(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
name=name,
|
||||||
|
entity_type=entity_type,
|
||||||
|
principal_type=principal_type,
|
||||||
|
principal_id=principal_id,
|
||||||
|
effect=effect,
|
||||||
|
conditions=conditions,
|
||||||
|
priority=priority,
|
||||||
|
enabled=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def _seed_and_query(
|
||||||
|
db_session,
|
||||||
|
contacts: list[Contact],
|
||||||
|
policies: list[EntityPolicy],
|
||||||
|
user_id: uuid.UUID,
|
||||||
|
tenant_id: uuid.UUID,
|
||||||
|
):
|
||||||
|
"""Seed contacts + policies, then run visibility filter and return results."""
|
||||||
|
db_session.add_all(contacts)
|
||||||
|
db_session.add_all(policies)
|
||||||
|
await db_session.flush()
|
||||||
|
|
||||||
|
query = select(Contact).where(Contact.tenant_id == tenant_id)
|
||||||
|
query = await apply_visibility_filter(
|
||||||
|
db_session, query, "contact", Contact, user_id, tenant_id
|
||||||
|
)
|
||||||
|
result = await db_session.execute(query)
|
||||||
|
return result.scalars().all()
|
||||||
|
|
||||||
|
|
||||||
|
# ─── Tests ────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_deny_policy_blocks_access(db_session):
|
||||||
|
"""A deny policy matching specific rows must exclude those rows."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
# Two contacts owned by viewer — one VIP, one regular
|
||||||
|
vip = _make_contact(tenant_id, "VIP Corp", status="vip", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
regular = _make_contact(tenant_id, "Regular Inc", status="lead", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
|
||||||
|
# Deny policy: block contacts where status = 'vip'
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-vip",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=10,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [vip, regular], [deny_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Regular Inc" in names, "Regular contact should be visible"
|
||||||
|
assert "VIP Corp" not in names, "VIP contact should be blocked by deny policy"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_allow_policy_grants_access(db_session):
|
||||||
|
"""An allow policy matching specific rows must include those rows even if not owned."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
admin_id = seed["admin_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
# Contact owned by admin — viewer would not normally see it
|
||||||
|
owned_by_admin = _make_contact(
|
||||||
|
tenant_id, "Admin Secret", status="lead", owner_id=admin_id, created_by=admin_id
|
||||||
|
)
|
||||||
|
# Tenant-owned contact (owner_id=None) — viewer can see via tenant-owned rule
|
||||||
|
tenant_owned = _make_contact(tenant_id, "Public Co", status="lead", owner_id=None, created_by=admin_id)
|
||||||
|
|
||||||
|
# Allow policy: viewer can see contacts with status='lead'
|
||||||
|
allow_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"allow-lead",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="allow",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "lead"}],
|
||||||
|
},
|
||||||
|
priority=5,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [owned_by_admin, tenant_owned], [allow_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Public Co" in names, "Tenant-owned contact should be visible"
|
||||||
|
assert "Admin Secret" in names, "Admin-owned contact should be visible via allow policy"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_policy_with_conditions(db_session):
|
||||||
|
"""Policy with JSONB conditions (status='vip') filters correctly."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
vip1 = _make_contact(tenant_id, "VIP One", status="vip", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
vip2 = _make_contact(tenant_id, "VIP Two", status="vip", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
regular = _make_contact(tenant_id, "Regular Co", status="lead", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
|
||||||
|
# Deny policy with conditions: block where status='vip'
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-vip-conditional",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=10,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [vip1, vip2, regular], [deny_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Regular Co" in names, "Regular contact should be visible"
|
||||||
|
assert "VIP One" not in names, "VIP One should be blocked"
|
||||||
|
assert "VIP Two" not in names, "VIP Two should be blocked"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_policy_priority(db_session):
|
||||||
|
"""Higher priority deny policy takes precedence over allow policy."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
# Contact owned by viewer with status='vip'
|
||||||
|
contact = _make_contact(tenant_id, "Priority Test", status="vip", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
|
||||||
|
# Allow policy (low priority): allow status='vip'
|
||||||
|
allow_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"allow-vip-low",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="allow",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=1,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Deny policy (high priority): deny status='vip'
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-vip-high",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=100,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [contact], [allow_policy, deny_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Priority Test" not in names, (
|
||||||
|
"Deny policy with higher priority should block the contact despite allow policy"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_field_whitelist_blocks_unknown_field(db_session):
|
||||||
|
"""Policy with a non-whitelisted field should be skipped (not crash)."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
contact = _make_contact(tenant_id, "Whitelist Test", status="lead", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
|
||||||
|
# Deny policy referencing 'password_hash' — not in whitelist
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-bad-field",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "password_hash", "op": "eq", "value": "secret"}],
|
||||||
|
},
|
||||||
|
priority=10,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [contact], [deny_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Whitelist Test" in names, (
|
||||||
|
"Contact should remain visible because the deny policy with a non-whitelisted "
|
||||||
|
"field was skipped"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_does_not_break_owner_visibility(db_session):
|
||||||
|
"""Owner remains visible even when a deny policy exists for other rows."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
# Contact owned by viewer with status='lead' (not matching deny)
|
||||||
|
own_contact = _make_contact(tenant_id, "My Own", status="lead", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
# Contact owned by viewer with status='vip' (matching deny)
|
||||||
|
vip_contact = _make_contact(tenant_id, "My VIP", status="vip", owner_id=viewer_id, created_by=viewer_id)
|
||||||
|
|
||||||
|
# Deny policy: block status='vip'
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-vip-owner-test",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=10,
|
||||||
|
)
|
||||||
|
|
||||||
|
results = await _seed_and_query(db_session, [own_contact, vip_contact], [deny_policy], viewer_id, tenant_id)
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "My Own" in names, "Owner's own contact (non-matching deny) should be visible"
|
||||||
|
assert "My VIP" not in names, "Owner's VIP contact should be blocked by deny policy"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_abac_does_not_break_shared_visibility(db_session):
|
||||||
|
"""Shared entities remain visible even when a deny policy exists for other rows."""
|
||||||
|
from tests.conftest import seed_tenant_and_users
|
||||||
|
from app.models.entity_permission import EntityPermission
|
||||||
|
|
||||||
|
seed = await seed_tenant_and_users(db_session)
|
||||||
|
tenant_id = seed["tenant_a"].id
|
||||||
|
admin_id = seed["admin_a"].id
|
||||||
|
viewer_id = seed["viewer_a"].id
|
||||||
|
|
||||||
|
# Contact owned by admin, shared with viewer
|
||||||
|
shared_contact = _make_contact(
|
||||||
|
tenant_id, "Shared Contact", status="lead", owner_id=admin_id, created_by=admin_id
|
||||||
|
)
|
||||||
|
# Contact owned by admin, NOT shared, but allow policy grants access
|
||||||
|
allow_contact = _make_contact(
|
||||||
|
tenant_id, "Allow Contact", status="vip", owner_id=admin_id, created_by=admin_id
|
||||||
|
)
|
||||||
|
|
||||||
|
# Share the first contact with viewer via entity_permissions
|
||||||
|
db_session.add_all([shared_contact, allow_contact])
|
||||||
|
await db_session.flush()
|
||||||
|
|
||||||
|
permission = EntityPermission(
|
||||||
|
tenant_id=tenant_id,
|
||||||
|
entity_type="contact",
|
||||||
|
entity_id=shared_contact.id,
|
||||||
|
principal_type="user",
|
||||||
|
principal_id=viewer_id,
|
||||||
|
permission_level="read",
|
||||||
|
)
|
||||||
|
db_session.add(permission)
|
||||||
|
|
||||||
|
# Deny policy: deny status='vip' (should block allow_contact but not shared_contact)
|
||||||
|
deny_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"deny-vip-shared-test",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="deny",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=10,
|
||||||
|
)
|
||||||
|
# Allow policy: allow status='vip' (would grant access to allow_contact)
|
||||||
|
allow_policy = _make_policy(
|
||||||
|
tenant_id,
|
||||||
|
"allow-vip-shared-test",
|
||||||
|
"contact",
|
||||||
|
"user",
|
||||||
|
viewer_id,
|
||||||
|
effect="allow",
|
||||||
|
conditions={
|
||||||
|
"operator": "AND",
|
||||||
|
"rules": [{"field": "status", "op": "eq", "value": "vip"}],
|
||||||
|
},
|
||||||
|
priority=5,
|
||||||
|
)
|
||||||
|
db_session.add_all([deny_policy, allow_policy])
|
||||||
|
await db_session.flush()
|
||||||
|
|
||||||
|
query = select(Contact).where(Contact.tenant_id == tenant_id)
|
||||||
|
query = await apply_visibility_filter(
|
||||||
|
db_session, query, "contact", Contact, viewer_id, tenant_id
|
||||||
|
)
|
||||||
|
result = await db_session.execute(query)
|
||||||
|
results = result.scalars().all()
|
||||||
|
|
||||||
|
names = {r.name for r in results}
|
||||||
|
assert "Shared Contact" in names, "Shared contact should be visible (deny doesn't match it)"
|
||||||
|
assert "Allow Contact" not in names, (
|
||||||
|
"Allow contact should be blocked: deny policy (status=vip) takes precedence "
|
||||||
|
"over allow policy"
|
||||||
|
)
|
||||||
Reference in New Issue
Block a user