feat(mcp): add list and get tools for users and roles

[aminghadersohi]

SUMMARY

Adds 4 new MCP tools implementing the user and role management domain for the Superset MCP service:

• list_users / get_user_info — admin-only user directory tools backed by UserDAO
• list_roles / get_role_info — admin-only role listing tools backed by a new minimal RoleDAO wrapping the FAB Role model

Privacy controls: email and roles fields on UserInfo are redacted by default. They are only returned when the caller has data model metadata access (user_can_view_data_model_metadata()). Non-sensitive fields (id, username, first_name, last_name, active) are always returned.

Both domains follow the established MCP tool pattern:

• ModelListCore / ModelGetInfoCore from mcp_core.py
• Pydantic request/response schemas with 1-based pagination
• @tool decorator with class_permission_name RBAC enforcement
• event_logger instrumentation
• Apache license headers on all files

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend-only MCP tool additions.

TESTING INSTRUCTIONS

1. Run the new unit tests:

   pytest tests/unit_tests/mcp_service/user/tool/test_user_tools.py -v
   pytest tests/unit_tests/mcp_service/role/tool/test_role_tools.py -v

2. Call tools via MCP client (requires admin credentials):

   list_users(request={})
   get_user_info(request={"identifier": 1})
   list_roles(request={})
   get_role_info(request={"identifier": 1})

3. Verify privacy: connect as a user without can_get_drill_info/can_write on Dataset and confirm email/roles are null in list_users and get_user_info responses.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

❌ Patch coverage is 61.20401% with 116 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.82%. Comparing base (f09fd63) to head (01fe935).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/mcp_service/user/tool/list_users.py	31.57%	26 Missing ⚠️
superset/mcp_service/role/tool/list_roles.py	32.35%	23 Missing ⚠️
superset/mcp_service/user/schemas.py	77.27%	20 Missing ⚠️
superset/mcp_service/user/tool/get_user_info.py	37.93%	18 Missing ⚠️
superset/mcp_service/role/tool/get_role_info.py	43.47%	13 Missing ⚠️
superset/mcp_service/role/schemas.py	83.78%	12 Missing ⚠️
superset/daos/role.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #40305      +/-   ##
==========================================
- Coverage   64.15%   63.82%   -0.33%     
==========================================
  Files        2592     2601       +9     
  Lines      138861   140228    +1367     
  Branches    32208    32498     +290     
==========================================
+ Hits        89085    89505     +420     
- Misses      48244    49160     +916     
- Partials     1532     1563      +31     

Flag	Coverage Δ
hive	39.15% <61.20%> (-0.18%)	⬇️
mysql	58.27% <61.20%> (-0.60%)	⬇️
postgres	58.35% <61.20%> (-0.60%)	⬇️
presto	40.79% <61.20%> (-0.21%)	⬇️
python	59.87% <61.20%> (-0.63%)	⬇️
sqlite	57.99% <61.20%> (-0.59%)	⬇️
unit	100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aminghadersohi]

Closing in favor of a correctly named branch (amin/mcp-users-roles). Will open new draft PR.

[bito-code-review]

Code Review Agent Run #51f8a4

Actionable Suggestions - 1

• superset/mcp_service/role/schemas.py - 1

  • Semantic duplication in column constants · Line 42-46

Additional Suggestions - 3

• superset/mcp_service/user/tool/get_user_info.py - 1

  • Use UserError.create() classmethod · Line 108-111
    The exception handler manually constructs UserError with timestamp=datetime.now(timezone.utc), duplicating logic that UserError.create() already handles at schemas.py:224-229. Use UserError.create() instead for consistency and reduced maintenance risk.
    Code suggestion

    --- a/superset/mcp_service/user/tool/get_user_info.py
    +++ b/superset/mcp_service/user/tool/get_user_info.py
     @@ -105,9 +105,6 @@ async def get_user_info(
                  "User information retrieval failed: identifier=%s, error=%s, error_type=%s"
                  % (request.identifier, str(e), type(e).__name__)
              )
    -        return UserError(
    -            error=f"Failed to get user info: {str(e)}",
    -            error_type="InternalError",
    -            timestamp=datetime.now(timezone.utc),
    -        )
    +        return UserError.create(
    +            error=f"Failed to get user info: {str(e)}",
    +            error_type="InternalError",
    +        )

• superset/mcp_service/user/tool/list_users.py - 2

  • Incorrect return type annotation · Line 58-58
    The function signature declares return type `UserList | UserError` but the implementation always returns `dict` (from `model_dump(mode='json')` on lines 90 and 145). This type annotation mismatch is misleading for static type checkers and API consumers. Update the return type to `dict[str, Any]` to match the actual behavior.
    Code suggestion

    --- superset/mcp_service/user/tool/list_users.py
    +++ superset/mcp_service/user/tool/list_users.py
     @@ -55,7 +55,7 @@ from superset.mcp_service.user.schemas import (
      async def list_users(
          request: ListUsersRequest | None = None,
          ctx: Context | None = None,
    -) -> UserList | UserError:
    +) -> dict[str, Any]:
          """List users with filtering and search. Admin only.
     

  • Dead parameter in function signature · Line 101-101
    The `_serialize_user` function accepts a `cols: list[str] | None` parameter but never uses it. The function only references `can_view_sensitive`. This dead parameter creates misleading API expectations for callers and adds unnecessary cognitive load. Either remove it or implement column-based filtering using this parameter.
    Code suggestion

    --- superset/mcp_service/user/tool/list_users.py
    +++ superset/mcp_service/user/tool/list_users.py
     @@ -98,7 +98,7 @@ async def list_users(
                  )
     
    -        def _serialize_user(obj: Any, cols: list[str] | None) -> UserInfo | None:
    +        def _serialize_user(obj: Any) -> UserInfo | None:
                  return serialize_user_object(obj, include_sensitive=can_view_sensitive)
     

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

• superset/mcp_service/role/tool/get_role_info.py - 1

  • Broad exception catch · Line 90-90

Review Details

• Files reviewed - 18 · Commit Range: 3a8ea04..01fe935

  • superset/daos/role.py
  • superset/mcp_service/app.py
  • superset/mcp_service/role/__init__.py
  • superset/mcp_service/role/schemas.py
  • superset/mcp_service/role/tool/__init__.py
  • superset/mcp_service/role/tool/get_role_info.py
  • superset/mcp_service/role/tool/list_roles.py
  • superset/mcp_service/user/__init__.py
  • superset/mcp_service/user/schemas.py
  • superset/mcp_service/user/tool/__init__.py
  • superset/mcp_service/user/tool/get_user_info.py
  • superset/mcp_service/user/tool/list_users.py
  • tests/unit_tests/mcp_service/role/__init__.py
  • tests/unit_tests/mcp_service/role/tool/__init__.py
  • tests/unit_tests/mcp_service/role/tool/test_role_tools.py
  • tests/unit_tests/mcp_service/user/__init__.py
  • tests/unit_tests/mcp_service/user/tool/__init__.py
  • tests/unit_tests/mcp_service/user/tool/test_user_tools.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

Semantic duplication in column constants

The three column constants DEFAULT_ROLE_COLUMNS, ROLE_ALL_COLUMNS, and ROLE_SORTABLE_COLUMNS are semantically duplicated — all contain ["id", "name"]. This creates maintenance risk where a future column addition requires updating three locations instead of one. Consider consolidating to a single constant.

Code suggestion

Check the AI-generated fix before applying

 --- superset/mcp_service/role/schemas.py (lines 42-46) ---
 42: +DEFAULT_ROLE_COLUMNS = ["id", "name"]
 43: +
 44: +ROLE_ALL_COLUMNS = ["id", "name"]
 45: +
 46: +ROLE_SORTABLE_COLUMNS = ["id", "name"]
 ---
 Should become:
 42: +# All role columns (used for defaults, all-columns listing, and sorting)
 43: +ROLE_COLUMNS = ["id", "name"]
 44: +
 45: +DEFAULT_ROLE_COLUMNS = ROLE_COLUMNS
 46: +ROLE_ALL_COLUMNS = ROLE_COLUMNS
 47: +ROLE_SORTABLE_COLUMNS = ROLE_COLUMNS

Code Review Run #51f8a4

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them