[FEAT][UI]: Role based Admin UI visibility gating#3479
Conversation
|
Thanks @madhav165 — useful feature for #3478. The admin/non-admin split for hide sections config is well-designed. Good coverage across |
rakdutta
left a comment
There was a problem hiding this comment.
I verified all six acceptance scenarios (admin vs non-admin hide lists, admin-specific hide overrides, embedded header defaults, and query-param/cookie overrides) — behavior matches the acceptance criteria.
LGTM
77f16ff to
f2f9e38
Compare
f2f9e38 to
cd40a5f
Compare
cd40a5f to
4d78912
Compare
4d78912 to
9c4629b
Compare
|
@madhav165 - can you please resolve conflicts? |
47a1a20 to
62e33d9
Compare
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
…dmin Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
…and docs Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
…d query+cookie interaction Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
… config test Address codex review gaps: - Route-level integration test verifying admin_ui() passes is_admin through to get_ui_visibility_config and applies admin hide lists - JSON-array parsing test for mcpgateway_ui_hide_header_items_admin Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
62e33d9 to
fdedb56
Compare
crivetimihai
left a comment
There was a problem hiding this comment.
Reviewed and rebased onto current main. Clean implementation — the role-based branching logic is correct, embedded-mode bypass for admins works as designed, query/cookie overrides remain strictly additive (no visibility bypass), and is_admin is derived server-side from authenticated user state.
Added tests during review for full differential coverage: backward-compat default param, admin cookie/query interaction, route-level integration verifying admin_ui() wires is_admin through to get_ui_visibility_config, and JSON-array parsing for admin header items.
Resolved one rebase conflict: docs/docs/config.schema.json was deleted on main in #4001 — dropped the PR's additions to that file. Also included .secrets.baseline update for test line shifts.
* feat(ui): add admin-specific UI hide section/header config fields Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * feat(ui): make get_ui_visibility_config role-aware for admin vs non-admin Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * docs: add admin UI hide vars to env, docker-compose, charts, schema, and docs Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * style: black formatting for config description Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix(config): apply section alias normalization to admin hide list Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * test(ui): add coverage for admin visibility default param, cookie, and query+cookie interaction Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test(ui): add route-level admin visibility test and JSON-array header config test Address codex review gaps: - Route-level integration test verifying admin_ui() passes is_admin through to get_ui_visibility_config and applies admin hide lists - JSON-array parsing test for mcpgateway_ui_hide_header_items_admin Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: update secrets baseline after test line shifts Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
✨ Feature / Enhancement PR
🔗 Epic / Issue
Closes #3478
🚀 Summary (1-2 sentences)
Adds separate UI section/header hide configuration for admin vs non-admin users. Admins can retain full UI access in deployments where non-admin views are restricted, including embedded mode where logout and team selector auto-hiding now only applies to non-admins.
🧪 Checks
make lintpassesmake testpasses📓 Notes
New Environment Variables
MCPGATEWAY_UI_HIDE_SECTIONS[]MCPGATEWAY_UI_HIDE_HEADER_ITEMS[]MCPGATEWAY_UI_HIDE_SECTIONS_ADMIN[](see all)MCPGATEWAY_UI_HIDE_HEADER_ITEMS_ADMIN[](see all)Visibility Resolution
flowchart TD A[Request to /admin/] --> B{is_admin?} B -->|yes| C[Use _ADMIN env vars] B -->|no| D[Use existing env vars] D --> E{MCPGATEWAY_UI_EMBEDDED?} E -->|yes| F[Add logout, team_selector to hidden headers] E -->|no| G[No embedded defaults] C --> H[Apply query param / cookie overrides] F --> H G --> H H --> I[Map hidden sections → hidden tabs] I --> J[Return visibility config]Backward Compatibility
Existing deployments without
_ADMINvars behave identically — admins see everything (empty list = no hiding), non-admin behavior unchanged.Files Changed
mcpgateway/config.py— 2 new fields with shared validatorsmcpgateway/admin.py—get_ui_visibility_config()gainsis_adminparam.env.example— Document new varsdocker-compose.yml,docker-compose-embedded.yml— Add new varscharts/mcp-stack/values.yaml,values.schema.json— Helm chart entriesdocs/docs/config.schema.json— Schema entriesdocs/docs/manage/admin-ui-customization.md— Role-based visibility docstests/unit/mcpgateway/test_config.py— 5 new teststests/unit/mcpgateway/test_admin.py— 6 new tests