fix(authz): decouple Ontos admin from settings:ADMIN (#404)#458
Merged
Conversation
settings:ADMIN was being used as a proxy for "Ontos admin" in three places, which let any user with Settings write access impersonate arbitrary roles via the user-menu role switcher and manage MCP tokens. Gate admin-only capabilities on membership in an AppRole flagged is_admin=true instead — the canonical Ontos admin check — and stop leaking the full role catalog to non-admins. Changes: - AuthorizationManager.is_user_ontos_admin(groups): case-insensitive intersection with assigned_groups of any is_admin role; fails closed. - require_ontos_admin FastAPI dependency for admin-only endpoints. - POST /api/user/role-override now uses is_user_ontos_admin for caller_is_admin (computed from groups, never bootstrappable via override). - GET /api/settings/roles returns the full catalog to admins; non-admins see only roles whose assigned_groups intersect their own. - mcp_tokens_routes require_admin switched from PermissionChecker( settings, READ_WRITE) to require_ontos_admin. - Frontend isAdminActual in user-info.tsx computed from is_admin role membership instead of actualPermissions['settings']; same variable governs the alpha-features toggle. Tests: 8 new unit tests for is_user_ontos_admin, a new integration suite covering the three endpoints (roles scoping, role-override admin gate, MCP tokens gate), and a frontend regression test verifying a user with settings:ADMIN but no is_admin role no longer sees the switcher or any non-membership roles.
Resolve conflicts where main added complementary helpers next to the admin-decoupling work: - src/backend/src/common/authorization.py Keep require_ontos_admin (PR) and updated get_user_groups(request=None) signature from main side-by-side. - src/backend/src/controller/authorization_manager.py Keep both is_user_ontos_admin (#404) and get_user_effective_role_ids (#326). - src/backend/src/routes/mcp_tokens_routes.py Keep require_admin = require_ontos_admin (PR intent — stronger gate than the settings-mcp feature permission from main). - src/backend/src/tests/unit/test_authorization_manager.py Merge TestIsUserOntosAdmin (#404 regression coverage) with TestGetUserEffectiveRoleIds (#326), giving each class its own fixtures. No new test failures introduced relative to origin/main.
larsgeorge-db
added a commit
that referenced
this pull request
May 30, 2026
Resolve conflict in src/backend/src/routes/settings_routes.py: keep the CurrentUserDep gate that main's #458 security-hardening added to GET /api/settings/ui-customization, while preserving the expanded docstring describing the new branding fields (app_display_name, app_short_name, favicon_url). The endpoint is the frontend bootstrap hook, so it stays open to any authenticated user (no feature permission) — consistent with /api/settings/llm.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #404.
settings:ADMINwas being treated as a proxy for "Ontos admin" — granting a user write access on Settings implicitly turned on admin-only capabilities (most visibly the role switcher in the user menu, plus the alpha-features toggle and MCP token management).This PR gates admin-only capabilities on membership in an
AppRoleflaggedis_admin=trueinstead.settings:ADMINonce again only governs administration of the Settings feature.Changes
Backend
AuthorizationManager.is_user_ontos_admin(groups)— canonical predicate. Case-insensitive intersection between the caller's workspace groups and theassigned_groupsof anyAppRolewithis_admin=True. Fails closed if role loading errors.require_ontos_adminFastAPI dependency incommon/authorization.pyfor admin-only endpoints.POST /api/user/role-overridederivescaller_is_adminfromis_user_ontos_admin(user_details.groups)instead ofactual_perms['settings'] == ADMIN. Still computed from groups, so an override can never bootstrap admin.GET /api/settings/roles— Ontos admins receive the full catalog (required for the admin role-switcher); non-admins receive only roles whoseassigned_groupsintersect their own. Stops leaking the full role catalog to anyone authenticated.mcp_tokens_routes.py—require_adminis nowrequire_ontos_admin(wasPermissionChecker(settings, READ_WRITE)).Frontend
user-info.tsx—isAdminActualis nowavailableRoles.some(r => r.is_admin && r.assigned_groups intersects userInfo.groups). Same variable governs the role switcher impersonation path and the alpha-features toggle.Tests
test_authorization_manager.pycoveringis_user_ontos_admin(empty groups, case-insensitive match,settings:ADMIN-only roles, no matches, misconfigured admin role with emptyassigned_groups, multiple admin roles, settings-manager failures fail closed).test_admin_decoupling.pywith 8 cases across three classes (roles endpoint scoping, role-override admin gate, MCP tokens gate). Includes the explicit [Bug]: settings:ADMIN implicitly grants admin-level capabilities (e.g. role switcher) #404 regression: a user withsettings:ADMINbut nois_adminrole membership is denied impersonation and MCP token access.user-info.test.tsxfixtures to carryis_admin; added a regression test that confirms a user with onlysettings:ADMIN(nois_adminrole) sees neither the switcher nor any non-membership roles.Results: 43/43 across the touched backend test files, 6/6 in
user-info.test.tsx.Out of scope
is_adminor introducing a newapp:IMPERSONATEfeature (the issue suggested both — we use the existing flag).is_user_admin/APP_ADMIN_DEFAULT_GROUPS(separate "workspace admin" concept used for moderation; intentionally untouched).GET /settings/roles/summary(used for dropdowns; revisit if a leak concern surfaces).Test plan
Settings Adminwithfeature_permissions={settings: ADMIN},is_admin=false,assigned_groups=["settings-admins"]. Log in as a member ofsettings-admins:POST /api/user/role-overrideto a non-member role returns 403.GET /api/settings/rolesreturns only theSettings Adminrole.GET /api/mcp-tokensreturns 403.Admin(is_admin=true) role: all admin affordances reappear.Admin-role member (default seed) behaves identically to before.