Skip to content

chore(oauth): add oauth4webapi and wiring points - #82

Merged
jonnyparris merged 1 commit into
mainfrom
oauth/phase-0-scaffolding
May 26, 2026
Merged

chore(oauth): add oauth4webapi and wiring points#82
jonnyparris merged 1 commit into
mainfrom
oauth/phase-0-scaffolding

Conversation

@jonnyparris

Copy link
Copy Markdown
Owner

Phase 0 of 7 — cf-portal MCP OAuth support.

Pure dependency addition. No functional changes.

oauth4webapi will be used in subsequent phases for:

  • PKCE S256 authorization code flow
  • Dynamic Client Registration (RFC 7591)
  • Token refresh with single-use refresh token handling
  • Spec-compliant token introspection

Verification

  • npm run typecheck passes
  • Diff is two lines: package.json + package-lock.json
  • No src/ changes

Plan

Full architecture and 7-phase rollout: memory/workload/plans/2026-05-26-dodo-cf-portal-oauth.md (agent-memory).

Next: Phase 1 — per-user OAuth engine in UserControl DO with in-DO mutex to prevent Kenny Johnson's documented invalid_grant re-auth loop.

beep-boop-🤖

Phase 0 of cf-portal MCP OAuth support — pure dependency addition with
no functional changes. oauth4webapi will be used in subsequent phases
for PKCE + DCR + token refresh against cf-portal.

Plan: memory/workload/plans/2026-05-26-dodo-cf-portal-oauth.md
@jonnyparris
jonnyparris marked this pull request as ready for review May 26, 2026 10:21
@jonnyparris
jonnyparris merged commit f0ba603 into main May 26, 2026
1 of 2 checks passed
@jonnyparris
jonnyparris deleted the oauth/phase-0-scaffolding branch May 26, 2026 10:21
jonnyparris added a commit that referenced this pull request May 26, 2026
Six holes were identified in the audit of PRs #82-#91. This commit
fixes all of them.

Hole 1 (medium) — /test endpoint broken for refresh_token configs
  POST /mcp-configs/:id/test was building the HttpMcpClient via
  resolveMcpConfigHeaders, which doesn't know about refresh_token
  configs (no headerKeys). The client connected with no Authorization
  header → 401. Every Test click on a refresh_token integration
  returned 'Connection failed'.

  Fix: the /test handler now branches on auth_type. refresh_token
  rows pull the bearer via getMcpAccessToken (which auto-refreshes
  if stale) and inject it as 'Authorization: Bearer <token>'. The
  SELECT was also missing auth_type — added it so the mapper sees
  the real value.

Hole 2 (low) — updateMcpConfigEncrypted could corrupt refresh_token rows
  mcpConfigUpdateSchema only allowed auth_type ∈ {oauth, static_headers}.
  A client sending {auth_type:'static_headers'} would silently
  downgrade a refresh_token row. {headers:{...}} would call
  deleteMcpConfigSecrets, wiping the entire token chain (access_token,
  refresh_token, expires_at).

  Fix: schema now accepts refresh_token. updateMcpConfigEncrypted
  explicitly rejects mutations of headers/url/auth_type/type on
  refresh_token rows with a clear error. Only enabled and name can
  be patched — enough for UI toggle and rename to work.

Hole 3 (minor) — pending DCR row leaked on upsert failure
  completeOauthDcrFlow deleted the pending row AFTER upsertRefreshTokenMcp.
  If the upsert threw (e.g. envelope dropped, sqlite error), the row
  sat around until its 10-min TTL. The captured 'code' is single-use
  at the OAuth provider anyway, so the row is dead either way.

  Fix: wrap delete in try/finally so it runs unconditionally.

Hole 4 (cosmetic) — stale comment in dodo-settings.js
  Comment claimed the callback was at /agents/oauth/callback. Actual
  path is /agents/coding-agent/<userId-hex>/callback (PR #88).

  Fix: comment updated.

Hole 5 (minor) — test blind spot on rotation chaining
  The original suite mocked a rotated refresh token in the response
  but never made a second refresh to assert that the new value was
  what got sent. The code was correct — this just adds the assertion.

  Fix: new test 'rotates the stored refresh token — second refresh
  uses the rotated value' verifies the sent refresh_token over two
  refreshes is [v0, v1] (not [v0, v0]).

Hole 6 (deferred → fixed) — no UI affordance for refresh_token configs
  refresh_token configs rendered through the generic renderIntegCard,
  with no visual indicator and no way to force-refresh.

  Fix: new renderRefreshTokenCard with an explicit 'OAuth ·
  auto-refresh' badge and a 'Refresh token' action. Backed by a
  new POST /api/mcp-configs/:id/refresh-token endpoint that proxies
  to /mcp-configs/:id/access-token?force=1.

Three new tests added in test/refresh-token-mcp-unit.test.ts:
  - /mcp-configs/:id/test injects the refreshed bearer for refresh_token
  - PUT /mcp-configs/:id refuses to mutate headers/url/auth_type on
    a refresh_token config (and that enabled toggles still work)
  - rotation chain — second refresh uses the rotated value

Tests: 832/832 pass (829 + 3 new). Typecheck clean.

beep-boop-🤖
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.

1 participant