fix(rbac): close critical auth gaps — full RBAC audit remediation
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)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user