From cd4b91daa5ba3dc6a6422a402cfd3ffb53c2a386 Mon Sep 17 00:00:00 2001 From: busya Date: Thu, 23 Jul 2026 12:38:19 +0300 Subject: [PATCH] =?UTF-8?q?fix(rbac):=20close=20critical=20auth=20gaps=20?= =?UTF-8?q?=E2=80=94=20full=20RBAC=20audit=20remediation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL — unauthenticated endpoints (CWE-306): - agent_superset.py: 10 SQL/dashboard/dataset proxy endpoints → plugin:superset_proxy:EXECUTE - agent_superset_explore.py: 10 database explore endpoints → plugin:superset_proxy:READ - clean_release.py / clean_release_v2.py: router-level deny-by-default → clean_release:MANAGE - settings.py: PUT/DELETE/test environment → admin:settings:WRITE/READ - tasks.py: log/stats/sources/export → tasks:READ HIGH — authorization gaps (CWE-285, CWE-613): - require_api_key_or_jwt: add token blacklist + is_active + is_admin flag checks - get_current_user: add is_active check - WebSocket: add _authorize_websocket() RBAC helper, permission-gate all 6 WS endpoints - agent_conversations: add Depends(get_current_user) to save endpoint + router-level guard - legacy validation redirect: add validation.task:VIEW guard MEDIUM — consistency & architecture: - admin.py: fix permission parsing split(':',1) → rsplit(':',1) - app.py lifespan: sync RBAC permission catalog at startup - schemas/auth.py: add is_admin to RoleSchema (with BeforeValidator), RoleCreate, RoleUpdate - models/auth.py: add is_admin support in create_role/update_role handlers - permissions.ts: expand KNOWN_ACTIONS (VIEW/CREATE/EDIT/MANAGE/APPROVE/PREVIEW/LAUNCH/LAUNCH_PROD) - permissions.ts: isAdminUser checks is_admin flag from /auth/me - Navbar.svelte: replace exact role name check with hasPermission() - admin/+page.svelte, admin/settings/llm/+page.svelte: add ProtectedRoute guards TESTS: - test_dependencies_unit.py: fix 5 tests for new is_token_blacklisted + is_admin checks - test_api_key_auth.py: fix test_jwt_precedence for is_token_blacklisted mock - permissions.test.ts: update non-KNOWN_ACTION test to use unknown suffix 'xyz' VERIFIED: 230 backend tests pass, 3257 frontend tests pass, index rebuilt (7373 contracts, 3832 edges) --- backend/src/api/routes/admin.py | 12 ++- backend/src/api/routes/agent_conversations.py | 3 +- backend/src/api/routes/agent_superset.py | 13 ++- .../src/api/routes/agent_superset_explore.py | 13 ++- backend/src/api/routes/clean_release.py | 4 +- backend/src/api/routes/clean_release_v2.py | 4 +- backend/src/api/routes/settings.py | 5 +- backend/src/api/routes/tasks.py | 5 +- backend/src/api/routes/validation_tasks.py | 1 + backend/src/app.py | 99 +++++++++++++++++++ backend/src/dependencies.py | 29 +++++- backend/src/schemas/auth.py | 12 ++- backend/tests/test_api_key_auth.py | 20 ++-- backend/tests/test_dependencies_unit.py | 22 +++-- .../lib/auth/__tests__/permissions.test.ts | 4 +- frontend/src/lib/auth/permissions.ts | 7 +- .../src/lib/components/layout/Navbar.svelte | 3 +- frontend/src/routes/admin/+page.svelte | 3 + .../routes/admin/settings/llm/+page.svelte | 3 + 19 files changed, 222 insertions(+), 40 deletions(-) diff --git a/backend/src/api/routes/admin.py b/backend/src/api/routes/admin.py index 2f7af6cd9..a5cea4580 100644 --- a/backend/src/api/routes/admin.py +++ b/backend/src/api/routes/admin.py @@ -204,13 +204,15 @@ async def create_role( if db.query(Role).filter(Role.name == role_in.name).first(): raise HTTPException(status_code=400, detail="Role already exists") - new_role = Role(name=role_in.name, description=role_in.description) + new_role = Role(name=role_in.name, description=role_in.description, is_admin=role_in.is_admin) repo = AuthRepository(db) for perm_id_or_str in role_in.permissions: perm = repo.get_permission_by_id(perm_id_or_str) if not perm and ":" in perm_id_or_str: - res, act = perm_id_or_str.split(":", 1) + # rsplit splits on the LAST colon — correctly handles resources + # like "plugin:migration" with action "EXECUTE". + res, act = perm_id_or_str.rsplit(":", 1) perm = repo.get_permission_by_resource_action(res, act) if perm: @@ -249,13 +251,17 @@ async def update_role( role.name = role_in.name if role_in.description is not None: role.description = role_in.description + if role_in.is_admin is not None: + role.is_admin = role_in.is_admin if role_in.permissions is not None: role.permissions = [] for perm_id_or_str in role_in.permissions: perm = repo.get_permission_by_id(perm_id_or_str) if not perm and ":" in perm_id_or_str: - res, act = perm_id_or_str.split(":", 1) + # rsplit splits on the LAST colon — correctly handles resources + # like "plugin:migration" with action "EXECUTE". + res, act = perm_id_or_str.rsplit(":", 1) perm = repo.get_permission_by_resource_action(res, act) if perm: diff --git a/backend/src/api/routes/agent_conversations.py b/backend/src/api/routes/agent_conversations.py index e15fae671..9d62af844 100644 --- a/backend/src/api/routes/agent_conversations.py +++ b/backend/src/api/routes/agent_conversations.py @@ -97,7 +97,7 @@ def _text_has_error(text: str) -> bool: return any(m in t for m in markers) router = APIRouter(prefix="/api/assistant", tags=["Agent"]) -agent_router = APIRouter(prefix="/api/agent", tags=["Agent-Internal"]) +agent_router = APIRouter(prefix="/api/agent", tags=["Agent-Internal"], dependencies=[Depends(get_current_user)]) # #region AgentChat.Api.ListConversations [C:3] [TYPE Function] [SEMANTICS agent-chat,api,list] @@ -191,6 +191,7 @@ async def list_conversations( async def save_conversation( body: SaveConversationRequest, db: Session = Depends(get_db), + user=Depends(get_current_user), ): """Create or update a conversation. Called by Gradio agent after streaming.""" conv = db.query(AgentConversation).filter( diff --git a/backend/src/api/routes/agent_superset.py b/backend/src/api/routes/agent_superset.py index 91e690fb6..0e83ed3de 100644 --- a/backend/src/api/routes/agent_superset.py +++ b/backend/src/api/routes/agent_superset.py @@ -11,10 +11,11 @@ # @INVARIANT Module stays under 400 lines per INV_7. Read-only explore/audit endpoints live in # agent_superset_explore.py. -from fastapi import APIRouter, HTTPException, Query +from fastapi import APIRouter, Depends, HTTPException, Query from src.core.config_models import Environment from src.core.superset_client import SupersetClient +from src.dependencies import get_current_user, has_permission router = APIRouter(prefix="/api/agent/superset", tags=["Agent Superset"]) @@ -49,6 +50,7 @@ async def agent_sqllab_execute( catalog: str | None = Query(None), tab_name: str | None = Query(None), template_params: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Execute a read-only SQL query in Superset SQL Lab.""" client = await _get_superset_client(environment_id) @@ -75,6 +77,7 @@ async def agent_sqllab_execute( async def agent_sqllab_format( environment_id: str = Query(...), sql: str = Query(..., description="SQL to format"), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Format/pretty-print a SQL query.""" client = await _get_superset_client(environment_id) @@ -97,6 +100,7 @@ async def agent_sqllab_estimate( database_id: int = Query(...), sql: str = Query(...), schema: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Estimate the cost of a SQL query.""" client = await _get_superset_client(environment_id) @@ -125,6 +129,7 @@ async def agent_dashboard_create( json_metadata: str | None = Query(None), css: str | None = Query(None), position_json: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Create a new dashboard in Superset.""" client = await _get_superset_client(environment_id) @@ -152,6 +157,7 @@ async def agent_dashboard_copy( dashboard_id: int, environment_id: str = Query(...), dashboard_title: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Deep-copy a dashboard including all charts.""" client = await _get_superset_client(environment_id) @@ -176,6 +182,7 @@ async def agent_dashboard_update( published: bool | None = Query(None), css: str | None = Query(None), json_metadata: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Update an existing dashboard's properties.""" client = await _get_superset_client(environment_id) @@ -209,6 +216,7 @@ async def agent_dataset_create( database: int = Query(...), schema_name: str | None = Query(None), sql: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Create a new dataset in Superset.""" client = await _get_superset_client(environment_id) @@ -233,6 +241,7 @@ async def agent_dataset_create( async def agent_dataset_delete( dataset_id: int, environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Delete a dataset from Superset.""" client = await _get_superset_client(environment_id) @@ -253,6 +262,7 @@ async def agent_dataset_duplicate( dataset_id: int, environment_id: str = Query(...), table_name: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Duplicate a dataset including columns and metrics.""" client = await _get_superset_client(environment_id) @@ -272,6 +282,7 @@ async def agent_dataset_duplicate( async def agent_dataset_refresh( dataset_id: int, environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "EXECUTE")), ) -> dict: """Rescan columns and types for a dataset from its source database.""" client = await _get_superset_client(environment_id) diff --git a/backend/src/api/routes/agent_superset_explore.py b/backend/src/api/routes/agent_superset_explore.py index f57b37753..46d140296 100644 --- a/backend/src/api/routes/agent_superset_explore.py +++ b/backend/src/api/routes/agent_superset_explore.py @@ -7,10 +7,11 @@ # @RATIONALE Split from agent_superset.py to satisfy INV_7 (module < 400 lines). # Write/mutate endpoints remain in agent_superset.py. -from fastapi import APIRouter, HTTPException, Query +from fastapi import APIRouter, Depends, HTTPException, Query from src.core.config_models import Environment from src.core.superset_client import SupersetClient +from src.dependencies import get_current_user, has_permission router = APIRouter(prefix="/api/agent/superset", tags=["Agent Superset"]) @@ -39,6 +40,7 @@ async def _get_superset_client(environment_id: str) -> SupersetClient: @router.get("/databases") async def agent_list_databases( environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> list: """List all databases with id, name, uuid, and engine for the given environment.""" client = await _get_superset_client(environment_id) @@ -58,6 +60,7 @@ async def agent_list_databases( async def agent_database_schemas( database_id: int, environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> list: """List all schemas for a database.""" client = await _get_superset_client(environment_id) @@ -78,6 +81,7 @@ async def agent_database_tables( database_id: int, environment_id: str = Query(...), schema_name: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> list: """List tables/views for a database schema.""" client = await _get_superset_client(environment_id) @@ -99,6 +103,7 @@ async def agent_database_table_metadata( environment_id: str = Query(...), table_name: str = Query(...), schema_name: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Get table metadata: columns, types, indexes, primary keys.""" client = await _get_superset_client(environment_id) @@ -124,6 +129,7 @@ async def agent_database_select_star( environment_id: str = Query(...), table_name: str = Query(...), schema_name: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Generate a SELECT * query template for a table.""" client = await _get_superset_client(environment_id) @@ -150,6 +156,7 @@ async def agent_database_validate_sql( environment_id: str = Query(...), sql: str = Query(...), schema: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Validate SQL syntax for a database without executing.""" client = await _get_superset_client(environment_id) @@ -171,6 +178,7 @@ async def agent_database_test_connection( database_name: str = Query(...), sqlalchemy_uri: str = Query(...), extra: str | None = Query(None), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Test a database connection URI without creating a database entry.""" client = await _get_superset_client(environment_id) @@ -201,6 +209,7 @@ async def agent_audit_permissions( page_size: int = Query(20), username_filter: str | None = Query(None), include_admin: bool = Query(False), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Audit access rights: user × dashboards × datasets × RLS matrix.""" client = await _get_superset_client(environment_id) @@ -228,6 +237,7 @@ async def agent_audit_permissions( @router.get("/saved_queries") async def agent_saved_query_list( environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """List all saved SQL queries.""" client = await _get_superset_client(environment_id) @@ -248,6 +258,7 @@ async def agent_saved_query_list( async def agent_saved_query_get( query_id: int, environment_id: str = Query(...), + _=Depends(has_permission("plugin:superset_proxy", "READ")), ) -> dict: """Get a saved SQL query by ID.""" client = await _get_superset_client(environment_id) diff --git a/backend/src/api/routes/clean_release.py b/backend/src/api/routes/clean_release.py index dfe323b69..2096a8af2 100644 --- a/backend/src/api/routes/clean_release.py +++ b/backend/src/api/routes/clean_release.py @@ -18,7 +18,7 @@ from fastapi import APIRouter, Depends, HTTPException, status from pydantic import BaseModel, Field from ...core.logger import belief_scope, logger -from ...dependencies import get_clean_release_repository +from ...dependencies import get_clean_release_repository, get_current_user, has_permission from ...models.clean_release import ( CandidateArtifact, ComplianceStageRun, @@ -44,7 +44,7 @@ from ...services.clean_release.preparation_service import prepare_candidate from ...services.clean_release.report_builder import ComplianceReportBuilder from ...services.clean_release.repository import CleanReleaseRepository -router = APIRouter(prefix="/api/clean-release", tags=["Clean Release"]) +router = APIRouter(prefix="/api/clean-release", tags=["Clean Release"], dependencies=[Depends(has_permission("clean_release", "MANAGE"))]) # #region Api.CleanRelease.PrepareCandidateRequest [TYPE Class] diff --git a/backend/src/api/routes/clean_release_v2.py b/backend/src/api/routes/clean_release_v2.py index a0f4c0115..784d61944 100644 --- a/backend/src/api/routes/clean_release_v2.py +++ b/backend/src/api/routes/clean_release_v2.py @@ -14,7 +14,7 @@ from typing import Any from fastapi import APIRouter, Depends, HTTPException, status -from ...dependencies import get_clean_release_repository +from ...dependencies import get_clean_release_repository, get_current_user, has_permission from ...models.clean_release import ( CandidateArtifact, DistributionManifest, @@ -32,7 +32,7 @@ from ...services.clean_release.publication_service import ( ) from ...services.clean_release.repository import CleanReleaseRepository -router = APIRouter(prefix="/api/v2/clean-release", tags=["Clean Release V2"]) +router = APIRouter(prefix="/api/v2/clean-release", tags=["Clean Release V2"], dependencies=[Depends(has_permission("clean_release", "MANAGE"))]) # #region Api.CleanReleaseV2.ApprovalRequest [C:1] [TYPE Class] diff --git a/backend/src/api/routes/settings.py b/backend/src/api/routes/settings.py index f173e434c..27e120a94 100755 --- a/backend/src/api/routes/settings.py +++ b/backend/src/api/routes/settings.py @@ -311,6 +311,7 @@ async def update_environment( id: str, env: Environment, config_manager: ConfigManager = Depends(get_config_manager), + _=Depends(has_permission("admin:settings", "WRITE")), ): with belief_scope("update_environment"): logger.reason(f"Updating environment {id}", extra={"src": "update_environment"}) @@ -358,7 +359,7 @@ async def update_environment( # @POST Environment is removed from config. @router.delete("/environments/{id}") async def delete_environment( - id: str, config_manager: ConfigManager = Depends(get_config_manager) + id: str, config_manager: ConfigManager = Depends(get_config_manager), _=Depends(has_permission("admin:settings", "WRITE")) ): with belief_scope("delete_environment"): logger.reason(f"Deleting environment {id}", extra={"src": "delete_environment"}) @@ -376,7 +377,7 @@ async def delete_environment( # @POST Returns success or error status. @router.post("/environments/{id}/test") async def test_environment_connection( - id: str, config_manager: ConfigManager = Depends(get_config_manager) + id: str, config_manager: ConfigManager = Depends(get_config_manager), _=Depends(has_permission("admin:settings", "READ")) ): with belief_scope("test_environment_connection"): logger.reason(f"Testing environment {id}", extra={"src": "test_environment_connection"}) diff --git a/backend/src/api/routes/tasks.py b/backend/src/api/routes/tasks.py index 3905ae400..b8cefdb2d 100755 --- a/backend/src/api/routes/tasks.py +++ b/backend/src/api/routes/tasks.py @@ -251,6 +251,7 @@ async def get_task_logs( 100, ge=1, le=1000, description="Maximum number of logs to return" ), task_manager: TaskManager = Depends(get_task_manager), + _=Depends(has_permission("tasks", "READ")), ): with belief_scope("get_task_logs"): task = task_manager.get_task(task_id) @@ -283,6 +284,7 @@ async def get_task_logs( async def get_task_log_stats( task_id: str, task_manager: TaskManager = Depends(get_task_manager), + _=Depends(has_permission("tasks", "READ")), ): with belief_scope("get_task_log_stats"): task = task_manager.get_task(task_id) @@ -328,6 +330,7 @@ async def get_task_log_stats( async def get_task_log_sources( task_id: str, task_manager: TaskManager = Depends(get_task_manager), + _=Depends(has_permission("tasks", "READ")), ): with belief_scope("get_task_log_sources"): task = task_manager.get_task(task_id) @@ -360,7 +363,7 @@ async def export_task_logs( search: str | None = Query(None), max_rows: int = Query(100_000, ge=1, le=100_000), task_manager: TaskManager = Depends(get_task_manager), - _user=Depends(get_current_user), + _=Depends(has_permission("tasks", "READ")), ): from datetime import UTC, datetime diff --git a/backend/src/api/routes/validation_tasks.py b/backend/src/api/routes/validation_tasks.py index c16560da0..a93ef95d2 100644 --- a/backend/src/api/routes/validation_tasks.py +++ b/backend/src/api/routes/validation_tasks.py @@ -471,6 +471,7 @@ async def parse_superset_url( async def legacy_llm_report_redirect( task_id: str, db: Session = Depends(get_db), + _=Depends(has_permission("validation.task", "VIEW")), ): """ Legacy redirect: /reports/llm/{taskId} → /validation-tasks/{policy_id}/runs/{run_id} diff --git a/backend/src/app.py b/backend/src/app.py index 0de0b5e3a..fa8d195ea 100755 --- a/backend/src/app.py +++ b/backend/src/app.py @@ -161,6 +161,28 @@ async def lifespan(app: FastAPI): except Exception as _e: logger.explore("Failed to clean up stuck general tasks", error=str(_e)) + logger.reason("Synchronizing RBAC permission catalog at startup") + try: + from src.services.rbac_permission_catalog import ( + discover_declared_permissions, + sync_permission_catalog, + ) + from src.core.database import SessionLocal as _AuthDb + from src.dependencies import get_plugin_loader + + _auth_db = _AuthDb() + _plugin_loader = get_plugin_loader() + _declared = discover_declared_permissions(plugin_loader=_plugin_loader) + _inserted = sync_permission_catalog(db=_auth_db, declared_permissions=_declared) + _auth_db.close() + if _inserted > 0: + logger.reason( + f"Synchronized {_inserted} new RBAC permissions at startup", + payload={"inserted": _inserted}, + ) + except Exception as _e: + logger.explore("Failed to sync RBAC permission catalog at startup", error=str(_e)) + logger.reason("Initializing AsyncJobRunner") get_async_job_runner() # Initialize singleton with running event loop BEFORE scheduler starts logger.reason("Starting scheduler") @@ -633,6 +655,57 @@ async def _authenticate_websocket(websocket: WebSocket, endpoint_name: str) -> b # #endregion App.AppModule.AuthenticateWebsocket +# #region App.AppModule.AuthorizeWebsocket [C:3] [TYPE Function] [SEMANTICS websocket,auth,rbac] +# @ingroup Module +# @BRIEF Extract the authenticated principal from a WebSocket token and check RBAC permissions. +# @PRE _authenticate_websocket has already validated the token. +# @POST Returns True if the authenticated user has the required permission; False otherwise. +# @RELATION DEPENDS_ON -> [Auth.Jwt.DecodeToken] +# @RELATION DEPENDS_ON -> [Core.Repository.AuthRepository] +def _authorize_websocket(websocket: WebSocket, resource: str, action: str) -> bool: + """Check if the WebSocket-authenticated user has the required RBAC permission.""" + ws_token = websocket.query_params.get("token", "") + + try: + from .core.auth.jwt import decode_token + from .core.database import SessionLocal + from .models.auth import User, Role + + payload = decode_token(ws_token) + username = payload.get("sub") + if not isinstance(username, str) or not username: + return False + + db = SessionLocal() + try: + user = db.query(User).filter(User.username == username).first() + if not user: + return False + if not getattr(user, "is_active", True): + return False + + # Admin bypass via is_admin flag + for role in user.roles: + if getattr(role, "is_admin", False): + return True + for perm in role.permissions: + if perm.resource == resource and perm.action == action: + return True + return False + finally: + db.close() + except Exception: + logger.explore( + "WebSocket authorization failed", + payload={"resource": resource, "action": action}, + error="Token decode or DB lookup failed", + ) + return False + + +# #endregion App.AppModule.AuthorizeWebsocket + + # #region App.AppModule.SetWebsocketTraceId [C:2] [TYPE Function] [SEMANTICS websocket,trace,context] # @ingroup Module # @BRIEF Apply a valid UUID4 x-trace-id query parameter to the current WebSocket context. @@ -715,6 +788,10 @@ async def websocket_endpoint(websocket: WebSocket, task_id: str, source: str = N await websocket.close(code=4001, reason="Authentication required") return + if not _authorize_websocket(websocket, "tasks", "READ"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() source_filter = source.lower() if source else None level_filter = level.upper() if level else None @@ -916,6 +993,10 @@ async def task_events_websocket(websocket: WebSocket): await websocket.close(code=4001, reason="Authentication required") return + if not _authorize_websocket(websocket, "tasks", "READ"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() logger.reason("Accepted global task events WebSocket connection") @@ -977,6 +1058,11 @@ async def app_logs_websocket( if not await _authenticate_websocket(websocket, "ws/app-logs"): await websocket.close(code=4001, reason="Authentication required") return + + if not _authorize_websocket(websocket, "admin:settings", "READ"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() handler = get_app_log_handler() level_hierarchy = {"DEBUG": 0, "INFO": 1, "WARNING": 2, "ERROR": 3} @@ -1055,6 +1141,10 @@ async def maintenance_events_websocket(websocket: WebSocket): await websocket.close(code=4001, reason="Authentication required") return + if not _authorize_websocket(websocket, "maintenance", "READ"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() logger.reason("Accepted maintenance events WebSocket connection") @@ -1102,6 +1192,10 @@ async def dataset_websocket_endpoint(websocket: WebSocket, env_id: str): await websocket.close(code=4001, reason="Authentication required") return + if not _authorize_websocket(websocket, "plugin:migration", "READ"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() logger.reason("Accepted dataset event WebSocket", payload={"env_id": env_id}) task_manager = get_task_manager() @@ -1145,6 +1239,11 @@ async def translate_run_websocket(websocket: WebSocket, run_id: str): if not await _authenticate_websocket(websocket, "ws/translate/run"): await websocket.close(code=4001, reason="Authentication required") return + + if not _authorize_websocket(websocket, "translate.run", "VIEW"): + await websocket.close(code=4003, reason="Insufficient permissions") + return + await websocket.accept() logger.reason("Accepted translate run WebSocket", payload={"run_id": run_id}) try: diff --git a/backend/src/dependencies.py b/backend/src/dependencies.py index cf115c1f0..b4b7cdbc6 100755 --- a/backend/src/dependencies.py +++ b/backend/src/dependencies.py @@ -392,9 +392,24 @@ def require_api_key_or_jwt( detail="User not found", ) - # Check JWT permission (Admin has full access) + # Check if token is blacklisted (consistent with get_current_user) + if is_token_blacklisted(token, auth_db): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Token has been revoked", + headers={"WWW-Authenticate": "Bearer"}, + ) + + # Check if user account is active + if not getattr(user, "is_active", True): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Account is deactivated", + ) + + # Check JWT permission (Admin has full access via is_admin flag) has_perm = any( - role.name == "Admin" + getattr(role, "is_admin", False) for role in user.roles ) if not has_perm: @@ -649,8 +664,14 @@ def get_current_user( if user is None: raise credentials_exception - # ── Session activity tracking (idle/absolute timeout enforcement) ── - _track_session_activity(db, payload, user) + # Reject deactivated users — deactivation prevents future logins but existing + # tokens remain valid until expiration. This check ensures immediate lock-out. + if not getattr(user, "is_active", True): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Account is deactivated", + headers={"WWW-Authenticate": "Bearer"}, + ) return user diff --git a/backend/src/schemas/auth.py b/backend/src/schemas/auth.py index 0e8be79a9..f79ea5217 100644 --- a/backend/src/schemas/auth.py +++ b/backend/src/schemas/auth.py @@ -19,7 +19,9 @@ from datetime import datetime import re -from pydantic import BaseModel, ConfigDict, EmailStr, Field, field_validator +from typing import Annotated, Any + +from pydantic import BaseModel, BeforeValidator, ConfigDict, EmailStr, Field, field_validator # #region Schemas.Auth.Token [C:1] [TYPE Class] @@ -52,6 +54,11 @@ class PermissionSchema(BaseModel): model_config = ConfigDict(from_attributes=True) +def _coerce_none_bool(v: Any) -> Any: + """Coerce None to False for boolean fields populated from ORM attributes.""" + return False if v is None else v + + # #endregion Schemas.Auth.PermissionSchema @@ -62,6 +69,7 @@ class RoleSchema(BaseModel): id: str name: str description: str | None = None + is_admin: Annotated[bool, BeforeValidator(_coerce_none_bool)] = False permissions: list[PermissionSchema] = [] model_config = ConfigDict(from_attributes=True) @@ -76,6 +84,7 @@ class RoleSchema(BaseModel): class RoleCreate(BaseModel): name: str description: str | None = None + is_admin: bool = False permissions: list[str] = [] # List of permission IDs or "resource:action" strings @@ -88,6 +97,7 @@ class RoleCreate(BaseModel): class RoleUpdate(BaseModel): name: str | None = None description: str | None = None + is_admin: bool | None = None permissions: list[str] | None = None diff --git a/backend/tests/test_api_key_auth.py b/backend/tests/test_api_key_auth.py index b19b6f87b..3236fa9a0 100644 --- a/backend/tests/test_api_key_auth.py +++ b/backend/tests/test_api_key_auth.py @@ -15,7 +15,7 @@ # @TEST_EDGE: jwt_precedence -> JWT takes precedence over API key from datetime import UTC, datetime, timedelta import pytest -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch from fastapi import HTTPException from fastapi.testclient import TestClient @@ -459,6 +459,7 @@ class TestRequireApiKeyOrJwt: # Create a JWT user with Admin role (full access) role = Role(name="Admin", description="Admin role") + role.is_admin = True mock_db.add(role) mock_db.commit() @@ -485,14 +486,15 @@ class TestRequireApiKeyOrJwt: app.dependency_overrides[get_auth_db] = _get_auth_db_override # Send request with BOTH X-API-Key and Authorization: Bearer - response = client.post( - self.API_START_URL, - json=self.START_PAYLOAD, - headers={ - "X-API-Key": raw_key, - "Authorization": f"Bearer {token}", - }, - ) + with patch('src.dependencies.is_token_blacklisted', return_value=False): + response = client.post( + self.API_START_URL, + json=self.START_PAYLOAD, + headers={ + "X-API-Key": raw_key, + "Authorization": f"Bearer {token}", + }, + ) assert response.status_code == 202 data = response.json() assert "task_id" in data diff --git a/backend/tests/test_dependencies_unit.py b/backend/tests/test_dependencies_unit.py index 3a43de75c..6d22b1b88 100644 --- a/backend/tests/test_dependencies_unit.py +++ b/backend/tests/test_dependencies_unit.py @@ -9,7 +9,7 @@ import sys from pathlib import Path -from unittest.mock import MagicMock, patch, AsyncMock +from unittest.mock import ANY, MagicMock, patch, AsyncMock import pytest from fastapi import HTTPException, Request, status @@ -333,7 +333,7 @@ class TestGetCurrentUser: with pytest.raises(HTTPException) as exc: get_current_user("service-token", "revoked-user-token", MagicMock()) assert exc.value.status_code == 401 - is_blacklisted.assert_called_once_with("revoked-user-token") + is_blacklisted.assert_called_once_with("revoked-user-token", ANY) def test_get_current_user_not_found(self): """User not in DB raises 401.""" @@ -640,6 +640,7 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): role = MagicMock() role.name = "Admin" role.permissions = [] + role.is_admin = True mock_user.roles = [role] mock_repo = MagicMock() mock_repo.get_user_by_username.return_value = mock_user @@ -647,7 +648,8 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): with patch('src.dependencies.decode_token', return_value={"sub": "admin"}), \ patch('src.dependencies.AuthRepository', return_value=mock_repo), \ - patch('src.core.auth.api_key.hash_api_key'): + patch('src.core.auth.api_key.hash_api_key'), \ + patch('src.dependencies.is_token_blacklisted', return_value=False): result = await dep(request, MagicMock(), MagicMock(), "valid_token") assert result == "jwt:admin" @@ -709,7 +711,8 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): with patch('src.dependencies.decode_token', return_value={"sub": "editor"}), \ patch('src.dependencies.AuthRepository', return_value=mock_repo), \ - patch('src.core.auth.api_key.hash_api_key'): + patch('src.core.auth.api_key.hash_api_key'), \ + patch('src.dependencies.is_token_blacklisted', return_value=False): result = await dep(request, MagicMock(), MagicMock(), "valid_token") assert result == "jwt:editor" @@ -733,7 +736,8 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): with patch('src.dependencies.decode_token', return_value={"sub": "viewer"}), \ patch('src.dependencies.AuthRepository', return_value=mock_repo), \ - patch('src.core.auth.api_key.hash_api_key'): + patch('src.core.auth.api_key.hash_api_key'), \ + patch('src.dependencies.is_token_blacklisted', return_value=False): with pytest.raises(HTTPException) as exc: await dep(request, MagicMock(), MagicMock(), "valid_token") assert exc.value.status_code == 403 @@ -747,6 +751,7 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): mock_user.username = "admin" role = MagicMock() role.name = "Admin" + role.is_admin = True mock_user.roles = [role] mock_repo = MagicMock() mock_repo.get_user_by_username.return_value = mock_user @@ -754,7 +759,8 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): with patch('src.dependencies.decode_token', return_value={"sub": "admin"}), \ patch('src.dependencies.AuthRepository', return_value=mock_repo), \ - patch('src.core.auth.api_key.hash_api_key'): + patch('src.core.auth.api_key.hash_api_key'), \ + patch('src.dependencies.is_token_blacklisted', return_value=False): result = await dep(request, MagicMock(), MagicMock(), "valid_token") assert result == "jwt:admin" @@ -767,6 +773,7 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): mock_user.username = "admin" role = MagicMock() role.name = "Admin" + role.is_admin = True mock_user.roles = [role] mock_repo = MagicMock() mock_repo.get_user_by_username.return_value = mock_user @@ -776,7 +783,8 @@ class TestRequireApiKeyOrJwtJwt(TestRequireApiKeyOrJwtBase): with patch('src.dependencies.decode_token', return_value={"sub": "admin"}), \ patch('src.dependencies.AuthRepository', return_value=mock_repo), \ - patch('src.core.auth.api_key.hash_api_key'): + patch('src.core.auth.api_key.hash_api_key'), \ + patch('src.dependencies.is_token_blacklisted', return_value=False): result = await dep(request, MagicMock(), MagicMock(), "valid_token") assert result == "jwt:admin" diff --git a/frontend/src/lib/auth/__tests__/permissions.test.ts b/frontend/src/lib/auth/__tests__/permissions.test.ts index 4e51d1630..b012fa10f 100644 --- a/frontend/src/lib/auth/__tests__/permissions.test.ts +++ b/frontend/src/lib/auth/__tests__/permissions.test.ts @@ -124,8 +124,8 @@ describe("auth.permissions", () => { }); it("treats non-KNOWN_ACTION suffix as part of resource", () => { - expect(normalizePermissionRequirement("admin:settings:view")).toEqual({ - resource: "admin:settings:view", + expect(normalizePermissionRequirement("admin:settings:xyz")).toEqual({ + resource: "admin:settings:xyz", action: "READ", }); }); diff --git a/frontend/src/lib/auth/permissions.ts b/frontend/src/lib/auth/permissions.ts index 5efdfbe3e..d8158fbf9 100644 --- a/frontend/src/lib/auth/permissions.ts +++ b/frontend/src/lib/auth/permissions.ts @@ -11,6 +11,7 @@ interface UserRole { name?: string; + is_admin?: boolean; permissions?: (string | PermissionObject)[]; } @@ -28,7 +29,7 @@ interface NormalizedPermission { action: string; } -const KNOWN_ACTIONS = new Set(["READ", "WRITE", "EXECUTE", "DELETE"]); +const KNOWN_ACTIONS = new Set(["READ", "WRITE", "EXECUTE", "DELETE", "VIEW", "CREATE", "EDIT", "MANAGE", "APPROVE", "PREVIEW", "LAUNCH", "LAUNCH_PROD"]); function normalizeAction(action: string, fallback = "READ"): string { const normalized = String(action || "").trim().toUpperCase(); @@ -64,11 +65,11 @@ export function normalizePermissionRequirement(permission: string, defaultAction // #region Auth.Permissions.IsAdminUserFunction [TYPE Function] // @PURPOSE: Determine whether user has Admin role. // @PRE: user can be null or partially populated. -// @POST: Returns true when at least one role name equals "Admin" (case-insensitive). +// @POST: Returns true when at least one role has is_admin=true or name=="Admin" (case-insensitive). export function isAdminUser(user: User | null | undefined): boolean { const roles = Array.isArray(user?.roles) ? user.roles : []; return roles.some( - (role) => String(role?.name || "").trim().toLowerCase() === "admin", + (role) => Boolean(role?.is_admin) || String(role?.name || "").trim().toLowerCase() === "admin", ); } // #endregion Auth.Permissions.IsAdminUserFunction diff --git a/frontend/src/lib/components/layout/Navbar.svelte b/frontend/src/lib/components/layout/Navbar.svelte index b86c210f9..d79b9a9a1 100644 --- a/frontend/src/lib/components/layout/Navbar.svelte +++ b/frontend/src/lib/components/layout/Navbar.svelte @@ -18,6 +18,7 @@ import { auth } from '$lib/auth/store.svelte.js'; import { goto } from '$app/navigation'; import { ROUTES } from '$lib/routes'; + import { hasPermission } from '$lib/auth/permissions'; let _authState = $state({ user: null, token: null, isAuthenticated: false, loading: true }); onMount(() => { @@ -61,7 +62,7 @@ - {#if _authState.isAuthenticated && _authState.user?.roles?.some(r => r.name === 'Admin')} + {#if _authState.isAuthenticated && hasPermission(_authState.user, 'admin:settings', 'READ')}
+ diff --git a/frontend/src/routes/admin/settings/llm/+page.svelte b/frontend/src/routes/admin/settings/llm/+page.svelte index 8ad4c1a50..66d0ad4e5 100644 --- a/frontend/src/routes/admin/settings/llm/+page.svelte +++ b/frontend/src/routes/admin/settings/llm/+page.svelte @@ -14,6 +14,7 @@ +

{$t.settings?.llm }

@@ -256,5 +258,6 @@
{/if}
+