Skip to content

feat(auth): bearer middleware - /mcp accepts oauth bearers (closes #217)#218

Merged
IgorShevchik merged 2 commits into
mainfrom
claude/oauth-pr217-bearer-middleware
Jun 5, 2026
Merged

feat(auth): bearer middleware - /mcp accepts oauth bearers (closes #217)#218
IgorShevchik merged 2 commits into
mainfrom
claude/oauth-pr217-bearer-middleware

Conversation

@IgorShevchik

Copy link
Copy Markdown
Collaborator

Summary

The last wire of the OAuth rollout. After this lands the OAuth flow is end-to-end usable: /install/callback → Bearer → /mcp accepts it. Operators flip NUXT_BITRIX24_OAUTH_ENABLED=true and the multi-tenant flow is live; webhook-only forks (flag off) see zero behaviour change.

Architecture

  • server/middleware/mcp-auth.ts (existing h3 global middleware) — early-returns when NUXT_BITRIX24_OAUTH_ENABLED=true. Webhook-only forks keep the existing NUXT_MCP_AUTH_TOKEN constant-time compare exactly as today.
  • server/mcp/index.ts (NEW) — defineMcpHandler({ middleware }) is the toolkit seam that lets us enclose next() in an AsyncLocalStorage scope (an h3 middleware can THROW or CONTINUE, but it cannot wrap). The middleware extracts Authorization: Bearer, hashes with sha256, resolves via the new TokenStore.inspectBearer verb, and on the happy path calls runWithTenant({memberId, userId, requestId}, () => next()).
  • TokenStore.inspectBearer(hash) (NEW verb) — does NOT filter revoked rows. Returns { memberId, userId, revokedAt: number | null } | undefined so the middleware can distinguish the three §11 deny branches:
    • mcp.auth.deny.bearer-unknown — no row at all, or no Bearer header.
    • mcp.auth.deny.bearer-revoked — row exists, revoked_at set.
    • mcp.auth.deny.bearer-orphan — row active, oauth_tokens parent missing (CASCADE prevents this, but a manual SQLite edit could create the state; logged ERROR per §11).
  • Every 401 carries a WWW-Authenticate: Bearer error="invalid_token", errorCode="…" header (RFC 6750 §3) so the user pasting the error into Slack and the operator greping the JSONL log find the same string.

Side effects

What's in the diff

File Change
server/mcp/index.ts (NEW, ~140 lines) defineMcpHandler({ middleware }) — the Bearer resolver + ALS wrap
server/middleware/mcp-auth.ts Early-return when flag on
server/utils/token-store.ts inspectBearer verb + BearerInspection interface + prepared statement
server/api/oauth/callback.get.ts Removed "Not active yet" warning
docs/OAUTH-DESIGN.md §10/§11 Taxonomy flipped to live, rollout table updated
skills/run-manual-qa/references/issue-scaffold.md OAuth paragraph rewritten
scripts/manual-qa-pr2c.{sh,ps1} /mcp Bearer-auth probe
tests/unit/mcp/auth-middleware.test.ts (NEW, 7 cases) flag-off pass-through, 4 deny branches, happy path proves runWithTenant scope, requestId is 32-char hex, no Bearer in any log, distinct requestIds per request
tests/unit/middleware/mcp-auth.test.ts (+1 case) Early-return on flag
tests/unit/utils/token-store.test.ts (+3 cases) inspectBearer revokedAt semantics

663 unit tests passed (+11 vs main), typecheck + eslint + bash syntax clean.

Test plan

  • npx vitest run — 663 passed.
  • npx nuxt typecheck — clean.
  • npx eslint . — clean.
  • bash -n scripts/manual-qa-pr2c.sh — clean.
  • Reviewer manual: boot with flag=true + a valid Bearer + run an MCP tool → tenant context populated, tool dispatches through useBitrix24TenantuseBitrix24OAuth.
  • Reviewer manual: boot with flag=true + an unknown Bearer → 401 + WWW-Authenticate: ... errorCode="BEARER-UNKNOWN".
  • Reviewer manual: boot with flag=false → existing NUXT_MCP_AUTH_TOKEN flow unchanged.

Refs / closes

Changelog framing (no BREAKING CHANGE)

  • Feature → Auth: /mcp now accepts OAuth Bearers minted via /api/oauth/callback when the flag is on (the wire that completes the multi-tenant flow). Webhook-only forks unchanged.
  • Security: every Bearer-auth 401 carries a WWW-Authenticate header with the §11 errorCode so support tickets land with a grep-able identifier.

Generated by Claude Code

claude added 2 commits June 5, 2026 16:27
The last wire of the OAuth rollout. After this lands the OAuth flow is
end-to-end usable: install -> callback -> Bearer -> /mcp accepts it.

Architecture:

- server/middleware/mcp-auth.ts (h3 global middleware) — early-return
  when NUXT_BITRIX24_OAUTH_ENABLED=true. Webhook-only forks (flag off)
  keep the existing NUXT_MCP_AUTH_TOKEN constant-time compare; the
  OAuth-on path yields to the toolkit middleware because an h3
  middleware can't enclose the rest of the request in an ALS scope.

- server/mcp/index.ts (NEW) — defineMcpHandler({ middleware }). On the
  OAuth-on path: extract `Authorization: Bearer`, hash with sha256,
  resolve via the new TokenStore.inspectBearer verb (which surfaces
  revoked_at so we can tell the three §11 deny branches apart).

  Happy path: generate a 16-byte hex requestId and wrap next() in
  runWithTenant({memberId, userId, requestId}, () => next()). Every
  oauth.* and tool-side log line inside the request now carries the
  same correlation id (via getRequestId()) — one jq query reconstructs
  the whole timeline.

  Deny path: 401 with a distinct errorCode and a WWW-Authenticate
  header carrying the same code, so a user pasting it into Slack and
  an operator grepping logs find the same string.

- TokenStore.inspectBearer(hash) - new verb that does NOT filter
  revoked rows. Returns { memberId, userId, revokedAt: number | null }
  | undefined so the middleware can distinguish:
    - bearer-unknown   (no row at all, or no Bearer header)
    - bearer-revoked   (row exists, revoked_at set)
    - bearer-orphan    (row active, oauth_tokens parent missing)
  findByBearerHash stays the hot-path lookup for callers that already
  treat unknown + revoked uniformly (the token-store internals).

- Removed the "Not active yet" warning from bearerSuccessPage in
  callback.get.ts - the warning was correct in PR #216 but is now
  stale: the Bearer IS active immediately after mint.

- docs/OAUTH-DESIGN.md §11 — flipped the mcp.auth.* taxonomy from
  "(deferred)" to live, added mcp.auth.ok for the happy path. §10
  rollout table marks #217 as the last wire.

- skills/run-manual-qa/references/issue-scaffold.md — updated the
  OAuth-scaffolding paragraph: with the flag on, /mcp accepts the
  Bearer; NUXT_MCP_AUTH_TOKEN is bypassed on /mcp (the h3 middleware
  yields).

- scripts/manual-qa-pr2c.{sh,ps1} — added a /mcp probe that asserts
  401 + WWW-Authenticate carrying BEARER-UNKNOWN.

Tests (+11):
  - tests/unit/mcp/auth-middleware.test.ts (NEW, 7 cases): flag-off
    pass-through; 4 deny branches with distinct errorCodes +
    WWW-Authenticate; happy path proves runWithTenant scope visible
    inside next() (sniff getTenantContext + getRequestId), requestId
    is 32 hex chars, raw Bearer never appears in any log; each
    request gets a fresh requestId (Set-size bijection).
  - tests/unit/middleware/mcp-auth.test.ts (+1 case): the new
    early-return on bitrix24OauthEnabled=true.
  - tests/unit/utils/token-store.test.ts (+3 cases): inspectBearer
    surfaces revoked_at, distinguishes unknown from revoked.

663 passed (+11), typecheck + eslint clean.

Changelog framing (no BREAKING CHANGE):
  Feature -> Auth: /mcp accepts OAuth Bearers when the flag is on
    (the wire that completes the multi-tenant flow); webhook-only
    forks unchanged.
  Security: WWW-Authenticate header on every 401 carries the §11
    errorCode so support tickets land with a grep-able identifier.

Closes #217.
Five Sonnet agents reviewed the Bearer middleware PR. CTO verdict was
"ship", all critical findings in-PR. Programmer/Security/Tester each
caught 2-3 actionable items.

Docs (stale text now that the middleware is live):
  - §11 dropped "(once the Bearer middleware lands)" — the WWW-Authenticate
    header is set NOW, not "future". Added an explicit example of the
    header shape.
  - §10 step 5: removed "callback HTML page warns the user about this"
    (the warning was deleted earlier in this PR) — now reads as historical
    context ("between #216 and #217 the page warned about Y").
  - §7 token-store inventory updated: inspectBearer added next to
    findByBearerHash with the rationale for the two-verb split.
  - §6 mcp-auth.ts description rewritten to reflect the toolkit-vs-h3
    middleware split + the three distinct errorCodes.
  - §10 step 8 (PR-5) gained the operator-facing migration warning
    requirement from the CTO review: "NUXT_MCP_AUTH_TOKEN is bypassed on
    /mcp when OAUTH_ENABLED=true" must land in DEPLOYMENT.md before any
    public OAuth announcement.

Tester (3 must-fix):
  - ALS scope cleanup test: assert getTenantContext() is undefined
    OUTSIDE the middleware wrap. Defends against a future als.enterWith
    regression that would leak tenant context across requests.
  - WWW-Authenticate full RFC 6750 §3 shape now pinned (Bearer
    error="invalid_token" + errorCode + error_description) on
    BEARER-REVOKED and BEARER-ORPHAN tests, not just the errorCode
    substring.
  - BEARER-REVOKED takes precedence over BEARER-ORPHAN test added —
    catches a future refactor that swaps the two if-blocks.
  - Concurrent N=2 cross-tenant ALS-isolation test added: two parallel
    middleware invocations with different Bearers each see THEIR OWN
    tenant inside next(). The middleware-layer counterpart of PR-2a's
    #64 ALS spike.

Programmer:
  - Replaced `denyBearer(...)` calls with `return denyBearer(...)` so
    TypeScript narrows `token` to string without a non-null assertion.
    The `token!` on the hash construction is gone.
  - Added a one-line comment on the String(inspection.userId)
    coercion documenting the PR-2a TenantContext.userId-is-string
    contract.

Security:
  - wwwAuthHeader now escapes backslash and quote in `code` /
    `description` defensively (current callers pass literals, but a
    future refactor threading a Bitrix24-controlled value into
    `description` shouldn't be able to inject extra header attributes).
  - server/middleware/mcp-auth.ts no longer yields unconditionally on
    flag=true — defence-in-depth: it requires AT LEAST the
    `Authorization: Bearer …` shape before yielding. Worst case if the
    toolkit middleware is missing (HMR / module-init bug), /mcp still
    returns 401, not an auth bypass.

Tests: 667 passed (+4 round-1 cases), typecheck + eslint clean.
Round 1 is the FINAL review round per CTO verdict.
@IgorShevchik IgorShevchik merged commit 1541f0a into main Jun 5, 2026
17 checks passed
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.

PR-2c-bearer: wire OAuth Bearer middleware into /mcp (the last connection)

2 participants