From 62184bb98d19e6dc271c3ec13303bfe872219d5d Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Thu, 8 Oct 2026 18:21:46 -0400 Subject: [PATCH] fix(mcp): handle refresh_token grant in the consent proxy /token endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live PROD failure (2026-10-08) after a SUCCESSFUL initial auth: on reconnect mcp-remote POSTs grant_type=refresh_token to our /token, but handleToken only accepted authorization_code → 400 "unsupported_grant_type" on every refresh. The discovery doc advertises refresh_token in grant_types_supported, and we pass IAS's refresh_token through to the client verbatim at the code exchange — so the client correctly tries to refresh against us (its advertised AS). We just never implemented the branch. Fix: add an idpRefresh() boundary (mirrors idpExchange) that relays the client's refresh_token to IAS /oauth2/token with grant_type=refresh_token + the client's own client_id (public client, no secret, no PKCE on refresh). handleToken now branches: refresh_token → idpRefresh; authorization_code → unchanged; anything else → unsupported_grant_type. Passes rotated refresh_token through, falls back to the old one when IAS doesn't rotate. Tests: +6 (refresh proxy happy path, rotation + no-rotation fallback, missing refresh_token 400, IAS-failure 400, real idpRefresh body with fetch mocked). 31 unit tests green. --- approuter/lib/mcp-consent.js | 51 ++++++++++++++++++++++++ test/unit/mcp-consent.test.js | 73 +++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/approuter/lib/mcp-consent.js b/approuter/lib/mcp-consent.js index 616cae1d6..7ec7b24d4 100644 --- a/approuter/lib/mcp-consent.js +++ b/approuter/lib/mcp-consent.js @@ -188,6 +188,27 @@ async function idpExchange({ code, redirectUri, codeVerifier, clientId }) { return await res.json() } +// Refresh leg (stubbed in unit tests via the exported boundary). We passed IAS's +// refresh_token through to the client verbatim at /token, so the client hands it +// right back here — we relay it to IAS. clientId is the MCP client's own id (public +// client, no secret). No PKCE on refresh. +async function idpRefresh({ refreshToken, clientId }) { + const issuer = iasIssuer() + if (!issuer || !clientId) throw new Error('IAS issuer/client not configured') + const res = await fetch(`${issuer}/oauth2/token`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json' }, + body: new URLSearchParams({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + client_id: clientId, + }), + signal: AbortSignal.timeout(15_000), + }) + if (!res.ok) throw new Error(`IAS token refresh: ${res.status}`) + return await res.json() +} + // ── consent page ───────────────────────────────────────────────────────────── // The three acknowledgement statements are BYTE-IDENTICAL to the other SAP MCP // tools' screen, substituting only the service name. Styled with SAP Fundamental @@ -399,6 +420,35 @@ async function handleToken(req, res) { const rawBody = await readBody(req) const params = parseForm(req, rawBody) + // Refresh grant: the client holds IAS's refresh_token (we passed it through + // verbatim at the authorization_code exchange). Relay it to IAS. The discovery + // doc advertises refresh_token in grant_types_supported, so clients WILL use it + // once the first access_token expires — not handling it 400s every reconnect. + if (params.grant_type === 'refresh_token') { + if (!params.refresh_token) { + return sendJson(res, 400, { error: 'invalid_request', error_description: 'refresh_token required' }) + } + let t + try { + // Via the exported reference so unit tests can stub this boundary. + t = await module.exports.idpRefresh({ + refreshToken: params.refresh_token, + clientId: params.client_id, + }) + } catch (err) { + return sendJson(res, 400, { error: 'invalid_grant', error_description: String(err.message || err) }) + } + return sendJson(res, 200, { + access_token: t.access_token, + token_type: t.token_type || 'Bearer', + expires_in: t.expires_in, + id_token: t.id_token, + // IAS may rotate the refresh_token; pass the new one through, else keep the old. + refresh_token: t.refresh_token || params.refresh_token, + scope: t.scope, + }) + } + if (params.grant_type !== 'authorization_code') { return sendJson(res, 400, { error: 'unsupported_grant_type' }) } @@ -458,6 +508,7 @@ module.exports = { mcpConsentHandler, // exported for unit tests / stubbing idpExchange, + idpRefresh, resolveBaseUrl, verifyPkce, sealState, diff --git a/test/unit/mcp-consent.test.js b/test/unit/mcp-consent.test.js index c25b5bc94..681af22a6 100644 --- a/test/unit/mcp-consent.test.js +++ b/test/unit/mcp-consent.test.js @@ -316,6 +316,28 @@ describe('idpExchange (real body) — client_id forwarding', () => { }) }) +describe('idpRefresh (real body) — refresh_token forwarding', () => { + it('POSTs grant_type=refresh_token + the client refresh_token + clientId to IAS', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ access_token: 'new-at', token_type: 'Bearer', expires_in: 3600 }), + }) + const out = await consent.idpRefresh({ refreshToken: 'ias-rt', clientId: 'joule-work-desktop' }) + expect(out.access_token).toBe('new-at') + expect(fetchSpy).toHaveBeenCalledOnce() + const [url, opts] = fetchSpy.mock.calls[0] + expect(url).toBe('https://tenant.accounts.ondemand.com/oauth2/token') + const body = new URLSearchParams(opts.body) + expect(body.get('grant_type')).toBe('refresh_token') + expect(body.get('refresh_token')).toBe('ias-rt') + expect(body.get('client_id')).toBe('joule-work-desktop') + }) + + it('throws when clientId is missing', async () => { + await expect(consent.idpRefresh({ refreshToken: 'rt' })).rejects.toThrow(/not configured/) + }) +}) + describe('token endpoint — client PKCE verification + passthrough', () => { it('returns the IAS tokens when the client code_verifier matches', async () => { const code = await sealedCode() @@ -355,6 +377,57 @@ describe('token endpoint — client PKCE verification + passthrough', () => { await vi.waitFor(() => expect(res.statusCode).toBe(400)) expect(res.json().error).toBe('unsupported_grant_type') }) + + // refresh_token grant: mcp-remote uses it once the first access_token expires. + // The discovery doc advertises refresh_token in grant_types_supported, so NOT + // handling it 400s "unsupported_grant_type" on every reconnect (observed live + // on PROD 2026-10-08 after a successful initial auth). + it('proxies a refresh_token grant to IAS and returns the new tokens', async () => { + const spy = vi.spyOn(consent, 'idpRefresh').mockResolvedValue({ + access_token: 'new-at', token_type: 'Bearer', expires_in: 3600, id_token: 'new-id', refresh_token: 'rotated-rt', + }) + const body = new URLSearchParams({ + grant_type: 'refresh_token', + refresh_token: 'old-ias-rt', + client_id: 'joule-work-desktop', + }).toString() + const res = mockRes() + consent.mcpConsentHandler(postReq(consent.TOKEN_PATH, body), res, vi.fn()) + await vi.waitFor(() => expect(res.statusCode).toBe(200)) + expect(spy).toHaveBeenCalledOnce() + expect(spy.mock.calls[0][0]).toMatchObject({ refreshToken: 'old-ias-rt', clientId: 'joule-work-desktop' }) + const tok = res.json() + expect(tok.access_token).toBe('new-at') + expect(tok.refresh_token).toBe('rotated-rt') + }) + + it('refresh falls back to the old refresh_token when IAS does not rotate it', async () => { + vi.spyOn(consent, 'idpRefresh').mockResolvedValue({ + access_token: 'new-at', token_type: 'Bearer', expires_in: 3600, // no refresh_token in response + }) + const body = new URLSearchParams({ grant_type: 'refresh_token', refresh_token: 'keep-me', client_id: 'c' }).toString() + const res = mockRes() + consent.mcpConsentHandler(postReq(consent.TOKEN_PATH, body), res, vi.fn()) + await vi.waitFor(() => expect(res.statusCode).toBe(200)) + expect(res.json().refresh_token).toBe('keep-me') + }) + + it('400 invalid_request when refresh_token grant omits the refresh_token', async () => { + const body = new URLSearchParams({ grant_type: 'refresh_token', client_id: 'c' }).toString() + const res = mockRes() + consent.mcpConsentHandler(postReq(consent.TOKEN_PATH, body), res, vi.fn()) + await vi.waitFor(() => expect(res.statusCode).toBe(400)) + expect(res.json().error).toBe('invalid_request') + }) + + it('400 invalid_grant when the IAS refresh fails', async () => { + vi.spyOn(consent, 'idpRefresh').mockRejectedValue(new Error('IAS token refresh: 400')) + const body = new URLSearchParams({ grant_type: 'refresh_token', refresh_token: 'bad', client_id: 'c' }).toString() + const res = mockRes() + consent.mcpConsentHandler(postReq(consent.TOKEN_PATH, body), res, vi.fn()) + await vi.waitFor(() => expect(res.statusCode).toBe(400)) + expect(res.json().error).toBe('invalid_grant') + }) }) describe('stateless sealing', () => {