From b10756fd657bb14109a8dc5a8003beaa5dea2331 Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Tue, 21 Jul 2026 15:29:23 -0700 Subject: [PATCH 1/2] Honor the offline_access scope opt-in for refresh tokens (SEP-2207, #156) AS metadata now advertises scopes_supported: [offline_access], which per the MCP spec invites clients that want refresh tokens to request it explicitly. PRM deliberately continues to omit scopes_supported (refresh tokens are not a resource requirement). A new opt-in config, mcp.refreshTokenRequiresOfflineAccess (default false), makes the authorization_code exchange withhold refresh tokens unless the granted scope carries offline_access; the default policy is unchanged so existing MCP clients that never send offline_access keep receiving refresh tokens. Existing refresh-token families are unaffected either way. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0185AoLyjuZUuKAwg9NxUDiq --- docs/configuration.md | 1 + docs/mcp-oauth.md | 9 +++++ src/lib/config.ts | 9 +++++ src/lib/mcp/token.ts | 25 +++++++++++-- src/lib/mcp/wellKnown.ts | 10 ++++++ src/types.ts | 10 ++++++ test/lib/config.test.js | 8 +++++ test/lib/mcp/token.test.js | 64 ++++++++++++++++++++++++++++++++++ test/lib/mcp/wellKnown.test.js | 10 ++++++ 9 files changed, 144 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 8eec471..c9087e7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -105,6 +105,7 @@ Opt-in support for the Model Context Protocol authorization flow ([issue #86](ht | `mcp.signingAlgorithm` | string | `RS256` | JWT signing algorithm. Only `RS256` is supported in v1 (reserved for a future EdDSA option) | | `mcp.accessTokenTtl` | number | `3600` | Access-token lifetime in seconds (default 1 hour) | | `mcp.refreshTokenTtl` | number | `2592000` | Refresh-token (family) lifetime in seconds (default 30 days) | +| `mcp.refreshTokenRequiresOfflineAccess` | boolean | `false` | When `true`, the `authorization_code` exchange only issues a refresh token when the granted scope includes `offline_access` (the [SEP-2207](https://modelcontextprotocol.io/specification/draft/basic/authorization) OIDC-flavored opt-in). Off by default because most MCP clients never request `offline_access` — withholding refresh tokens from them would force re-auth every `accessTokenTtl`. Flipping this on does not revoke existing refresh-token families. The AS metadata always advertises `offline_access` in `scopes_supported` so clients know they may request it | Sensitive leaves inside `mcp` support `${ENV_VAR}` expansion (e.g., `initialAccessToken: ${OAUTH_MCP_REGISTRATION_TOKEN}`), the same way provider credentials do. diff --git a/docs/mcp-oauth.md b/docs/mcp-oauth.md index 9eca0f7..0e78a81 100644 --- a/docs/mcp-oauth.md +++ b/docs/mcp-oauth.md @@ -191,6 +191,15 @@ Refresh tokens rotate on use: presenting an already-used token from a family revokes the whole family (replay defense). Refresh families live for `refreshTokenTtl` (default 30 days). +By default any client whose registered `grant_types` include `refresh_token` +receives a refresh token on the code exchange. The AS metadata advertises +`offline_access` in `scopes_supported` (SEP-2207), so clients that want refresh +tokens may request that scope explicitly; setting +`mcp.refreshTokenRequiresOfflineAccess: true` makes that opt-in mandatory — +refresh tokens are then withheld unless the granted scope carries +`offline_access`. The PRM document never lists `offline_access` (refresh tokens +are not a resource requirement). + --- ## The `withMCPAuth` wrapper diff --git a/src/lib/config.ts b/src/lib/config.ts index b7d395d..f2ca40d 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -86,11 +86,20 @@ export function coerceConfigBoolean(value: unknown): boolean | undefined { * - `mcp.clientCredentials.enabled` is coerced the same way — this flag mints * tokens for headless agents, so a stray truthy string must not enable it * (it is explicit opt-in, default OFF). + * - `mcp.refreshTokenRequiresOfflineAccess` is coerced the same way — a stray + * truthy string would withhold refresh tokens from clients that never + * request `offline_access` (SEP-2207 opt-in, default OFF). */ export function normalizeMcpSecurityConfig(mcpConfig: Record): void { const enabled = coerceConfigBoolean(mcpConfig.enabled); if (enabled !== undefined) mcpConfig.enabled = enabled; + // An env-expanded "false" is truthy and would activate the offline_access + // gate the operator disabled — withholding refresh tokens from every client + // that didn't request the scope. + const requiresOfflineAccess = coerceConfigBoolean(mcpConfig.refreshTokenRequiresOfflineAccess); + if (requiresOfflineAccess !== undefined) mcpConfig.refreshTokenRequiresOfflineAccess = requiresOfflineAccess; + const clientCredentials = mcpConfig.clientCredentials; if (clientCredentials && typeof clientCredentials === 'object') { const ccEnabled = coerceConfigBoolean(clientCredentials.enabled); diff --git a/src/lib/mcp/token.ts b/src/lib/mcp/token.ts index c60125e..4faa6e7 100644 --- a/src/lib/mcp/token.ts +++ b/src/lib/mcp/token.ts @@ -130,6 +130,26 @@ function allowsRefresh(client: MCPClientRecord): boolean { return !client.grant_types || client.grant_types.includes('refresh_token'); } +/** Does a space-delimited scope string include `offline_access` (SEP-2207)? */ +function scopeIncludesOfflineAccess(scope: string | undefined): boolean { + return !!scope && scope.split(/\s+/).includes('offline_access'); +} + +/** + * Should the authorization_code exchange issue a refresh token? Always gated + * on the client's registered grant_types; when + * `mcp.refreshTokenRequiresOfflineAccess` is set, additionally requires the + * granted scope to carry the explicit `offline_access` opt-in (SEP-2207). + * Default policy is grant_types-only — most MCP clients never request + * offline_access, and withholding refresh tokens from them would force + * hourly re-auth. + */ +function shouldIssueRefresh(client: MCPClientRecord, scope: string | undefined, mcpConfig: MCPConfig): boolean { + if (!allowsRefresh(client)) return false; + if (mcpConfig.refreshTokenRequiresOfflineAccess) return scopeIncludesOfflineAccess(scope); + return true; +} + /** Constant-time string compare; length-checks first (timingSafeEqual needs equal length). */ function safeEqual(a: string, b: string): boolean { const ab = Buffer.from(a); @@ -272,7 +292,8 @@ async function mintTokenPair( }; if (grant.scope) responseBody.scope = grant.scope; - // Only issue a refresh token if the client registered the refresh_token grant. + // Refresh issuance is decided by the caller (shouldIssueRefresh: client + // grant_types, plus the offline_access scope opt-in when configured). if (grant.issueRefresh) { const familyId = newFamilyId(); const { token: refreshToken, hash } = makeRefreshToken(familyId); @@ -376,7 +397,7 @@ async function handleAuthorizationCodeGrant( resource: record.resource, scope: record.scope, clientId: client.client_id, - issueRefresh: allowsRefresh(client), + issueRefresh: shouldIssueRefresh(client, record.scope, mcpConfig), }, hookManager, logger diff --git a/src/lib/mcp/wellKnown.ts b/src/lib/mcp/wellKnown.ts index d564902..746f749 100644 --- a/src/lib/mcp/wellKnown.ts +++ b/src/lib/mcp/wellKnown.ts @@ -136,6 +136,10 @@ export function protectedResourceMetadataUrl(request: HarperRequest, mcpConfig: export function buildProtectedResourceMetadata(request: HarperRequest, mcpConfig: MCPConfig): Record { const resource = resolveResource(request, mcpConfig); const issuer = resolveIssuer(request, mcpConfig); + // No scopes_supported here by design: SEP-2207 — the protected resource + // must not advertise `offline_access` (refresh tokens are not a resource + // requirement), and MCP token scoping is role-level with no fixed resource + // scope vocabulary to publish. return { resource, authorization_servers: [issuer], @@ -171,6 +175,12 @@ export function buildAuthorizationServerMetadata( ...(clientCredentialsEnabled ? ['client_credentials'] : []), ], code_challenge_methods_supported: ['S256'], + // SEP-2207: advertising `offline_access` here signals that clients + // wanting refresh tokens MAY add it to their authorization/token scope + // (the MCP spec conditions that MAY on AS metadata listing it). RFC 8414 + // permits a partial list; upstream-provider scopes are opaque + // pass-through and are deliberately not enumerated. + scopes_supported: ['offline_access'], token_endpoint_auth_methods_supported: [ 'none', 'client_secret_basic', diff --git a/src/types.ts b/src/types.ts index 53e7793..1ab93e7 100644 --- a/src/types.ts +++ b/src/types.ts @@ -105,6 +105,16 @@ export interface MCPConfig { accessTokenTtl?: number; /** Refresh-token (family) lifetime in seconds. Default: 2592000 (30d). */ refreshTokenTtl?: number; + /** + * When true, the authorization_code exchange only issues a refresh token + * when the granted scope includes `offline_access` (the SEP-2207 + * OIDC-flavored opt-in). Default: false — any client whose registered + * grant_types include refresh_token receives one, since most MCP clients + * never request offline_access and withholding refresh tokens would force + * re-auth every accessTokenTtl. Flipping this on does not revoke existing + * refresh-token families. + */ + refreshTokenRequiresOfflineAccess?: boolean; /** * Client ID Metadata Document (CIMD) resolution settings. * CIMD is enabled by default when `mcp.enabled: true`. Override here diff --git a/test/lib/config.test.js b/test/lib/config.test.js index cd84b24..ba9e05f 100644 --- a/test/lib/config.test.js +++ b/test/lib/config.test.js @@ -69,6 +69,14 @@ describe('OAuth Configuration', () => { assert.equal(cfg.enabled, false); assert.equal(cfg.clientIdMetadataDocuments.enabled, false); }); + it('coerces refreshTokenRequiresOfflineAccess (string "false" must not activate the gate)', () => { + const cfg = { refreshTokenRequiresOfflineAccess: 'false' }; + normalizeMcpSecurityConfig(cfg); + assert.equal(cfg.refreshTokenRequiresOfflineAccess, false); + const cfgTrue = { refreshTokenRequiresOfflineAccess: 'true' }; + normalizeMcpSecurityConfig(cfgTrue); + assert.equal(cfgTrue.refreshTokenRequiresOfflineAccess, true); + }); it('coerces clientCredentials.enabled the same way (token-minting switch must not be string-truthy)', () => { const cfg = { clientCredentials: { enabled: 'false' } }; normalizeMcpSecurityConfig(cfg); diff --git a/test/lib/mcp/token.test.js b/test/lib/mcp/token.test.js index 49d6961..65c3a1b 100644 --- a/test/lib/mcp/token.test.js +++ b/test/lib/mcp/token.test.js @@ -251,6 +251,70 @@ describe('handleToken', () => { assert.equal(families.size, 0, 'no refresh family persisted'); }); + // ---- offline_access opt-in (SEP-2207) ---- + + it('withholds the refresh token when refreshTokenRequiresOfflineAccess is set and the scope lacks offline_access', async () => { + seedCode('code-1'); // scope: 'mcp:read' + const res = await handleToken( + { headers: {} }, + { + grant_type: 'authorization_code', + code: 'code-1', + code_verifier: CODE_VERIFIER, + redirect_uri: REDIRECT, + client_id: 'public-1', + }, + { ...mcpConfig, refreshTokenRequiresOfflineAccess: true } + ); + assert.equal(res.status, 200); + assert.ok(res.body.access_token); + assert.equal(res.body.refresh_token, undefined, 'no refresh token without the offline_access opt-in'); + assert.equal(families.size, 0, 'no refresh family persisted'); + }); + + it('issues the refresh token when refreshTokenRequiresOfflineAccess is set and the scope carries offline_access', async () => { + seedCode('code-1', { scope: 'mcp:read offline_access' }); + const res = await handleToken( + { headers: {} }, + { + grant_type: 'authorization_code', + code: 'code-1', + code_verifier: CODE_VERIFIER, + redirect_uri: REDIRECT, + client_id: 'public-1', + }, + { ...mcpConfig, refreshTokenRequiresOfflineAccess: true } + ); + assert.equal(res.status, 200); + assert.ok(res.body.refresh_token, 'offline_access opt-in honored'); + assert.equal(res.body.scope, 'mcp:read offline_access', 'granted scope echoed back'); + assert.equal(families.size, 1, 'refresh family persisted'); + }); + + it('still gates on grant_types even when offline_access is requested', async () => { + clients.set('noref-1', { + client_id: 'noref-1', + token_endpoint_auth_method: 'none', + grant_types: JSON.stringify(['authorization_code']), + redirect_uris: JSON.stringify([REDIRECT]), + client_id_issued_at: 1700000000, + }); + seedCode('code-1', { client_id: 'noref-1', scope: 'offline_access' }); + const res = await handleToken( + { headers: {} }, + { + grant_type: 'authorization_code', + code: 'code-1', + code_verifier: CODE_VERIFIER, + redirect_uri: REDIRECT, + client_id: 'noref-1', + }, + { ...mcpConfig, refreshTokenRequiresOfflineAccess: true } + ); + assert.equal(res.status, 200); + assert.equal(res.body.refresh_token, undefined, 'grant_types gate still applies'); + }); + // ---- authorization_code rejection branches ---- it('rejects when code, code_verifier, or redirect_uri is missing', async () => { diff --git a/test/lib/mcp/wellKnown.test.js b/test/lib/mcp/wellKnown.test.js index 7685c0d..5b46b1b 100644 --- a/test/lib/mcp/wellKnown.test.js +++ b/test/lib/mcp/wellKnown.test.js @@ -85,6 +85,11 @@ describe('MCP well-known: PRM document (RFC 9728)', () => { }); assert.equal(doc.resource, 'https://my-app.example.com/mcp'); }); + + it('does not advertise scopes_supported — offline_access must stay out of PRM (SEP-2207)', () => { + const doc = buildProtectedResourceMetadata(makeRequest(), { enabled: true }); + assert.ok(!('scopes_supported' in doc), 'PRM must not carry scopes_supported'); + }); }); describe('MCP well-known: AS metadata document (RFC 8414)', () => { @@ -170,6 +175,11 @@ describe('MCP well-known: AS metadata document (RFC 8414)', () => { assert.deepEqual(doc.id_token_signing_alg_values_supported, ['RS256']); }); + it('advertises offline_access in scopes_supported (SEP-2207 refresh-token opt-in)', () => { + const doc = buildAuthorizationServerMetadata(makeRequest(), { enabled: true }); + assert.deepEqual(doc.scopes_supported, ['offline_access']); + }); + it('signals RFC 8707 resource-parameter support', () => { const doc = buildAuthorizationServerMetadata(makeRequest(), { enabled: true }); assert.equal(doc.resource_parameter_supported, true); From 160992d4ad82b8cddcbeeb3ff69ffdc7162aff5a Mon Sep 17 00:00:00 2001 From: Nathan Heskew Date: Wed, 22 Jul 2026 07:38:15 -0700 Subject: [PATCH 2/2] Make boolean config normalization total (drop junk values with a warning) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unresolved ${FLAG} placeholder (or any other non-coercible value) on a documented boolean survived normalization as a truthy string and flipped whichever direction the consuming gate tested — e.g. activating the default-off refreshTokenRequiresOfflineAccess gate (caught in review). normalizeMcpSecurityConfig now guarantees every documented boolean (enabled, refreshTokenRequiresOfflineAccess, clientCredentials.enabled, dynamicClientRegistration.enabled, clientIdMetadataDocuments.enabled) is a real boolean or absent: coercible values coerce, anything else is dropped with a startup warning so the documented default applies. Consumers keep gating on plain truthiness. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0185AoLyjuZUuKAwg9NxUDiq --- src/index.ts | 9 +++-- src/lib/config.ts | 76 ++++++++++++++++++++++++++------------ test/lib/config.test.js | 35 ++++++++++++++++++ test/lib/mcp/token.test.js | 23 ++++++++++++ 4 files changed, 116 insertions(+), 27 deletions(-) diff --git a/src/index.ts b/src/index.ts index 579db24..48046a4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -163,10 +163,11 @@ export async function handleApplication(scope: Scope): Promise { // sensitive leaves (mcp.dynamicClientRegistration.initialAccessToken) and // a pinned issuer/resource support ${ENV_VAR}. const mcpConfig = options.mcp ? expandEnvVarsDeep(options.mcp) : undefined; - // Coerce documented boolean strings and normalize allowedHosts BEFORE any - // `enabled` check, so an env-expanded "false" can't leave a security switch - // truthy or turn a scalar allowedHosts into substring matching. - if (mcpConfig) normalizeMcpSecurityConfig(mcpConfig); + // Normalize documented booleans (totally — coerce or drop-with-warning) + // and allowedHosts BEFORE any `enabled` check, so an env-expanded "false" + // or an unresolved "${FLAG}" placeholder can't leave a gate string-truthy, + // and a scalar allowedHosts can't become substring matching. + if (mcpConfig) normalizeMcpSecurityConfig(mcpConfig, logger); if (mcpConfig?.enabled && !mcpConfig.issuer) { // Without a pinned issuer, resolveIssuer() (wellKnown.ts) derives it from // the client-controlled Host header — and resolveResource() defaults to diff --git a/src/lib/config.ts b/src/lib/config.ts index f2ca40d..d041458 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -72,44 +72,74 @@ export function coerceConfigBoolean(value: unknown): boolean | undefined { return undefined; } +/** + * Normalize one documented-boolean config field in place, TOTALLY: after this + * call the field is either a real boolean or absent. Coercible values + * (booleans, "true"/"false" strings) are coerced; anything else present — + * including an unresolved `${ENV_VAR}` placeholder left by expandEnvVarsDeep + * when the variable is unset — is DELETED with a warning, so the field's + * documented default applies. Without this, a truthy junk value silently + * flips whichever direction the consuming gate happens to test (e.g. + * `refreshTokenRequiresOfflineAccess: ${FLAG}` with FLAG unset would activate + * a documented default-off gate). + */ +function normalizeBooleanField(obj: Record, field: string, path: string, logger?: Logger): void { + const value = obj[field]; + if (value === undefined || value === null) return; // Absent (or bare YAML key) — default applies already. + const coerced = coerceConfigBoolean(value); + if (coerced !== undefined) { + obj[field] = coerced; + return; + } + const isUnresolvedPlaceholder = typeof value === 'string' && /^\$\{[^}]*\}$/.test(value.trim()); + logger?.warn?.( + isUnresolvedPlaceholder + ? `MCP: ${path} is the unresolved env placeholder ${JSON.stringify(value)} (variable unset). ` + + 'Treating the option as absent — its documented default applies.' + : `MCP: ${path} must be a boolean; got ${JSON.stringify(value)}. ` + + 'Treating the option as absent — its documented default applies.' + ); + delete obj[field]; +} + /** * Normalize the security-relevant fields of the `mcp` config block in place, - * so a mis-typed value can never silently fail open: - * - `mcp.enabled` and `mcp.clientIdMetadataDocuments.enabled` are coerced from - * documented boolean strings to real booleans (an env-expanded `"false"` - * would otherwise be truthy and enable the feature the operator disabled). + * so a mis-typed value can never silently flip a gate: + * - Every documented boolean (`mcp.enabled`, + * `mcp.refreshTokenRequiresOfflineAccess`, `mcp.clientCredentials.enabled`, + * `mcp.clientIdMetadataDocuments.enabled`, + * `mcp.dynamicClientRegistration.enabled`) is normalized totally via + * {@link normalizeBooleanField}: coerced to a real boolean, or removed with + * a warning so the documented default applies. Consumers may therefore gate + * on plain truthiness / `!== false` without re-validating types. * - `mcp.clientIdMetadataDocuments.allowedHosts` is normalized to an array of * exact, lowercased hostnames. A scalar string (which `Array.includes` / * `String.includes` would turn into substring matching) is wrapped into a * single-element array; anything that isn't a string or array of strings is * rejected rather than treated as "no restriction". - * - `mcp.clientCredentials.enabled` is coerced the same way — this flag mints - * tokens for headless agents, so a stray truthy string must not enable it - * (it is explicit opt-in, default OFF). - * - `mcp.refreshTokenRequiresOfflineAccess` is coerced the same way — a stray - * truthy string would withhold refresh tokens from clients that never - * request `offline_access` (SEP-2207 opt-in, default OFF). */ -export function normalizeMcpSecurityConfig(mcpConfig: Record): void { - const enabled = coerceConfigBoolean(mcpConfig.enabled); - if (enabled !== undefined) mcpConfig.enabled = enabled; - - // An env-expanded "false" is truthy and would activate the offline_access - // gate the operator disabled — withholding refresh tokens from every client - // that didn't request the scope. - const requiresOfflineAccess = coerceConfigBoolean(mcpConfig.refreshTokenRequiresOfflineAccess); - if (requiresOfflineAccess !== undefined) mcpConfig.refreshTokenRequiresOfflineAccess = requiresOfflineAccess; +export function normalizeMcpSecurityConfig(mcpConfig: Record, logger?: Logger): void { + normalizeBooleanField(mcpConfig, 'enabled', 'mcp.enabled', logger); + normalizeBooleanField( + mcpConfig, + 'refreshTokenRequiresOfflineAccess', + 'mcp.refreshTokenRequiresOfflineAccess', + logger + ); const clientCredentials = mcpConfig.clientCredentials; if (clientCredentials && typeof clientCredentials === 'object') { - const ccEnabled = coerceConfigBoolean(clientCredentials.enabled); - if (ccEnabled !== undefined) clientCredentials.enabled = ccEnabled; + normalizeBooleanField(clientCredentials, 'enabled', 'mcp.clientCredentials.enabled', logger); + } + + const dcr = mcpConfig.dynamicClientRegistration; + if (dcr && typeof dcr === 'object') { + normalizeBooleanField(dcr, 'enabled', 'mcp.dynamicClientRegistration.enabled', logger); } const cimd = mcpConfig.clientIdMetadataDocuments; if (cimd && typeof cimd === 'object') { - const cimdEnabled = coerceConfigBoolean(cimd.enabled); - if (cimdEnabled !== undefined) cimd.enabled = cimdEnabled; + normalizeBooleanField(cimd, 'enabled', 'mcp.clientIdMetadataDocuments.enabled', logger); if (cimd.allowedHosts !== undefined) { const raw = Array.isArray(cimd.allowedHosts) ? cimd.allowedHosts : [cimd.allowedHosts]; diff --git a/test/lib/config.test.js b/test/lib/config.test.js index ba9e05f..83b6bab 100644 --- a/test/lib/config.test.js +++ b/test/lib/config.test.js @@ -77,6 +77,41 @@ describe('OAuth Configuration', () => { normalizeMcpSecurityConfig(cfgTrue); assert.equal(cfgTrue.refreshTokenRequiresOfflineAccess, true); }); + it('drops an unresolved "${FLAG}" placeholder so a documented-off gate stays off, and warns', () => { + // expandEnvVarsDeep leaves "${FLAG}" intact when FLAG is unset; that + // string is truthy and must not activate the gate (PR #192 review). + const warnings = []; + const logger = { warn: (...args) => warnings.push(args.join(' ')) }; + const cfg = { refreshTokenRequiresOfflineAccess: '${FLAG}' }; + normalizeMcpSecurityConfig(cfg, logger); + assert.equal(cfg.refreshTokenRequiresOfflineAccess, undefined, 'placeholder dropped — default applies'); + assert.equal(warnings.length, 1); + assert.match(warnings[0], /unresolved env placeholder/); + }); + it('drops any other non-boolean value with a warning (total normalization)', () => { + const warnings = []; + const logger = { warn: (...args) => warnings.push(args.join(' ')) }; + const cfg = { + enabled: 'yes', + refreshTokenRequiresOfflineAccess: 1, + clientCredentials: { enabled: {} }, + dynamicClientRegistration: { enabled: 'on' }, + clientIdMetadataDocuments: { enabled: 'nope' }, + }; + normalizeMcpSecurityConfig(cfg, logger); + assert.equal(cfg.enabled, undefined); + assert.equal(cfg.refreshTokenRequiresOfflineAccess, undefined); + assert.equal(cfg.clientCredentials.enabled, undefined); + assert.equal(cfg.dynamicClientRegistration.enabled, undefined); + assert.equal(cfg.clientIdMetadataDocuments.enabled, undefined); + assert.equal(warnings.length, 5, 'one warning per dropped field'); + assert.ok(warnings.every((w) => /must be a boolean/.test(w))); + }); + it('normalizes dynamicClientRegistration.enabled ("false" string must actually disable DCR)', () => { + const cfg = { dynamicClientRegistration: { enabled: 'false' } }; + normalizeMcpSecurityConfig(cfg); + assert.equal(cfg.dynamicClientRegistration.enabled, false); + }); it('coerces clientCredentials.enabled the same way (token-minting switch must not be string-truthy)', () => { const cfg = { clientCredentials: { enabled: 'false' } }; normalizeMcpSecurityConfig(cfg); diff --git a/test/lib/mcp/token.test.js b/test/lib/mcp/token.test.js index 65c3a1b..2a5d04f 100644 --- a/test/lib/mcp/token.test.js +++ b/test/lib/mcp/token.test.js @@ -15,6 +15,7 @@ import { resetMCPClientsTableCache } from '../../../dist/lib/mcp/clientStore.js' import { resetMCPKeysTableCache, SIGNING_KEY_ID } from '../../../dist/lib/mcp/keyStore.js'; import { resetMCPRefreshFamiliesTableCache, makeRefreshToken } from '../../../dist/lib/mcp/refreshTokenStore.js'; import { verifyAccessToken } from '../../../dist/lib/mcp/tokenIssuer.js'; +import { normalizeMcpSecurityConfig } from '../../../dist/lib/config.js'; function asTrackedObject(plain) { return new Proxy(plain, { @@ -291,6 +292,28 @@ describe('handleToken', () => { assert.equal(families.size, 1, 'refresh family persisted'); }); + it('an unresolved "${FLAG}" placeholder does not activate the gate once config is normalized (PR #192 review)', async () => { + // The startup path always runs normalizeMcpSecurityConfig before any + // handler sees the config; this pins the composed behavior — the truthy + // placeholder string is dropped, so the default (issue refresh) applies. + const cfg = { ...mcpConfig, refreshTokenRequiresOfflineAccess: '${FLAG}' }; + normalizeMcpSecurityConfig(cfg); + seedCode('code-1'); // scope: 'mcp:read' — no offline_access + const res = await handleToken( + { headers: {} }, + { + grant_type: 'authorization_code', + code: 'code-1', + code_verifier: CODE_VERIFIER, + redirect_uri: REDIRECT, + client_id: 'public-1', + }, + cfg + ); + assert.equal(res.status, 200); + assert.ok(res.body.refresh_token, 'refresh token issued — documented default-off gate stayed off'); + }); + it('still gates on grant_types even when offline_access is requested', async () => { clients.set('noref-1', { client_id: 'noref-1',