phase3: plugin routes static only (no dynamic registration) + require_active_plugin Redis cache + cache invalidation on activate/deactivate
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
This commit is contained in:
+74
-19
@@ -293,7 +293,12 @@ def require_active_plugin(plugin_name: str):
|
|||||||
Checks both global activation (permission registry) and per-tenant
|
Checks both global activation (permission registry) and per-tenant
|
||||||
activation (tenant_plugin_activation table).
|
activation (tenant_plugin_activation table).
|
||||||
|
|
||||||
|
Uses Redis cache for per-tenant check to avoid DB query on every request.
|
||||||
|
Cache key: plugin-activation:{tenant_id}:{plugin_name}
|
||||||
|
TTL: 60 seconds. Invalidated on activate/deactivate.
|
||||||
|
|
||||||
Returns 403 if the plugin is not active.
|
Returns 403 if the plugin is not active.
|
||||||
|
Fails closed (503) on errors.
|
||||||
"""
|
"""
|
||||||
async def _check() -> None:
|
async def _check() -> None:
|
||||||
from app.core.permission_registry import get_permission_registry
|
from app.core.permission_registry import get_permission_registry
|
||||||
@@ -307,31 +312,81 @@ def require_active_plugin(plugin_name: str):
|
|||||||
"code": "plugin_inactive",
|
"code": "plugin_inactive",
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
# Per-tenant activation check (P1.4 fix)
|
# Per-tenant activation check with Redis cache
|
||||||
# If there's an entry in tenant_plugin_activation for this
|
from app.core.redis import get_redis
|
||||||
# tenant+plugin with is_active=false, deny access.
|
|
||||||
# If no entry exists, default to active (backward compatible).
|
|
||||||
from app.core.db import async_session_maker
|
from app.core.db import async_session_maker
|
||||||
from sqlalchemy import text
|
from sqlalchemy import text
|
||||||
|
import json
|
||||||
|
|
||||||
|
redis = get_redis()
|
||||||
|
# Get tenant_id from current session context
|
||||||
async with async_session_maker() as db:
|
async with async_session_maker() as db:
|
||||||
# Get tenant_id from current session context (RLS)
|
|
||||||
result = await db.execute(
|
result = await db.execute(
|
||||||
text("""
|
text("SELECT current_setting('app.current_tenant_id', true)::uuid")
|
||||||
SELECT is_active FROM tenant_plugin_activation
|
|
||||||
WHERE plugin_name = :name
|
|
||||||
AND tenant_id = current_setting('app.current_tenant_id', true)::uuid
|
|
||||||
"""),
|
|
||||||
{"name": plugin_name},
|
|
||||||
)
|
)
|
||||||
row = result.first()
|
tenant_id = result.scalar()
|
||||||
if row is not None and not row[0]:
|
|
||||||
raise HTTPException(
|
if tenant_id is not None and redis is not None:
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
cache_key = f"plugin-activation:{tenant_id}:{plugin_name}"
|
||||||
detail={
|
cached = await redis.get(cache_key)
|
||||||
"detail": f"Plugin '{plugin_name}' is not active for this tenant",
|
if cached is not None:
|
||||||
"code": "plugin_inactive_tenant",
|
is_active = json.loads(cached)
|
||||||
},
|
if not is_active:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail={
|
||||||
|
"detail": f"Plugin '{plugin_name}' is not active for this tenant",
|
||||||
|
"code": "plugin_inactive_tenant",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
return # Cache hit — plugin is active for this tenant
|
||||||
|
|
||||||
|
# Cache miss — query DB
|
||||||
|
async with async_session_maker() as db:
|
||||||
|
result = await db.execute(
|
||||||
|
text("""
|
||||||
|
SELECT is_active FROM tenant_plugin_activation
|
||||||
|
WHERE plugin_name = :name
|
||||||
|
AND tenant_id = :tid
|
||||||
|
"""),
|
||||||
|
{"name": plugin_name, "tid": tenant_id},
|
||||||
)
|
)
|
||||||
|
row = result.first()
|
||||||
|
if row is not None:
|
||||||
|
is_active = row[0]
|
||||||
|
# Cache the result (60s TTL)
|
||||||
|
await redis.setex(cache_key, 60, json.dumps(is_active))
|
||||||
|
if not is_active:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail={
|
||||||
|
"detail": f"Plugin '{plugin_name}' is not active for this tenant",
|
||||||
|
"code": "plugin_inactive_tenant",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
# No entry = default active (backward compatible)
|
||||||
|
await redis.setex(cache_key, 60, json.dumps(True))
|
||||||
|
else:
|
||||||
|
# No Redis or no tenant_id — fallback to DB query without cache
|
||||||
|
async with async_session_maker() as db:
|
||||||
|
result = await db.execute(
|
||||||
|
text("""
|
||||||
|
SELECT is_active FROM tenant_plugin_activation
|
||||||
|
WHERE plugin_name = :name
|
||||||
|
AND tenant_id = current_setting('app.current_tenant_id', true)::uuid
|
||||||
|
"""),
|
||||||
|
{"name": plugin_name},
|
||||||
|
)
|
||||||
|
row = result.first()
|
||||||
|
if row is not None and not row[0]:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail={
|
||||||
|
"detail": f"Plugin '{plugin_name}' is not active for this tenant",
|
||||||
|
"code": "plugin_inactive_tenant",
|
||||||
|
},
|
||||||
|
)
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
|||||||
+23
-23
@@ -609,29 +609,9 @@ class PluginRegistry:
|
|||||||
# with the automation plugin if it is active
|
# with the automation plugin if it is active
|
||||||
await self._register_contributions_for_plugin(db, name, plugin)
|
await self._register_contributions_for_plugin(db, name, plugin)
|
||||||
|
|
||||||
# Register routes on FastAPI app if available
|
# NOTE: Routes are registered statically in main.py at app creation time.
|
||||||
# Track actual route objects by identity to avoid cross-plugin removal
|
# Activation status is enforced per-request via require_active_plugin().
|
||||||
if self._app is not None:
|
# No dynamic route registration here — prevents duplicate routes.
|
||||||
routers = plugin.get_routes()
|
|
||||||
mounted_routes: list[Any] = []
|
|
||||||
for router in routers:
|
|
||||||
# Check if routes from this router are already registered (P1.2 fix)
|
|
||||||
# This prevents duplicate route registration when plugins are
|
|
||||||
# statically registered in main.py AND dynamically activated
|
|
||||||
existing_paths = {getattr(r, 'path', None) for r in self._app.router.routes}
|
|
||||||
router_paths = {getattr(r, 'path', None) for r in router.routes}
|
|
||||||
if router_paths & existing_paths:
|
|
||||||
# Routes already registered — skip to avoid duplicates
|
|
||||||
logger.debug("Plugin '%s' routes already registered, skipping", name)
|
|
||||||
continue
|
|
||||||
# Snapshot existing route object IDs before inclusion
|
|
||||||
existing_ids = {id(r) for r in self._app.router.routes}
|
|
||||||
self._app.include_router(router)
|
|
||||||
# Collect newly added route objects
|
|
||||||
for r in self._app.router.routes:
|
|
||||||
if id(r) not in existing_ids:
|
|
||||||
mounted_routes.append(r)
|
|
||||||
self._mounted_routes[name] = mounted_routes
|
|
||||||
|
|
||||||
# Sync notification types from this plugin
|
# Sync notification types from this plugin
|
||||||
await self.sync_notification_types(db)
|
await self.sync_notification_types(db)
|
||||||
@@ -642,6 +622,16 @@ class PluginRegistry:
|
|||||||
await db.flush()
|
await db.flush()
|
||||||
self._db_status[name] = record
|
self._db_status[name] = record
|
||||||
|
|
||||||
|
# Invalidate Redis cache for this plugin across all tenants
|
||||||
|
try:
|
||||||
|
from app.core.redis import get_redis
|
||||||
|
redis = get_redis()
|
||||||
|
if redis is not None:
|
||||||
|
async for key in redis.scan_iter(f"plugin-activation:*:{name}"):
|
||||||
|
await redis.delete(key)
|
||||||
|
except Exception:
|
||||||
|
pass # Cache invalidation is best-effort
|
||||||
|
|
||||||
return record
|
return record
|
||||||
|
|
||||||
async def deactivate(self, db: AsyncSession, name: str) -> PluginModel:
|
async def deactivate(self, db: AsyncSession, name: str) -> PluginModel:
|
||||||
@@ -704,6 +694,16 @@ class PluginRegistry:
|
|||||||
await db.flush()
|
await db.flush()
|
||||||
self._db_status[name] = record
|
self._db_status[name] = record
|
||||||
|
|
||||||
|
# Invalidate Redis cache for this plugin across all tenants
|
||||||
|
try:
|
||||||
|
from app.core.redis import get_redis
|
||||||
|
redis = get_redis()
|
||||||
|
if redis is not None:
|
||||||
|
async for key in redis.scan_iter(f"plugin-activation:*:{name}"):
|
||||||
|
await redis.delete(key)
|
||||||
|
except Exception:
|
||||||
|
pass # Cache invalidation is best-effort
|
||||||
|
|
||||||
return record
|
return record
|
||||||
|
|
||||||
async def uninstall(
|
async def uninstall(
|
||||||
|
|||||||
Reference in New Issue
Block a user