Skip to content

Add role-based filtering to dynamic tool list#81

Open
Vortiago wants to merge 30 commits intomainfrom
claude/review-auth-viewer-role-nkvob
Open

Add role-based filtering to dynamic tool list#81
Vortiago wants to merge 30 commits intomainfrom
claude/review-auth-viewer-role-nkvob

Conversation

@Vortiago
Copy link
Owner

@Vortiago Vortiago commented Mar 8, 2026

What does this PR do?

Extends the dynamic tool list filtering system to support role-based access control in addition to scope-based filtering. Users now see only tools their Outline role permits, determined by a new min_role metadata field on each tool.

Key Changes

Architecture:

  • Replaced hand-maintained TOOL_ENDPOINT_MAP and WRITE_TOOL_NAMES files with a new introspect.py module that builds metadata maps dynamically from tool decorators at startup
  • Added build_tool_endpoint_map() and build_role_blocked_map() functions to extract meta["endpoint"] and meta["min_role"] from registered tools
  • Updated install_dynamic_tool_list() to accept and use these maps instead of importing static constants

Runtime Filtering:

  • get_blocked_tools() now performs two independent checks:
    1. Role check: calls auth.info to get user's Outline role, looks up blocked tools from role_blocked_map
    2. Scope check: calls apiKeys.list to check endpoint scopes (existing behavior)
  • Results are unioned; each check fails open independently (except 401 on scope check which blocks all tools)
  • Added _get_role_blocked_tools() helper with proper error handling and logging

Tool Metadata:

  • Every tool now carries meta={"endpoint": "...", "min_role": "viewer"|"member"|"admin"} on its @mcp.tool() decorator
  • min_role declares the minimum Outline role required to access the tool
  • Updated all tool registrations across document modules to include this metadata

Testing:

  • Refactored unit tests to use dynamically-built maps instead of static constants
  • Added TestRoleBlockedMap class with 15+ assertions verifying role-to-tool mappings match Outline permissions
  • Added test_all_tools_have_min_role_meta() to ensure every tool declares its minimum role
  • Added test_build_role_blocked_map_rejects_invalid_min_role() to catch configuration errors at startup
  • Extended E2E tests with viewer role scenarios and role+scope interaction tests
  • Added helper functions in conftest.py for managing viewer user credentials and role changes

Documentation:

  • Added docs/dynamic-tool-list.md with architecture diagrams, scope matching algorithm details, and role hierarchy explanation
  • Updated CLAUDE.md with role-based filtering overview

Why This Matters

Previously, tool visibility was determined solely by API key scopes. Now it's also constrained by the user's Outline role:

  • Viewers cannot create documents, export collections, or list archived items — even with a full-access key
  • Members cannot export all collections (admin-only)
  • Admins see all tools

This aligns MCP tool availability with Outline's actual permission model, preventing users from attempting operations their role forbids.

Checklist

  • I ran uv run pre-commit install and committed with hooks enabled
  • Tests pass locally (uv run pytest tests/ -v)

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN

claude added 17 commits March 7, 2026 09:43
The dynamic tool list now performs two independent checks:
1. auth.info — blocks write tools for viewer-role users
2. apiKeys.list — blocks tools excluded by API key scopes

Results are unioned. Each check fails open independently, so a
failure in one doesn't prevent the other from applying. This
brings the implementation in line with the documented behavior.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Rewrite the Dynamic Tool List section to explain the two independent
checks (auth.info role check + apiKeys.list scope check), their
fail-open behavior, and link to Outline's API docs for scope details.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
When apiKeys.list returns 401 (invalid/expired/revoked key), all tools
are hidden but the user had no indication why. Add a logger.warning
with the key's last4 and guidance to check Outline Settings.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
When a scoped API key lacks the apiKeys.list scope, Outline returns
403. Previously this was silently logged at DEBUG level. Now emits a
WARNING with actionable guidance to add apiKeys.list to the key's
scope array.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
When a scoped API key lacks the auth.info scope, Outline returns 403
and role-based filtering silently degrades. Now emits a WARNING with
actionable guidance to add auth.info to the key's scope array.

Mirrors the existing 403 warning for apiKeys.list.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Three new e2e scenarios test dynamic tool filtering for viewer users:

1. Viewer + full-access key: auth.info succeeds, write tools blocked
2. Viewer + scoped key WITH auth.info: role+scope union applied
3. Viewer + scoped key WITHOUT auth.info: 403 fails open, write
   tools leak through (documents the consequence of missing scope)

Refactors conftest.py to support a second Dex user:
- Split _login_and_create_api_key into _login() + _create_full_access_key()
- Add _get_user_id() and _set_user_role() helpers
- Add viewer_api_key and viewer_access_token fixtures

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Outline's policy forbids viewers from calling apiKeys.create.
The fixture now creates all API keys (full-access + scoped)
while the user is still a member, then demotes to viewer.

Scoped keys are pre-created in conftest and passed via the
viewer_scoped_keys fixture instead of inline creation in tests.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
The WRITE_TOOLS set in the e2e test duplicated the canonical
WRITE_TOOL_NAMES frozenset from write_tool_names.py. Import the
source of truth instead and derive READ_TOOLS from it.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
…pleteness test

Replace the hardcoded 32-entry ALL_TOOLS set in e2e tests with
`set(TOOL_ENDPOINT_MAP) - {"ask_ai_about_documents"}`, eliminating
another manually maintained tool list.

Add test_all_tools_have_read_only_hint to ensure every registered
tool explicitly sets readOnlyHint, closing the gap where a tool
without annotations could slip through existing cross-checks.

Remaining source-of-truth lists (each guarded by integration tests):
- TOOL_ENDPOINT_MAP: tool → endpoint mapping
- WRITE_TOOL_NAMES: tools with readOnlyHint=False

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
…adata

Replace manually maintained static map files with introspection of
registered tool metadata. Each @mcp.tool() decorator now includes
meta={"endpoint": "..."} and the endpoint map and write-tool set are
built automatically by introspect.py after registration. Adding a new
tool no longer requires updating separate map files.

- Add meta={"endpoint": "..."} to all 33 tool decorators
- Create introspect.py with build_tool_endpoint_map() and
  build_write_tool_names() builder functions
- Delete tool_endpoint_map.py and write_tool_names.py
- Update filtering.py and scope_matching.py to accept maps as params
- Update server.py to build maps after register_all() and pass them
- Update all test files to use builder functions
- Fix MockMCP classes to accept **kwargs for meta parameter

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
New developer-facing document covering startup introspection, runtime
per-request filtering, role and scope checks, the scope matching
algorithm, error handling philosophy, and module structure.

Cross-referenced from configuration.md and CLAUDE.md.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
- Fix overview diagram: checks run sequentially, not in parallel
- Remove redundant introspection mermaid diagram (table+code suffice)
- Remove duplicate module structure table (tree comments suffice)

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
…ole meta

The viewer role check was reusing readOnlyHint (meant for OUTLINE_READ_ONLY
mode) to determine which tools to block. This caused over-blocking
(add_comment blocked for viewers despite Outline allowing it) and
under-blocking (list_archived_documents, list_trash, export_collection
shown to viewers despite requiring Member role; export_all_collections
shown to viewers despite requiring Admin role).

Add min_role meta field to every tool, verified against Outline route
handlers. Replace build_write_tool_names with build_role_blocked_map
that builds {role: blocked_tools} from min_role annotations. Update
filtering, tests, and documentation.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
list_archived_documents and list_trash have min_role=member, so the
role check now correctly blocks them for viewer users even when
documents:write scope grants access. Update the hardcoded expected
set in test_viewer_scoped_key_with_auth_info accordingly.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Prevents silent degradation when a tool has a typo in its min_role
meta field. Now fails loudly at startup instead of defaulting to
viewer level.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Registers a tool with min_role="moderator" and asserts that
build_role_blocked_map raises ValueError, preventing silent
degradation from typos.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
- Union role-blocked tools with 401 response (don't discard role check)
- Log warning for unknown roles from auth.info
- Use consistent PEP 585 type annotations in filtering.py
- Add tests for 401+role union and unknown role warning

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
…andling

Split get_blocked_tools into two focused helpers (_get_role_blocked_tools
and _get_scope_blocked_tools) for better separation of concerns. Added
401 handling in auth.info to log a warning and fail open. Updated docs
to clarify interaction between OUTLINE_READ_ONLY and min_role filtering.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
claude added 2 commits March 8, 2026 17:22
The _login() function crashed with KeyError: 'location' when the
second OIDC login (for viewer user) received a non-redirect response.
Extract _require_redirect() for safe header access with diagnostics,
and add retry logic (3 attempts with back-off) to handle transient
OIDC failures.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
CodeQL flagged `api_key[-4:]` in logger.warning() calls as
"clear-text logging of sensitive information". Refactored to
compute `last4 = api_key[-4:]` once and use only that variable
in subsequent log statements.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
claude added 7 commits March 8, 2026 18:24
The bcrypt hash for the 'user' password was invalid, causing Dex to
return 401 on login and failing all viewer-related E2E tests. Regenerated
with the correct password.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
After calling users.update to demote the viewer user, verify via
auth.info that the role is actually 'viewer'. Skip viewer tests
if the role change didn't take effect.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
- Remove last4 from all log messages in filtering.py to satisfy
  CodeQL taint analysis (the variable was derived from api_key
  and still flagged as sensitive).
- Use users.update_role instead of users.update for role changes.
  users.update only handles profile fields (name, avatar, etc.)
  and silently ignores the role parameter.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
The viewer's API key returns 401 on auth.info after the role
change, so verify the role via users.info using the admin token
instead.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Add a diagnostic check that calls auth.info with the viewer's
full-access key after the role change. If the key was invalidated,
the dependent tests are skipped with a clear message rather than
failing with empty tool sets.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
- Fix get_blocked_tools docstring: says "concurrently" but checks
  run sequentially (two awaits, no asyncio.gather)
- Fix _viewer_credentials docstring: says users.update, should be
  users.update_role
- Broaden _login retry to catch httpx.RequestError alongside
  RuntimeError for network errors and timeouts
- Add comment explaining why viewer E2E tests skip on Outline 1.5.0
  (users.update_role invalidates API keys)

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
- Unify _create_full_access_key, _create_scoped_key, and
  _create_api_key_with_scope into a single _create_api_key helper
  with optional scope and skip_on_error parameters
- Extract _outline_api helper to eliminate repeated httpx.post
  boilerplate in _get_user_id, _get_user_role, _set_user_role
- Rename _ROLE_LEVELS to ROLE_LEVELS and export from __init__.py

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
claude added 3 commits March 9, 2026 11:54
…eded

Change `json=json or {}` to `json=json` so httpx omits the
body entirely when json=None, rather than sending `{}`.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Replace `exc` with `type(exc).__name__` in all 6 debug logger
calls to break CodeQL's taint chain from api_key through
OutlineClient exceptions. The exception class name is sufficient
for debugging and is not derived from sensitive data.

https://claude.ai/code/session_0122umEU4tP9VMzCTrV6SdZN
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants