Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions docs/prds/prd-settings-permissions-refactor.md

Large diffs are not rendered by default.

360 changes: 264 additions & 96 deletions src/backend/src/common/features.py

Large diffs are not rendered by default.

79 changes: 77 additions & 2 deletions src/backend/src/controller/settings_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,74 @@ def ensure_default_roles_exist(self):
logger.error(f"Unexpected error during default role check/creation: {e}", exc_info=True)
raise # Re-raise other unexpected errors

def upgrade_admin_role_for_new_features(self):
"""Idempotent migration: ensure the built-in Admin role has ADMIN on every feature.

Runs on startup. When new feature IDs are added to ``APP_FEATURES`` (e.g.,
the ``settings-*`` sub-page permissions introduced by the Settings
permissions refactor), the existing Admin role in the database will be
missing entries for them. This method backfills those entries with
``FeatureAccessLevel.ADMIN`` so administrators don't need to manually
re-grant access after every release.

Only the built-in Admin role (``is_admin=True``) is touched. Non-admin
roles are left untouched so their explicit grants are preserved — admins
must explicitly delegate the new sub-permissions.
"""
try:
all_features_config = get_feature_config()
roles_db = self.app_role_repo.get_all_roles(db=self._db)
updated_count = 0

for role_db in roles_db:
if not getattr(role_db, 'is_admin', False):
continue
try:
existing_perms_raw = json.loads(getattr(role_db, 'feature_permissions', '{}') or '{}')
except Exception:
logger.warning(
"Admin role '%s' has unparseable feature_permissions; resetting to ADMIN on all features.",
role_db.name,
)
existing_perms_raw = {}

existing_perms: Dict[str, str] = {
k: v for k, v in existing_perms_raw.items() if isinstance(k, str) and isinstance(v, str)
}
missing_ids = [
feat_id for feat_id in all_features_config.keys()
if feat_id not in existing_perms
]
if not missing_ids:
continue

for feat_id in missing_ids:
allowed_levels = all_features_config[feat_id].get('allowed_levels', [])
target = FeatureAccessLevel.ADMIN if FeatureAccessLevel.ADMIN in allowed_levels else (
allowed_levels[-1] if allowed_levels else FeatureAccessLevel.NONE
)
existing_perms[feat_id] = target.value

role_db.feature_permissions = json.dumps(existing_perms)
self._db.add(role_db)
updated_count += 1
logger.info(
"Backfilled Admin role '%s' with ADMIN on %d new feature ids: %s",
role_db.name, len(missing_ids), missing_ids,
)

if updated_count > 0:
self._db.flush()
logger.info("Upgraded %d Admin role(s) with new feature permissions.", updated_count)
else:
logger.info("No Admin roles needed upgrade for new feature permissions.")

except SQLAlchemyError as e:
logger.error(f"Database error during Admin role upgrade: {e}", exc_info=True)
self._db.rollback()
except Exception as e:
logger.error(f"Unexpected error during Admin role upgrade: {e}", exc_info=True)

def _setup_default_role_hierarchy(self):
"""Sets up the default role request/approval hierarchy.

Expand Down Expand Up @@ -1620,14 +1688,21 @@ def _map_db_to_api(self, role_db: AppRoleDb) -> AppRole:
)

def get_features_with_access_levels(self) -> Dict[str, Dict[str, str | List[str]]]:
"""Returns a dictionary of features and their allowed access levels."""
"""Returns a dictionary of features and their allowed access levels.

Each entry contains:
- name: human-readable label
- allowed_levels: list of allowed FeatureAccessLevel values (as strings)
- group: one of Discover/Build/Govern/Deploy/Settings/Other for UI grouping
"""
features_config = get_feature_config()
all_levels = get_all_access_levels()
# Convert enum members to their string values for API response
return {
feature_id: {
'name': config['name'],
'allowed_levels': [level.value for level in config['allowed_levels']]
'allowed_levels': [level.value for level in config['allowed_levels']],
'group': config.get('group', 'Other'),
}
for feature_id, config in features_config.items()
}
Expand Down
6 changes: 3 additions & 3 deletions src/backend/src/routes/approvals_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ async def list_agreements(
entity_id: Optional[str] = Query(None),
limit: int = Query(50, ge=1, le=100),
wizard_manager: AgreementWizardManager = Depends(get_agreement_wizard_manager),
_: bool = Depends(PermissionChecker('settings', FeatureAccessLevel.READ_ONLY)),
_: bool = Depends(PermissionChecker('settings-workflows', FeatureAccessLevel.READ_ONLY)),
):
"""List agreements, optionally filtered by entity type/id."""
return wizard_manager.list_agreements(
Expand All @@ -259,7 +259,7 @@ async def list_agreements(
async def get_agreement(
agreement_id: str,
wizard_manager: AgreementWizardManager = Depends(get_agreement_wizard_manager),
_: bool = Depends(PermissionChecker('settings', FeatureAccessLevel.READ_ONLY)),
_: bool = Depends(PermissionChecker('settings-workflows', FeatureAccessLevel.READ_ONLY)),
):
"""Get a single agreement by ID."""
result = wizard_manager.get_agreement(agreement_id)
Expand All @@ -272,7 +272,7 @@ async def get_agreement(
async def download_agreement_pdf(
agreement_id: str,
wizard_manager: AgreementWizardManager = Depends(get_agreement_wizard_manager),
_: bool = Depends(PermissionChecker('settings', FeatureAccessLevel.READ_ONLY)),
_: bool = Depends(PermissionChecker('settings-workflows', FeatureAccessLevel.READ_ONLY)),
):
"""Download the agreement as a PDF document.

Expand Down
2 changes: 1 addition & 1 deletion src/backend/src/routes/certification_levels_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

router = APIRouter(prefix="/api", tags=["Certification Levels"])

SETTINGS_FEATURE_ID = "settings"
SETTINGS_FEATURE_ID = "settings-certification-levels"


@router.get(
Expand Down
2 changes: 1 addition & 1 deletion src/backend/src/routes/connection_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

router = APIRouter(prefix="/api", tags=["Connections"])

FEATURE_ID = "settings"
FEATURE_ID = "settings-connectors"


def _get_manager(db: Session = Depends(get_db)) -> ConnectionsManager:
Expand Down
2 changes: 1 addition & 1 deletion src/backend/src/routes/mcp_tokens_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def register_routes(app):


# Require admin access for token management
require_admin = PermissionChecker(feature_id="settings", required_level=FeatureAccessLevel.READ_WRITE)
require_admin = PermissionChecker(feature_id="settings-mcp", required_level=FeatureAccessLevel.READ_WRITE)


@router.post(
Expand Down
56 changes: 39 additions & 17 deletions src/backend/src/routes/settings_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
from ..common.config import get_settings
from ..common.sanitization import sanitize_markdown_input
from ..common.search_config_loader import get_search_config_loader
from ..common.authorization import PermissionChecker
from ..common.features import FeatureAccessLevel

# Configure logging
from src.common.logging import get_logger
Expand All @@ -38,7 +40,10 @@
ROLE_NOT_FOUND = "Role not found"

@router.get('/settings')
async def get_settings_route(manager: SettingsManager = Depends(get_settings_manager)):
async def get_settings_route(
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-general', FeatureAccessLevel.READ_ONLY)),
):
"""Get all settings including available job clusters"""
try:
settings_data = manager.get_settings() # Renamed variable to avoid conflict
Expand All @@ -55,7 +60,8 @@ async def update_settings(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
settings_payload: dict, # Renamed to avoid conflict with module
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-general', FeatureAccessLevel.READ_WRITE)),
):
"""Update settings"""
success = False
Expand Down Expand Up @@ -192,7 +198,8 @@ async def create_role(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
role_data: AppRoleCreate = Body(..., embed=False),
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-roles', FeatureAccessLevel.ADMIN)),
):
"""Create a new application role."""
success = False
Expand Down Expand Up @@ -252,7 +259,8 @@ async def update_role(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
role_data: AppRole = Body(..., embed=False),
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-roles', FeatureAccessLevel.ADMIN)),
):
"""Update an existing application role."""
success = False
Expand Down Expand Up @@ -300,7 +308,8 @@ async def delete_role(
db: DBSessionDep,
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-roles', FeatureAccessLevel.ADMIN)),
):
"""Delete an application role."""
success = False
Expand Down Expand Up @@ -343,7 +352,8 @@ async def handle_role_request_decision(
request_data: HandleRoleRequest = Body(...),
settings_manager: SettingsManager = Depends(get_settings_manager),
notifications_manager: NotificationsManager = Depends(get_notifications_manager),
change_log_manager = Depends(get_change_log_manager)
change_log_manager = Depends(get_change_log_manager),
_: bool = Depends(PermissionChecker('settings-roles', FeatureAccessLevel.ADMIN)),
):
"""Handles the admin decision (approve/deny) for a role access request."""
try:
Expand Down Expand Up @@ -873,7 +883,9 @@ async def get_database_schema(manager: SettingsManager = Depends(get_settings_ma
# --- Search Configuration ---

@router.get('/settings/search-config', response_model=SearchConfigResponse)
async def get_search_config():
async def get_search_config(
_: bool = Depends(PermissionChecker('settings-search', FeatureAccessLevel.READ_ONLY)),
):
"""
Get the current search configuration.

Expand All @@ -899,7 +911,8 @@ async def update_search_config(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
config_update: SearchConfigUpdate = Body(...),
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-search', FeatureAccessLevel.ADMIN)),
):
"""
Update the search configuration.
Expand Down Expand Up @@ -987,7 +1000,8 @@ async def rebuild_search_index(
db: DBSessionDep,
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-search', FeatureAccessLevel.ADMIN)),
):
"""
Rebuild the search index.
Expand Down Expand Up @@ -1060,7 +1074,8 @@ def _field_config_to_dict(field) -> dict:
@router.get('/settings/git/status')
async def get_git_status(
include_diffs: bool = False,
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-git', FeatureAccessLevel.READ_ONLY)),
):
"""Get the current status of the Git repository.

Expand Down Expand Up @@ -1092,7 +1107,8 @@ async def clone_git_repository(
db: DBSessionDep,
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-git', FeatureAccessLevel.READ_WRITE)),
):
"""Clone the Git repository to the UC Volume.

Expand Down Expand Up @@ -1145,7 +1161,8 @@ async def pull_git_repository(
db: DBSessionDep,
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-git', FeatureAccessLevel.READ_WRITE)),
):
"""Pull latest changes from the remote Git repository."""
success = False
Expand Down Expand Up @@ -1185,7 +1202,8 @@ async def pull_git_repository(

@router.get('/settings/git/diff')
async def get_git_diff(
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-git', FeatureAccessLevel.READ_ONLY)),
):
"""Get detailed diff of all pending changes in the Git repository."""
try:
Expand Down Expand Up @@ -1218,7 +1236,8 @@ async def push_git_repository(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
commit_message: Optional[str] = Body(None, embed=True),
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-git', FeatureAccessLevel.READ_WRITE)),
):
"""Commit and push all pending changes to the remote Git repository.

Expand Down Expand Up @@ -1265,7 +1284,8 @@ async def push_git_repository(

@router.get('/settings/delivery/status')
async def get_delivery_status(
manager: SettingsManager = Depends(get_settings_manager)
manager: SettingsManager = Depends(get_settings_manager),
_: bool = Depends(PermissionChecker('settings-delivery', FeatureAccessLevel.READ_ONLY)),
):
"""Get the current delivery mode configuration and status."""
try:
Expand Down Expand Up @@ -1303,7 +1323,8 @@ async def get_delivery_status(
async def get_pending_delivery_tasks(
db: DBSessionDep,
change_type: Optional[str] = None,
notifications_manager = Depends(get_notifications_manager)
notifications_manager = Depends(get_notifications_manager),
_: bool = Depends(PermissionChecker('settings-delivery', FeatureAccessLevel.READ_ONLY)),
):
"""Get pending manual delivery tasks (notifications)."""
try:
Expand All @@ -1329,7 +1350,8 @@ async def complete_delivery_task(
audit_manager: AuditManagerDep,
current_user: AuditCurrentUserDep,
notes: Optional[str] = Body(None, embed=True),
notifications_manager = Depends(get_notifications_manager)
notifications_manager = Depends(get_notifications_manager),
_: bool = Depends(PermissionChecker('settings-delivery', FeatureAccessLevel.READ_WRITE)),
):
"""Mark a manual delivery task as completed.

Expand Down
Loading
Loading