Skip to content

[Feat] Add inbound MCP client access management and revocation - #3657

Merged
mrubens merged 2 commits into
developfrom
feature/inbound-mcp-authorizations-15kv6hbm8y7iw
Nov 4, 2026
Merged

mrubens merged 2 commits into
developfrom
feature/inbound-mcp-authorizations-15kv6hbm8y7iw

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

​Created by Roomote. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

Related issue

Fixes #3584, including the latest request to revoke all of a user's clients.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

Operators and members need deployment-side controls for inbound MCP access. Deleting a refresh session alone left previously issued access tokens usable for up to an hour.

What changed

  • Add Settings → Personal → Connected MCP clients for members, and an owner selector in Settings → Users for admins. Both support individual and bulk revocation with confirmations, empty/error/loading states, and separate request/token activity timestamps.
  • Move per-user authorization lifecycle into a shared Redis module. Bounded indexes include code-only and pending authorizations. Atomic code exchange, refresh rotation/replay detection, revoke and migration prevent revoked grants from being recreated by in-flight exchanges. Reconnecting creates a new generation; shared client registrations survive revocation.
  • Include client/grant identity in OAuth JWTs and check live state on subsequent API/MCP requests, including model listing and artifact downloads. Redis failures refuse OAuth authentication; internal user and run credential rules remain unchanged. RFC 7009 revocation uses the same lifecycle.
  • Backfill live legacy refresh sessions from bounded user/client pointers or at refresh, preserving their remaining absolute expiry. Per-user and per-client migration fences prevent resurrection after revoke. Historical authorization/activity times are shown as unknown rather than inferred from registration-expiry scores.
  • Update public docs and add a major changeset with the explicit breaking upgrade note. Replace the old mocked-Lua lifecycle coverage with real Redis execution; provision the isolated Redis test binary in CI.

Upgrade impact

Deploy the new web token issuer, drain all old issuer instances, then deploy API enforcement. Legacy already-issued access JWTs contain no client/grant identity and receive 401 from the enforcing API. Existing usable refresh tokens can obtain tracked access tokens without extending the original session expiry. Clients without a usable refresh session, or unable to recover by refreshing after 401, must reconnect/re-consent. In-progress old authorization codes must repeat consent. Redis data loss cannot be backfilled; outages fail closed. Rolling back the API removes prompt revocation enforcement; rolling back the issuer can require reconnecting migrated clients.

How it was tested

  • 12 real Redis lifecycle tests: pending/code-only grants, identity and expiry, same-client multiple-user isolation, individual/bulk revocation, replay/idempotence, code/refresh vs revoke races, legacy preservation and migration/revoke races, and Redis failures.
  • 75 web tests across seven files: OAuth authorize/token/revoke/register routes, real Redis registration/consent, protected management ownership/admin/anonymous boundaries with database member fixtures, and personal settings composition.
  • 17 auth tests: tracked claim issuance/validation, explicit legacy identity-less JWT rejection, and broker credential separation.
  • Full API suite: 3,434 tests across 290 files passed using REDIS_URL=redis://127.0.0.1:16379 pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/api exec vitest run --maxWorkers=4. Reproduced all 13 initial CI failures in older API fixtures, then updated injected OAuth contexts to carry grant identity. Database-backed download fixtures now create real Redis grants. Added regressions for download access after live grant revoke and catalog refusal on revoked/unavailable grants. All 63 tests in the three directly affected suites pass. Real signed-token HTTP tests cover refusal on subsequent /mcp, model and artifact-download requests, including old-token rejection after reconnect.
  • pnpm lint:fast, all 27 check-types:fast tasks, and pnpm knip passed, including normal pre-push hooks. Docs build validation and broken-link checks passed. Full monorepo tests were not run.
  • Real local authenticated UI: personal single revoke and admin bulk revoke for a different member completed; the admin's remaining grant stayed intact. Owner dropdown was opened as requested. Six final screenshots were visually inspected; no UI/network mocks. Disposable grants were seeded through actual lifecycle functions and cleaned up. Docs upgrade section was rendered in the local preview.
  • CodeQL triage: analysis jobs succeeded, but the result check reports js/insufficient-password-hash alerts 159 and 160. SARIF traces public user IDs and generated 256-bit OAuth codes/tokens into SHA-256 Redis lookup keys, not human-password storage. Targeted false-positive dismissal was denied HTTP 403: Resource not accessible by integration. No password-hash rule exclusion, scanner-evasion source change or unnecessary slow KDF was introduced; clearing the alerts requires code-scanning write authorization.
  • The follow-up commit's managed Judgement hook passed; the full PR's Judgement coverage remains incomplete. External Claude/ChatGPT/Codex refresh recovery, mobile layout and dark theme were not exercised.

The latest fix changes test files only; new browser proof is not applicable. The initial feature proof report retains the earlier real-app screenshots and provenance. Those images establish UI states, not the CI correction or backend enforcement.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change — one complete authorization-management feature spans issuer, API, UI and lifecycle tests.
  • pnpm lint and pnpm check-types pass locally — the repository's normal lint:fast, check-types:fast and pre-push gates passed; the full formatting-inclusive commands were not run.
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset — an equivalent changeset was authored directly at .changeset/inbound-mcp-authorizations.md, with a major bump and explicit compatibility note.

@roomote-community

roomote-community Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

No new code issues found. See session

Reviewed the three test-file updates since c935614: grant-aware injected credentials, catalog refusal on revoked grants/Redis errors, and real Redis grant creation, cleanup and download revocation. The new download regression keeps the verified credential present and checks that revoked access is rejected before object storage is touched.

Current-head CI: lint, type checks, Knip, docs, upgrade compatibility and backup restore passed. Tests, both Docker builds and JavaScript/TypeScript CodeQL analysis were still running when inspected. The earlier CodeQL alert discussion remains unresolved; this test-only update does not alter the hashing paths. Repository validation suites were left to CI. No approval submitted.

Reviewed 6f6267d

Comment thread packages/redis/src/mcp-oauth-grants.ts Dismissed
@roomote-roomote
roomote-roomote Bot marked this pull request as ready for review November 3, 2026 11:37
@roomote-roomote

roomote-roomote Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Pushed 6f6267d7 to fix the Test failures. Reproduced all 13 failures: older injected OAuth test contexts lacked the newly required live-grant identity. Updated the three affected suites; database-backed download tests now create and clean up real Redis grants. Added a regression keeping the verified credential present while revoking its grant, proving the subsequent download is refused before object storage, plus catalog refusal on revoked/unavailable grants.

Verified on the pushed head: GitHub Test now passes (8m43s), and the fresh automated review found no new code issues. Current-head lint, typecheck, knip, docs, compatibility/restore, both Docker builds and all three CodeQL analysis jobs passed. Judgement reports no rule violations with incomplete coverage.

Local validation: 63/63 targeted tests, then the full API suite: 3,434/3,434 tests across 290 files. API typecheck, scoped lint, managed commit check and normal pre-push lint/typecheck/knip gates passed. This follow-up is test-only, so no new browser proof applies. PR metadata has been refreshed.

CodeQL's result check remains blocked on alert-write permission, not an identified code defect. Current check 125898620951 reports the same alerts 159 / 160. Inspected SARIF: sources lead to public user-ID index hashing or hashes of 256-bit random OAuth codes/tokens—not human passwords. Targeted false-positive dismissal is the appropriate correction; the attempt returned HTTP 403: Resource not accessible by integration. Default CodeQL setup does not justify a checked-in rule-wide exclusion, and changing the key encoding or adding a slow password KDF would be inappropriate. No scanner suppression or hashing workaround was added. The CodeQL thread remains unresolved because the alerts/check are not cleared; security/code-scanning write authorization is required. No merge performed.

@mrubens
mrubens merged commit 5182dab into develop Nov 4, 2026
26 checks passed
@mrubens
mrubens deleted the feature/inbound-mcp-authorizations-15kv6hbm8y7iw branch November 4, 2026 12:51
@roomote-roomote roomote-roomote Bot mentioned this pull request Nov 4, 2026
8 of 12 tasks
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.

No deployment-side way to list or revoke inbound MCP OAuth client access

3 participants