From f67ce0f7e78c6a2a804fe1c819274948177aa87e Mon Sep 17 00:00:00 2001 From: andypalmi Date: Mon, 24 Aug 2026 19:47:09 +0200 Subject: [PATCH 01/14] feat(mcp): add .well-known OAuth discovery endpoints Add RFC 8414 authorization-server metadata and RFC 9728 protected-resource metadata under /.well-known so MCP clients can auto-discover the OAuth endpoints and the MCP resource URL. Public, license-tier independent. Ref FlowFuse/flowfuse#7431 --- forge/routes/index.js | 1 + forge/routes/wellKnown.js | 59 +++++++++++++++++++++ test/unit/forge/routes/wellKnown_spec.js | 65 ++++++++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 forge/routes/wellKnown.js create mode 100644 test/unit/forge/routes/wellKnown_spec.js diff --git a/forge/routes/index.js b/forge/routes/index.js index 4679deae1f..3b62a74498 100644 --- a/forge/routes/index.js +++ b/forge/routes/index.js @@ -77,6 +77,7 @@ module.exports = fp(async function (app, opts) { await app.register(require('@fastify/websocket')) await app.register(require('./auth'), { logLevel: app.config.logging.http }) await app.register(require('./api'), { prefix: '/api/v1', logLevel: app.config.logging.http }) + await app.register(require('./wellKnown'), { prefix: '/.well-known', logLevel: app.config.logging.http }) await app.register(require('./ui'), { logLevel: app.config.logging.http }) await app.register(require('./setup'), { logLevel: app.config.logging.http }) await app.register(require('./storage'), { prefix: '/storage', logLevel: app.config.logging.http }) diff --git a/forge/routes/wellKnown.js b/forge/routes/wellKnown.js new file mode 100644 index 0000000000..3c0f4c0a5c --- /dev/null +++ b/forge/routes/wellKnown.js @@ -0,0 +1,59 @@ +module.exports = async function (app) { + // RFC 8414: OAuth 2.0 Authorization Server Metadata + app.get('/oauth-authorization-server', { + config: { allowAnonymous: true }, + schema: { + tags: ['Authentication', 'X-HIDDEN'], + response: { + 200: { + type: 'object', + properties: { + issuer: { type: 'string' }, + authorization_endpoint: { type: 'string' }, + token_endpoint: { type: 'string' }, + response_types_supported: { type: 'array', items: { type: 'string' } }, + grant_types_supported: { type: 'array', items: { type: 'string' } }, + code_challenge_methods_supported: { type: 'array', items: { type: 'string' } }, + token_endpoint_auth_methods_supported: { type: 'array', items: { type: 'string' } }, + registration_endpoint: { type: 'string' } + } + } + } + } + }, async (request, reply) => { + const baseUrl = app.config.base_url + reply.send({ + issuer: baseUrl, + authorization_endpoint: `${baseUrl}/account/authorize`, + token_endpoint: `${baseUrl}/account/token`, + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256'], + token_endpoint_auth_methods_supported: ['none'], + registration_endpoint: `${baseUrl}/account/client` + }) + }) + + // RFC 9728: OAuth 2.0 Protected Resource Metadata + app.get('/oauth-protected-resource', { + config: { allowAnonymous: true }, + schema: { + tags: ['Authentication', 'X-HIDDEN'], + response: { + 200: { + type: 'object', + properties: { + resource: { type: 'string' }, + authorization_servers: { type: 'array', items: { type: 'string' } } + } + } + } + } + }, async (request, reply) => { + const baseUrl = app.config.base_url + reply.send({ + resource: `${baseUrl}/mcp`, + authorization_servers: [baseUrl] + }) + }) +} diff --git a/test/unit/forge/routes/wellKnown_spec.js b/test/unit/forge/routes/wellKnown_spec.js new file mode 100644 index 0000000000..d0bbd37979 --- /dev/null +++ b/test/unit/forge/routes/wellKnown_spec.js @@ -0,0 +1,65 @@ +const should = require('should') // eslint-disable-line no-unused-vars + +const setup = require('./setup') + +describe('.well-known OAuth discovery', function () { + let app + const baseUrl = 'http://localhost:3000' + + before(async function () { + app = await setup({ base_url: baseUrl }) + }) + + after(async function () { + await app.close() + }) + + describe('GET /.well-known/oauth-authorization-server (RFC 8414)', function () { + let body + + before(async function () { + const response = await app.inject({ method: 'GET', url: '/.well-known/oauth-authorization-server' }) + response.statusCode.should.equal(200) + body = response.json() + }) + + it('advertises the issuer and endpoints derived from base_url', function () { + body.should.have.property('issuer', baseUrl) + body.should.have.property('authorization_endpoint', `${baseUrl}/account/authorize`) + body.should.have.property('token_endpoint', `${baseUrl}/account/token`) + body.should.have.property('registration_endpoint', `${baseUrl}/account/client`) + }) + + it('advertises the authorization code and refresh token grants', function () { + body.response_types_supported.should.containEql('code') + body.grant_types_supported.should.containDeep(['authorization_code', 'refresh_token']) + }) + + it('advertises PKCE S256 and public clients (no secret)', function () { + body.code_challenge_methods_supported.should.eql(['S256']) + body.token_endpoint_auth_methods_supported.should.eql(['none']) + }) + }) + + describe('GET /.well-known/oauth-protected-resource (RFC 9728)', function () { + let body + + before(async function () { + const response = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource' }) + response.statusCode.should.equal(200) + body = response.json() + }) + + it('advertises the MCP resource and its authorization server', function () { + body.should.have.property('resource', `${baseUrl}/mcp`) + body.authorization_servers.should.eql([baseUrl]) + }) + }) + + it('serves both documents anonymously, without a session', async function () { + const authServer = await app.inject({ method: 'GET', url: '/.well-known/oauth-authorization-server' }) + const resource = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource' }) + authServer.statusCode.should.equal(200) + resource.statusCode.should.equal(200) + }) +}) From 6747cbd6d3369669ae4f54792fbccec4227003b7 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Mon, 24 Aug 2026 19:47:54 +0200 Subject: [PATCH 02/14] feat(mcp): OAuth2 PKCE flow with dynamic client registration for MCP agents Extend the OAuth2 flow so external MCP agents (Claude, Cursor, etc.) can authenticate. Rather than a single hardcoded client id, agents register dynamically per RFC 7591: POST /account/client persists a public AuthClient (type 'mcp', no secret) with its approved redirect URIs and returns a generated client id. The authorize, complete, and token endpoints recognise these clients by looking them up, skip the project/device ownership checks (MCP is user-scoped), and drive an MCP consent step that records the read-only and team selection before issuing a scoped personal access token. Redirect URIs must be loopback http (RFC 8252, port-flexible) or https for hosted clients; token issuance and refresh require no client secret. Also forward the caller scope through the platform automation handler and add a platform_get_active_user tool that reports the calling token's scope. Adds AuthClient.type/name/redirectURIs (migration + model), an AuthClient.createMCPClient controller, and AccessToken.createMCPOAuthToken. Ref FlowFuse/flowfuse#7432 --- forge/comms/platformAutomation.js | 10 +- forge/db/controllers/AccessToken.js | 31 ++ forge/db/controllers/AuthClient.js | 14 + .../20260824-01-add-mcp-authclient-fields.js | 39 +++ forge/db/models/AuthClient.js | 15 +- forge/ee/lib/mcp/tools/users.js | 34 ++ forge/routes/auth/oauth.js | 297 ++++++++++++++---- .../forge/comms/platformAutomation_spec.js | 39 ++- .../unit/forge/ee/lib/mcp/tools/users_spec.js | 54 ++++ test/unit/forge/routes/auth/oauth_spec.js | 157 +++++++++ 10 files changed, 620 insertions(+), 70 deletions(-) create mode 100644 forge/db/migrations/20260824-01-add-mcp-authclient-fields.js create mode 100644 forge/ee/lib/mcp/tools/users.js create mode 100644 test/unit/forge/ee/lib/mcp/tools/users_spec.js diff --git a/forge/comms/platformAutomation.js b/forge/comms/platformAutomation.js index 1dc14fe6d8..9a054d4f93 100644 --- a/forge/comms/platformAutomation.js +++ b/forge/comms/platformAutomation.js @@ -104,7 +104,7 @@ class PlatformAutomationHandler { this.client.on('request/platform-automation:forge', this.eventHandler) } - eventHandler = async ({ userId, mcpSessionId, command, data, meta } = {}, onSuccess, onError) => { + eventHandler = async ({ userId, mcpSessionId, command, data, meta, scope } = {}, onSuccess, onError) => { try { let result = {} this.app.log.info(`platform-automation request: userId=${userId} mcpSessionId=${mcpSessionId} command=${command} tool=${data?.name || 'n/a'}`) @@ -123,6 +123,12 @@ class PlatformAutomationHandler { const toolName = data?.name const args = data?.input || {} + // The caller scope (readOnly plus any team restriction from the + // session token) rides with the request so tools can report and + // enforce what the session is allowed to do. Prefer the top-level + // value, falling back to meta for callers that attach it there. + const callerScope = scope ?? meta?.scope + // TODO: Probably sensible to verify that toolDefinition matches the tool to ensure no tampering has occurred const { toolDefinition } = meta || {} @@ -163,7 +169,7 @@ class PlatformAutomationHandler { } const { formatResponse } = require('../ee/lib/mcp/toolLoader') - const response = await tool.handler(args, { inject, app: this.app, user, mcpSessionId }) + const response = await tool.handler(args, { inject, app: this.app, user, mcpSessionId, scope: callerScope }) result = formatResponse(response) } break diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index 38111550a4..064e83d1df 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -265,6 +265,37 @@ module.exports = { await app.settings.set('platform:stats:token', false) }, + createMCPOAuthToken: async function (app, userId, { readOnly = false, teamIds = [] } = {}) { + const token = generateToken(32, 'ffpat') + const refreshToken = generateToken(32, 'ffpat') + const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY + + await app.db.sequelize.transaction(async (t) => { + const tok = await app.db.models.AccessToken.create({ + name: 'MCP Agent', + token, + refreshToken, + scope: '', + expiresAt, + readOnly, + adminOptIn: false, + ownerId: '' + userId, + ownerType: 'user' + }, { transaction: t }) + + if (teamIds.length > 0) { + const scopes = teamIds.map(teamId => ({ + AccessTokenId: tok.id, + TeamId: app.db.models.Team.decodeHashid(teamId), + UserId: userId + })) + await app.db.models.AccessTokenTeamScope.bulkCreate(scopes, { transaction: t }) + } + }) + + return { token, expiresAt, refreshToken } + }, + createPersonalAccessToken: async function (app, user, scope, expiresAt, name, { readOnly = false, adminOptIn = false, teamIds = [] } = {}) { const userId = typeof user === 'number' ? user : user.id const token = generateToken(32, 'ffpat') diff --git a/forge/db/controllers/AuthClient.js b/forge/db/controllers/AuthClient.js index ede14db8d1..adb0d06cca 100644 --- a/forge/db/controllers/AuthClient.js +++ b/forge/db/controllers/AuthClient.js @@ -42,6 +42,20 @@ module.exports = { return client }, + /** + * Register a public MCP client (RFC 7591 Dynamic Client Registration). + * MCP clients have no owner and no secret - they authenticate with PKCE. + * The generated clientID is the only credential returned to the client. + */ + createMCPClient: async function (app, { name, redirectURIs } = {}) { + return app.db.models.AuthClient.create({ + clientID: generateToken(32, 'ffmcp'), + type: 'mcp', + name: name || 'MCP Agent', + redirectURIs: redirectURIs || [] + }) + }, + removeClientForDevice: async function (app, device) { const existingAuthClient = await device.getAuthClient() if (existingAuthClient) { diff --git a/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js b/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js new file mode 100644 index 0000000000..df7fa920b8 --- /dev/null +++ b/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js @@ -0,0 +1,39 @@ +/** + * Add fields to AuthClients so MCP agents can register dynamically (RFC 7591). + * + * Existing clients (project/device editor auth) are owned by a resource via + * ownerType/ownerId and authenticate with a clientSecret. MCP clients have no + * owner and are public (PKCE, no secret), so they need somewhere to record the + * client type, a display name, and the redirect URIs approved at registration. + * + * type - 'mcp' for dynamically registered MCP clients, null otherwise + * name - the client_name supplied at registration + * redirectURIs - JSON array of redirect URIs the client may use + */ + +const { DataTypes } = require('sequelize') + +module.exports = { + up: async (context) => { + await context.addColumn('AuthClients', 'type', { + type: DataTypes.STRING, + allowNull: true, + defaultValue: null + }) + await context.addColumn('AuthClients', 'name', { + type: DataTypes.STRING, + allowNull: true, + defaultValue: null + }) + await context.addColumn('AuthClients', 'redirectURIs', { + type: DataTypes.TEXT, + allowNull: true, + defaultValue: null + }) + }, + down: async (context) => { + await context.removeColumn('AuthClients', 'type') + await context.removeColumn('AuthClients', 'name') + await context.removeColumn('AuthClients', 'redirectURIs') + } +} diff --git a/forge/db/models/AuthClient.js b/forge/db/models/AuthClient.js index dfbb602b1a..17def3bb02 100644 --- a/forge/db/models/AuthClient.js +++ b/forge/db/models/AuthClient.js @@ -18,7 +18,20 @@ module.exports = { } }, ownerId: { type: DataTypes.STRING }, - ownerType: { type: DataTypes.STRING } + ownerType: { type: DataTypes.STRING }, + // 'mcp' for dynamically registered MCP clients (public, no secret), null otherwise + type: { type: DataTypes.STRING, allowNull: true }, + name: { type: DataTypes.STRING, allowNull: true }, + redirectURIs: { + type: DataTypes.TEXT, + get () { + const rawValue = this.getDataValue('redirectURIs') + return rawValue ? JSON.parse(rawValue) : [] + }, + set (value) { + this.setDataValue('redirectURIs', JSON.stringify(value || [])) + } + } }, associations: function (M) { this.belongsTo(M.Project, { foreignKey: 'ownerId', constraints: false }) diff --git a/forge/ee/lib/mcp/tools/users.js b/forge/ee/lib/mcp/tools/users.js new file mode 100644 index 0000000000..01df71d48f --- /dev/null +++ b/forge/ee/lib/mcp/tools/users.js @@ -0,0 +1,34 @@ +module.exports = [ + { + name: 'platform_get_active_user', + title: 'Get Active User', + description: `FlowFuse platform automation tool: + Get the profile of the user this MCP session is authenticated as, along with what the session's token is allowed to do. + Returns the user's ID (hashid), username, name, email and admin flag, plus a token object: + token.readOnly - when true, write and delete tools are rejected, so only read tools can be used. + token.allTeams - when true, the token is not restricted to a subset of teams and every team the user belongs to is in reach. + token.teams - when allTeams is false, the IDs of the only teams the token may act on. Calls against any other team will fail. + Use this to resolve the current user's ID before calling tools that need a userId, such as listing browser sessions, + and to check up front whether an action the user asked for is within the session's access.`, + annotations: { readOnlyHint: true, destructiveHint: false }, + inputSchema: {}, + handler: async (args, { inject, scope }) => { + const response = await inject({ method: 'GET', url: '/api/v1/user' }) + if (response.statusCode >= 400) { + return response + } + + // No scope means the tool was not called through a scoped token (the FlowFuse + // Expert path), so the session has the user's full access. + const teams = Array.isArray(scope?.teams) ? scope.teams : [] + return { + ...response.json(), + token: { + readOnly: scope?.readOnly === true, + allTeams: teams.length === 0, + teams + } + } + } + } +] diff --git a/forge/routes/auth/oauth.js b/forge/routes/auth/oauth.js index df128ee715..bfb3a04cd6 100644 --- a/forge/routes/auth/oauth.js +++ b/forge/routes/auth/oauth.js @@ -14,6 +14,37 @@ function badRequest (reply, error, description) { }) } +// A loopback redirect_uri may vary its port between registration and use +// (RFC 8252 Section 7.3), and localhost/127.0.0.1 are interchangeable. Every +// other redirect must match a registered value exactly. +function isLoopbackHost (hostname) { + return hostname === 'localhost' || hostname === '127.0.0.1' +} + +function redirectUriMatches (registeredURIs, requested) { + if (registeredURIs.includes(requested)) { + return true + } + let req + try { + req = new URL(requested) + } catch (err) { + return false + } + if (!isLoopbackHost(req.hostname)) { + return false + } + return registeredURIs.some((uri) => { + let reg + try { + reg = new URL(uri) + } catch (err) { + return false + } + return isLoopbackHost(reg.hostname) && reg.protocol === req.protocol && reg.pathname === req.pathname + }) +} + function redirectInvalidRequest (reply, redirectURI, error, errorDescription, state) { const responseUrl = new URL(redirectURI) const response = { error, errorDescription } @@ -59,7 +90,7 @@ module.exports = async function (app) { code_challenge_method: { type: 'string' } }, // client_id and redirect_uri are handled manually - required: ['response_type', 'scope', 'code_challenge', 'code_challenge_method'] + required: ['response_type', 'code_challenge', 'code_challenge_method'] } }, attachValidation: true @@ -88,25 +119,8 @@ module.exports = async function (app) { } catch (err) { return badRequest(reply, 'invalid_request', 'Invalid redirect_uri') } - if (client_id !== 'ff-plugin') { - // Check client_id is valid. Note - no client_secret provided at this point - const authClient = await app.db.controllers.AuthClient.getAuthClient(client_id) - if (!authClient) { - return badRequest(reply, 'invalid_request', 'Invalid client_id') - } - // Ensure redirect_uri path component is correct - if ( - // HTTP Auth callback - !/\/_ffAuth\/callback$/.test(redirectURI.pathname) && - // Admin Auth callback - !/\/auth\/strategy\/callback$/.test(redirectURI.pathname) - ) { - return badRequest(reply, 'invalid_request', 'Invalid redirect_uri') - } - if (!/^(editor($|-))|httpAuth-/.test(scope)) { - return redirectInvalidRequest(reply, redirect_uri, 'invalid_request', "Invalid scope '" + scope + "'. Only 'editor[-version]' is supported", state) - } - } else { + let isMCP = false + if (client_id === 'ff-plugin') { // Ensure redirect_uri path component is correct for the tools plugin if (!/\/(flow(fuse|forge)-nr-tools|nr-assistant)\/auth\/callback$/.test(redirectURI.pathname)) { return badRequest(reply, 'invalid_request', 'Invalid redirect_uri') @@ -114,6 +128,34 @@ module.exports = async function (app) { if (scope !== 'ff-plugin' && scope !== 'ff-assistant') { return redirectInvalidRequest(reply, redirect_uri, 'invalid_request', "Invalid scope '" + scope + "'", state) } + } else { + const authClient = await app.db.controllers.AuthClient.getAuthClient(client_id) + if (!authClient) { + return badRequest(reply, 'invalid_request', 'Invalid client_id') + } + if (authClient.type === 'mcp') { + // MCP agent (dynamically registered): redirect_uri must match one + // approved at registration. Scope is not validated here - the access + // level is chosen by the user on the consent page (readOnly + teamIds). + if (!redirectUriMatches(authClient.redirectURIs, redirect_uri)) { + return badRequest(reply, 'invalid_request', 'Invalid redirect_uri') + } + isMCP = true + } else { + // Dynamic client (project/device editor auth) + // Ensure redirect_uri path component is correct + if ( + // HTTP Auth callback + !/\/_ffAuth\/callback$/.test(redirectURI.pathname) && + // Admin Auth callback + !/\/auth\/strategy\/callback$/.test(redirectURI.pathname) + ) { + return badRequest(reply, 'invalid_request', 'Invalid redirect_uri') + } + if (!/^(editor($|-))|httpAuth-/.test(scope)) { + return redirectInvalidRequest(reply, redirect_uri, 'invalid_request', "Invalid scope '" + scope + "'. Only 'editor[-version]' is supported", state) + } + } } // If anything else missing, redirect with details if (request.validationError) { @@ -137,7 +179,8 @@ module.exports = async function (app) { redirect_uri, state, code_challenge, - code_challenge_method + code_challenge_method, + mcp: isMCP } const requestId = base64URLEncode(crypto.randomBytes(32)) await requestCache.set(requestId, requestObject) @@ -159,6 +202,11 @@ module.exports = async function (app) { reply.redirect(`${app.config.base_url}/account/request/${requestId}/editor`) return } + if (isMCP) { + // Redirect to MCP-specific consent page + reply.redirect(`${app.config.base_url}/account/request/${requestId}/mcp`) + return + } // Redirect to login page with requestId in url - to bounce to an approve page reply.redirect(`${app.config.base_url}/account/request/${requestId}`) }) @@ -176,8 +224,8 @@ module.exports = async function (app) { if (request.sid) { request.session = await app.db.controllers.Session.getOrExpire(request.sid) if (request.session) { - if (requestObject.client_id === 'ff-plugin') { - // This is the FlowFuse Node-RED plugin. + if (requestObject.client_id === 'ff-plugin' || requestObject.mcp) { + // FlowFuse Node-RED plugin or MCP agent: user-scoped, no resource ownership checks } else { const authClient = await app.db.controllers.AuthClient.getAuthClient(requestObject.client_id) if (!authClient) { @@ -267,6 +315,106 @@ module.exports = async function (app) { return redirectInvalidRequest(reply, requestObject.redirect_uri, 'access_denied', 'Access Denied', requestObject.state) }) + // RFC 7591: Dynamic Client Registration for MCP agents + app.post('/account/client', { + config: { allowAnonymous: true }, + schema: { + tags: ['Authentication', 'X-HIDDEN'], + body: { + type: 'object', + properties: { + redirect_uris: { type: 'array', items: { type: 'string' } }, + client_name: { type: 'string' }, + grant_types: { type: 'array', items: { type: 'string' } }, + response_types: { type: 'array', items: { type: 'string' } }, + token_endpoint_auth_method: { type: 'string' } + }, + required: ['redirect_uris'] + } + } + }, async function (request, reply) { + const { redirect_uris, client_name, grant_types, response_types } = request.body + + if (!Array.isArray(redirect_uris) || redirect_uris.length === 0) { + return badRequest(reply, 'invalid_redirect_uri', 'At least one redirect_uri is required') + } + // A redirect_uri must either be a loopback http address (local dev tools, + // RFC 8252 Section 7.3) or a secure https address (a hosted client, e.g. a + // provider connector). Anything else is rejected. + for (const uri of redirect_uris) { + let parsed + try { + parsed = new URL(uri) + } catch (err) { + return badRequest(reply, 'invalid_redirect_uri', `Invalid redirect_uri: ${uri}`) + } + const loopbackHttp = isLoopbackHost(parsed.hostname) && parsed.protocol === 'http:' + const secure = parsed.protocol === 'https:' + if (!loopbackHttp && !secure) { + return badRequest(reply, 'invalid_redirect_uri', 'redirect_uri must be a loopback http address or an https address') + } + } + + const client = await app.db.controllers.AuthClient.createMCPClient({ + name: client_name, + redirectURIs: redirect_uris + }) + + // MCP clients are public (PKCE, no secret), so no client_secret is issued. + reply.code(201).send({ + client_id: client.clientID, + client_id_issued_at: Math.floor(client.createdAt.getTime() / 1000), + client_name: client.name, + redirect_uris, + grant_types: grant_types || ['authorization_code', 'refresh_token'], + response_types: response_types || ['code'], + token_endpoint_auth_method: 'none' + }) + }) + + // MCP consent: save the user's access choices before they approve + app.put('/account/authorize/:id/consent', { + preHandler: (request, reply) => app.verifySession(request, reply), + schema: { + tags: ['Authentication', 'X-HIDDEN'], + params: { + type: 'object', + properties: { + id: { type: 'string' } + }, + required: ['id'] + }, + body: { + type: 'object', + properties: { + readOnly: { type: 'boolean' }, + teamIds: { type: 'array', items: { type: 'string' } } + } + } + } + }, async function (request, reply) { + const requestId = request.params.id + const { readOnly = false, teamIds = [] } = request.body + + const session = await app.db.models.OAuthSession.findOne({ where: { id: requestId } }) + if (!session) { + return badRequest(reply, 'invalid_request', 'Invalid or expired request') + } + if (Date.now() - session.createdAt.getTime() >= 1000 * 60 * 5) { + await session.destroy() + return badRequest(reply, 'invalid_request', 'Request has expired') + } + const requestObject = session.value + if (!requestObject.mcp) { + return badRequest(reply, 'invalid_request', 'Invalid request') + } + + session.value = { ...requestObject, readOnly, teamIds } + await session.save() + + reply.send({ status: 'ok' }) + }) + app.post('/account/token', { config: { rateLimit: false // never rate limit this route @@ -342,7 +490,56 @@ module.exports = async function (app) { return } - if (client_id !== 'ff-plugin') { + if (client_id === 'ff-plugin') { + const scope = { + 'ff-plugin': [ + 'user:read', + 'user:team:list', + 'team:read', + 'team:projects:list', + 'project:read', + 'project:snapshot:list', + 'project:snapshot:create', + 'device:snapshot:list', + 'device:snapshot:create' + ], + 'ff-assistant': [ + 'user:read', + 'assistant:call' + ] + }[requestObject.scope] + if (!scope) { + return badRequest(reply, 'access_denied', 'Access Denied') + } + const accessToken = await app.db.controllers.AccessToken.createTokenForUser(requestObject.userId, + null, + scope, + true + ) + const response = { + access_token: accessToken.token, + expires_in: Math.floor((accessToken.expiresAt - Date.now()) / 1000), + refresh_token: accessToken.refreshToken, + state: requestObject.state + } + reply.send(response) + } else if (requestObject.mcp) { + const accessToken = await app.db.controllers.AccessToken.createMCPOAuthToken( + requestObject.userId, + { + readOnly: requestObject.readOnly || false, + teamIds: requestObject.teamIds || [] + } + ) + const response = { + access_token: accessToken.token, + token_type: 'bearer', + expires_in: Math.floor((accessToken.expiresAt - Date.now()) / 1000), + refresh_token: accessToken.refreshToken, + state: requestObject.state + } + reply.send(response) + } else { const authClient = await app.db.controllers.AuthClient.getAuthClient(client_id, client_secret) if (!authClient) { return badRequest(reply, 'invalid_request', 'Invalid client_id') @@ -404,39 +601,6 @@ module.exports = async function (app) { scope } reply.send(response) - } else { - const scope = { - 'ff-plugin': [ - 'user:read', - 'user:team:list', - 'team:read', - 'team:projects:list', - 'project:read', - 'project:snapshot:list', - 'project:snapshot:create', - 'device:snapshot:list', - 'device:snapshot:create' - ], - 'ff-assistant': [ - 'user:read', - 'assistant:call' - ] - }[requestObject.scope] - if (!scope) { - return badRequest(reply, 'access_denied', 'Access Denied') - } - const accessToken = await app.db.controllers.AccessToken.createTokenForUser(requestObject.userId, - null, - scope, - true - ) - const response = { - access_token: accessToken.token, - expires_in: Math.floor((accessToken.expiresAt - Date.now()) / 1000), - refresh_token: accessToken.refreshToken, - state: requestObject.state - } - reply.send(response) } } else if (grant_type === 'refresh_token') { const existingToken = await app.db.models.AccessToken.byRefreshToken(refresh_token) @@ -444,23 +608,26 @@ module.exports = async function (app) { badRequest(reply, 'invalid_request', 'Invalid refresh_token') return } + // ff-plugin and MCP clients are user-scoped; only project/device + // clients need their resource ownership re-checked on refresh. + let refreshAuthClient = null if (client_id !== 'ff-plugin') { - const authClient = await app.db.controllers.AuthClient.getAuthClient(client_id, client_secret) - if (!authClient) { + refreshAuthClient = await app.db.controllers.AuthClient.getAuthClient(client_id, client_secret) + if (!refreshAuthClient) { return badRequest(reply, 'invalid_request', 'Invalid client_id') } - // We have validated client_id and client_secret by this point. - + } + if (refreshAuthClient && refreshAuthClient.type !== 'mcp') { // Check the owner of the existing session still has access to the project // this client is owned by let owner = null let applicationId - if (authClient.ownerType === 'project') { - owner = await app.db.models.Project.byId(authClient.ownerId) + if (refreshAuthClient.ownerType === 'project') { + owner = await app.db.models.Project.byId(refreshAuthClient.ownerId) // Project.byId will include the full Application object applicationId = owner?.Application.hashid - } else if (authClient.ownerType === 'device') { - owner = await app.db.models.Device.byId(parseInt(authClient.ownerId)) + } else if (refreshAuthClient.ownerType === 'device') { + owner = await app.db.models.Device.byId(parseInt(refreshAuthClient.ownerId)) // Device.byId does not include the full Application object if (owner?.ApplicationId) { applicationId = app.db.models.Application.encodeHashid(owner.ApplicationId) diff --git a/test/unit/forge/comms/platformAutomation_spec.js b/test/unit/forge/comms/platformAutomation_spec.js index 01972422ad..a24bfd7f3b 100644 --- a/test/unit/forge/comms/platformAutomation_spec.js +++ b/test/unit/forge/comms/platformAutomation_spec.js @@ -24,7 +24,7 @@ describe('PlatformAutomationHandler', function () { } } - function invokeToolCall ({ userId, toolName, args, meta }) { + function invokeToolCall ({ userId, toolName, args, meta, scope }) { return new Promise((resolve) => { const onSuccess = (result) => resolve({ ok: true, result }) const onError = (message, code, err) => resolve({ ok: false, message, code, err }) @@ -33,7 +33,8 @@ describe('PlatformAutomationHandler', function () { userId, command: 'mcp-call-tool', data: { name: toolName, input: args || {} }, - meta + meta, + scope }, onSuccess, onError @@ -192,6 +193,40 @@ describe('PlatformAutomationHandler', function () { }) }) + describe('caller scope', function () { + function callActiveUser (message) { + const tool = handler.findTool('platform_get_active_user') + return invokeToolCall({ + ...message, + userId: app.adminUser.hashid, + toolName: 'platform_get_active_user', + meta: { toolDefinition: { annotations: tool.annotations }, ...message.meta } + }) + } + + it('passes the caller scope through to the tool handler', async function () { + const res = await callActiveUser({ scope: { readOnly: true, teams: [app.team.hashid] } }) + + res.ok.should.be.true() + res.result.should.have.property('username', app.adminUser.username) + res.result.token.should.eql({ readOnly: true, allTeams: false, teams: [app.team.hashid] }) + }) + + it('accepts the caller scope on meta', async function () { + const res = await callActiveUser({ meta: { scope: { readOnly: true, teams: [] } } }) + + res.ok.should.be.true() + res.result.token.should.eql({ readOnly: true, allTeams: true, teams: [] }) + }) + + it('treats a call with no scope as full user access', async function () { + const res = await callActiveUser({}) + + res.ok.should.be.true() + res.result.token.should.eql({ readOnly: false, allTeams: true, teams: [] }) + }) + }) + describe('mcp-get-features', function () { it('returns the tool list along with a catalogHash', async function () { const res = await invokeGetFeatures() diff --git a/test/unit/forge/ee/lib/mcp/tools/users_spec.js b/test/unit/forge/ee/lib/mcp/tools/users_spec.js new file mode 100644 index 0000000000..f832a1febd --- /dev/null +++ b/test/unit/forge/ee/lib/mcp/tools/users_spec.js @@ -0,0 +1,54 @@ +const should = require('should') // eslint-disable-line no-unused-vars +const sinon = require('sinon') + +const tools = require('../../../../../../../forge/ee/lib/mcp/tools/users') + +function getTool (name) { + return tools.find(tool => tool.name === name) +} + +describe('MCP Users Tools', function () { + let inject + + beforeEach(function () { + inject = sinon.stub() + }) + + describe('platform_get_active_user', function () { + const tool = getTool('platform_get_active_user') + + function userResponse (body = { id: 'user1', username: 'alice' }) { + return { statusCode: 200, json: () => body } + } + + it('calls the user endpoint and returns the profile with token metadata', async function () { + inject.resolves(userResponse()) + const response = await tool.handler({}, { inject, scope: { readOnly: true, teams: ['team1', 'team2'] } }) + inject.firstCall.args[0].should.eql({ method: 'GET', url: '/api/v1/user' }) + response.should.eql({ + id: 'user1', + username: 'alice', + token: { readOnly: true, allTeams: false, teams: ['team1', 'team2'] } + }) + }) + + it('reports full access when the token is not restricted to any team', async function () { + inject.resolves(userResponse()) + const response = await tool.handler({}, { inject, scope: { readOnly: false, teams: [] } }) + response.token.should.eql({ readOnly: false, allTeams: true, teams: [] }) + }) + + it('reports full access when no scope is provided', async function () { + inject.resolves(userResponse()) + const response = await tool.handler({}, { inject }) + response.token.should.eql({ readOnly: false, allTeams: true, teams: [] }) + }) + + it('returns the response unmodified when the request fails', async function () { + const injectResponse = { statusCode: 401, json: () => ({ code: 'unauthorized' }) } + inject.resolves(injectResponse) + const response = await tool.handler({}, { inject, scope: { readOnly: true, teams: [] } }) + response.should.equal(injectResponse) + }) + }) +}) diff --git a/test/unit/forge/routes/auth/oauth_spec.js b/test/unit/forge/routes/auth/oauth_spec.js index b403c90a21..2f0fbc7b20 100644 --- a/test/unit/forge/routes/auth/oauth_spec.js +++ b/test/unit/forge/routes/auth/oauth_spec.js @@ -319,4 +319,161 @@ describe('OAuth', async function () { scope.should.equal('read') }) }) + + describe('MCP agent auth (DCR + PKCE)', async function () { + let mcpApp + let sid + const redirectURI = 'http://localhost:9876/oauth/callback' + + function pkce () { + const verifier = base64URLEncode(crypto.randomBytes(32)) + const challenge = base64URLEncode(crypto.createHash('sha256').update(verifier).digest()) + return { verifier, challenge } + } + + async function register (redirectURIs = [redirectURI]) { + return mcpApp.inject({ + method: 'POST', + url: '/account/client', + payload: { redirect_uris: redirectURIs, client_name: 'Test MCP Client' } + }) + } + + function authorizeURL (clientID, redirect, challenge, state = '') { + const params = new URLSearchParams({ + client_id: clientID, + response_type: 'code', + redirect_uri: redirect, + state, + code_challenge: challenge, + code_challenge_method: 'S256' + }) + return `/account/authorize?${params}` + } + + before(async function () { + mcpApp = await setup({ + license: 'eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJGbG93Rm9yZ2UgSW5jLiIsInN1YiI6IkZsb3dGb3JnZSBJbmMuIERldmVsb3BtZW50IiwibmJmIjoxNjYyNDIyNDAwLCJleHAiOjc5ODY5MDIzOTksIm5vdGUiOiJEZXZlbG9wbWVudC1tb2RlIE9ubHkuIE5vdCBmb3IgcHJvZHVjdGlvbiIsInVzZXJzIjoxNTAsInRlYW1zIjo1MCwicHJvamVjdHMiOjUwLCJkZXZpY2VzIjo1MCwiZGV2Ijp0cnVlLCJpYXQiOjE2NjI0ODI5ODd9.e8Jeppq4aURwWYz-rEpnXs9RY2Y7HF7LJ6rMtMZWdw2Xls6-iyaiKV1TyzQw5sUBAhdUSZxgtiFH5e_cNJgrUg' + }) + const user = await mcpApp.factory.createUser({ + username: 'mcpuser', + name: 'MCP User', + email: 'mcp@example.com', + password: 'mmPassword' + }) + await mcpApp.team.addUser(user, { through: { role: mcpApp.factory.Roles.Roles.Owner } }) + const loginResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/login', + payload: { username: 'mcpuser', password: 'mmPassword', remember: false } + }) + sid = loginResponse.cookies[0].value + }) + after(async function () { + await mcpApp.close() + }) + + it('registers a public MCP client via DCR (RFC 7591)', async function () { + const response = await register() + response.should.have.property('statusCode', 201) + const body = response.json() + body.client_id.should.be.a.String().and.startWith('ffmcp') + body.should.have.property('token_endpoint_auth_method', 'none') + body.should.not.have.property('client_secret') + body.redirect_uris.should.eql([redirectURI]) + + const client = await mcpApp.db.controllers.AuthClient.getAuthClient(body.client_id) + client.should.have.property('type', 'mcp') + client.redirectURIs.should.eql([redirectURI]) + }) + + it('rejects a redirect_uri that is neither loopback-http nor https', async function () { + const response = await register(['http://example.com/callback']) + response.should.have.property('statusCode', 400) + response.json().should.have.property('error', 'invalid_redirect_uri') + }) + + it('accepts an https redirect_uri for hosted clients', async function () { + const response = await register(['https://claude.ai/api/mcp/callback']) + response.should.have.property('statusCode', 201) + }) + + it('completes the full flow: register -> authorize -> consent -> complete -> token -> refresh', async function () { + const reg = (await register()).json() + const clientID = reg.client_id + const { verifier, challenge } = pkce() + + // authorize -> redirected to the MCP consent page + const authResponse = await mcpApp.inject({ method: 'GET', url: authorizeURL(clientID, redirectURI, challenge, 'xyz'), cookies: { sid } }) + authResponse.should.have.property('statusCode', 302) + const m = /\/account\/request\/([^/]+)\/mcp$/.exec(authResponse.headers.location) + should.exist(m, 'expected redirect to MCP consent page: ' + authResponse.headers.location) + const requestId = m[1] + + // consent - user chooses read-only, scoped to their team + const consentResponse = await mcpApp.inject({ + method: 'PUT', + url: `/account/authorize/${requestId}/consent`, + payload: { readOnly: true, teamIds: [mcpApp.team.hashid] }, + cookies: { sid } + }) + consentResponse.should.have.property('statusCode', 200) + + // complete - issues the authorization code and redirects back to the client + const completeResponse = await mcpApp.inject({ method: 'GET', url: `/account/complete/${requestId}`, cookies: { sid } }) + completeResponse.should.have.property('statusCode', 302) + const callback = new URL(completeResponse.headers.location) + callback.host.should.equal('localhost:9876') + const authCode = callback.searchParams.get('code') + should.exist(authCode) + callback.searchParams.get('state').should.equal('xyz') + + // token - exchange the code for a scoped PAT + const tokenResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { + grant_type: 'authorization_code', + code: authCode, + redirect_uri: redirectURI, + client_id: clientID, + code_verifier: verifier + } + }) + tokenResponse.should.have.property('statusCode', 200) + const token = tokenResponse.json() + token.access_token.should.be.a.String().and.startWith('ffpat') + token.should.have.property('token_type', 'bearer') + token.should.have.property('refresh_token') + + // the issued token reflects the consent choices + const issued = await mcpApp.db.models.AccessToken.byRefreshToken(token.refresh_token) + issued.should.have.property('readOnly', true) + + // refresh - a public MCP client refreshes without a client secret + const refreshResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { grant_type: 'refresh_token', client_id: clientID, refresh_token: token.refresh_token } + }) + refreshResponse.should.have.property('statusCode', 200) + refreshResponse.json().access_token.should.be.a.String().and.startWith('ffpat') + }) + + it('rejects an authorize redirect_uri that was not registered', async function () { + const reg = (await register(['http://localhost:9876/oauth/callback'])).json() + const { challenge } = pkce() + const response = await mcpApp.inject({ method: 'GET', url: authorizeURL(reg.client_id, 'http://localhost:9876/evil', challenge), cookies: { sid } }) + response.should.have.property('statusCode', 400) + response.json().should.have.property('error', 'invalid_request') + }) + + it('accepts a loopback redirect on a different port than registered (RFC 8252)', async function () { + const reg = (await register(['http://localhost:1111/cb'])).json() + const { challenge } = pkce() + const response = await mcpApp.inject({ method: 'GET', url: authorizeURL(reg.client_id, 'http://127.0.0.1:2222/cb', challenge), cookies: { sid } }) + response.should.have.property('statusCode', 302) + response.headers.location.should.match(/\/account\/request\/[^/]+\/mcp$/) + }) + }) }) From af9e20a88569e3e4892532d5279599a1827f9b13 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Mon, 24 Aug 2026 19:48:33 +0200 Subject: [PATCH 03/14] feat(mcp): add MCP OAuth consent page Add the AccessRequestMCP.vue consent page and its /account/request/:id/mcp modal route. The page lets the user pick read-only or full access and scope the grant to specific teams before approving the MCP agent request. Ref FlowFuse/flowfuse#7433 --- .../src/pages/account/AccessRequestMCP.vue | 146 ++++++++++++++++++ frontend/src/pages/account/routes.js | 9 ++ 2 files changed, 155 insertions(+) create mode 100644 frontend/src/pages/account/AccessRequestMCP.vue diff --git a/frontend/src/pages/account/AccessRequestMCP.vue b/frontend/src/pages/account/AccessRequestMCP.vue new file mode 100644 index 0000000000..90bf4d789b --- /dev/null +++ b/frontend/src/pages/account/AccessRequestMCP.vue @@ -0,0 +1,146 @@ + + + diff --git a/frontend/src/pages/account/routes.js b/frontend/src/pages/account/routes.js index e5602c1657..8f3982bbdd 100644 --- a/frontend/src/pages/account/routes.js +++ b/frontend/src/pages/account/routes.js @@ -2,6 +2,7 @@ import { Cog8ToothIcon } from '@heroicons/vue/24/outline' import AccessRequest from './AccessRequest.vue' import AccessRequestEditor from './AccessRequestEditor.vue' +import AccessRequestMCP from './AccessRequestMCP.vue' import AccountCreate from './Create.vue' import ForgotPassword from './ForgotPassword.vue' import PasswordReset from './PasswordReset.vue' @@ -30,6 +31,14 @@ export default [ layout: 'modal' } }, + { + // MCP agent OAuth consent page with access level and team selection + path: '/account/request/:id/mcp', + component: AccessRequestMCP, + meta: { + layout: 'modal' + } + }, { // This is the FF Tools Plugin requesting access. This component asks the // user to confirm access From 691aa4b30a0ed22dd2756e370d9cb5e44a928711 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 10:52:56 +0200 Subject: [PATCH 04/14] feat(mcp): serve MCP resource metadata from the EE plugin Move the RFC 9728 protected-resource document out of the root .well-known handler into the license-gated EE mcp plugin, so it is only advertised where the /mcp resource exists. Serve it at the path-inserted /.well-known/oauth-protected-resource/mcp (RFC 9728 3.1) with the bare path kept as an alias, and challenge unauthenticated /mcp requests with a WWW-Authenticate header pointing at that metadata. --- forge/ee/routes/mcp/index.js | 1 + forge/ee/routes/mcp/server.js | 4 ++- forge/ee/routes/mcp/wellKnown.js | 34 ++++++++++++++++++++ forge/routes/wellKnown.js | 23 ------------- test/unit/forge/ee/routes/mcp/server_spec.js | 33 +++++++++++++++++++ test/unit/forge/routes/wellKnown_spec.js | 19 +---------- 6 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 forge/ee/routes/mcp/wellKnown.js diff --git a/forge/ee/routes/mcp/index.js b/forge/ee/routes/mcp/index.js index 13f87d2fd0..c8c6dbd8f6 100644 --- a/forge/ee/routes/mcp/index.js +++ b/forge/ee/routes/mcp/index.js @@ -7,6 +7,7 @@ * @param {import('../../../forge').ForgeApplication} app */ module.exports = async function (app) { + await app.register(require('./wellKnown'), { prefix: '/.well-known', logLevel: app.config.logging.http }) await app.register(require('./registrations'), { prefix: '/api/v1/teams/:teamId/mcp', logLevel: app.config.logging.http }) await app.register(require('./server'), { prefix: '/mcp', logLevel: app.config.logging.http }) } diff --git a/forge/ee/routes/mcp/server.js b/forge/ee/routes/mcp/server.js index 5a1c538507..a11bb6ed56 100644 --- a/forge/ee/routes/mcp/server.js +++ b/forge/ee/routes/mcp/server.js @@ -31,6 +31,8 @@ module.exports = async function (app) { // Resolves the caller's identity and scope, or sends an error reply and returns null. async function resolveCaller (request, reply) { if (!request.session?.User) { + // RFC 9728 §5.1: point unauthenticated callers at the resource metadata. + reply.header('WWW-Authenticate', `Bearer resource_metadata="${app.config.base_url}/.well-known/oauth-protected-resource/mcp"`) reply.code(401).send({ code: 'unauthorized', error: 'Unauthorized' }) return null } @@ -55,7 +57,7 @@ module.exports = async function (app) { // POST serves the MCP Streamable HTTP protocol in JSON mode: each request is // forwarded to the gateway and its response returned. Notifications (no id) // are acknowledged; the gateway populates its session on the first request. - app.post('/', async (request, reply) => { + app.post('/', { config: { allowAnonymous: true } }, async (request, reply) => { const caller = await resolveCaller(request, reply) if (!caller) { return diff --git a/forge/ee/routes/mcp/wellKnown.js b/forge/ee/routes/mcp/wellKnown.js new file mode 100644 index 0000000000..15f0107b46 --- /dev/null +++ b/forge/ee/routes/mcp/wellKnown.js @@ -0,0 +1,34 @@ +module.exports = async function (app) { + // RFC 9728: OAuth 2.0 Protected Resource Metadata for the /mcp resource. + // Lives in the EE mcp plugin so it is only advertised where /mcp exists. + function protectedResourceMetadata () { + const baseUrl = app.config.base_url + return { + resource: `${baseUrl}/mcp`, + authorization_servers: [baseUrl] + } + } + + const schema = { + tags: ['Authentication', 'X-HIDDEN'], + response: { + 200: { + type: 'object', + properties: { + resource: { type: 'string' }, + authorization_servers: { type: 'array', items: { type: 'string' } } + } + } + } + } + + // RFC 9728 §3.1: clients derive the metadata URL by inserting the resource + // path, so the /mcp resource is served at oauth-protected-resource/mcp. The + // bare path is kept as an alias for clients that omit path insertion. + app.get('/oauth-protected-resource/mcp', { config: { allowAnonymous: true }, schema }, async (request, reply) => { + reply.send(protectedResourceMetadata()) + }) + app.get('/oauth-protected-resource', { config: { allowAnonymous: true }, schema }, async (request, reply) => { + reply.send(protectedResourceMetadata()) + }) +} diff --git a/forge/routes/wellKnown.js b/forge/routes/wellKnown.js index 3c0f4c0a5c..fdb4040b32 100644 --- a/forge/routes/wellKnown.js +++ b/forge/routes/wellKnown.js @@ -33,27 +33,4 @@ module.exports = async function (app) { registration_endpoint: `${baseUrl}/account/client` }) }) - - // RFC 9728: OAuth 2.0 Protected Resource Metadata - app.get('/oauth-protected-resource', { - config: { allowAnonymous: true }, - schema: { - tags: ['Authentication', 'X-HIDDEN'], - response: { - 200: { - type: 'object', - properties: { - resource: { type: 'string' }, - authorization_servers: { type: 'array', items: { type: 'string' } } - } - } - } - } - }, async (request, reply) => { - const baseUrl = app.config.base_url - reply.send({ - resource: `${baseUrl}/mcp`, - authorization_servers: [baseUrl] - }) - }) } diff --git a/test/unit/forge/ee/routes/mcp/server_spec.js b/test/unit/forge/ee/routes/mcp/server_spec.js index 08c4803e6d..e97b3c8f21 100644 --- a/test/unit/forge/ee/routes/mcp/server_spec.js +++ b/test/unit/forge/ee/routes/mcp/server_spec.js @@ -41,6 +41,26 @@ describe('MCP Platform Tools Server', function () { }) }) + describe('GET /.well-known/oauth-protected-resource (RFC 9728)', function () { + it('serves the path-inserted resource metadata anonymously', async function () { + const response = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource/mcp' }) + response.statusCode.should.equal(200) + response.json().should.deepEqual({ + resource: `${app.config.base_url}/mcp`, + authorization_servers: [app.config.base_url] + }) + }) + + it('serves the bare alias anonymously', async function () { + const response = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource' }) + response.statusCode.should.equal(200) + response.json().should.deepEqual({ + resource: `${app.config.base_url}/mcp`, + authorization_servers: [app.config.base_url] + }) + }) + }) + describe('POST proxies to the MCP gateway', function () { let proxyRequest @@ -63,6 +83,19 @@ describe('MCP Platform Tools Server', function () { proxyRequest.called.should.be.false() }) + it('should challenge with the protected resource metadata URL', async function () { + const response = await app.inject({ + method: 'POST', + url: '/mcp', + payload: { jsonrpc: '2.0', method: 'initialize', id: 1 } + }) + response.statusCode.should.equal(401) + response.headers.should.have.property( + 'www-authenticate', + `Bearer resource_metadata="${app.config.base_url}/.well-known/oauth-protected-resource/mcp"` + ) + }) + it('should forward the request and return the gateway response', async function () { const response = await app.inject({ method: 'POST', diff --git a/test/unit/forge/routes/wellKnown_spec.js b/test/unit/forge/routes/wellKnown_spec.js index d0bbd37979..5e48abe2b4 100644 --- a/test/unit/forge/routes/wellKnown_spec.js +++ b/test/unit/forge/routes/wellKnown_spec.js @@ -41,25 +41,8 @@ describe('.well-known OAuth discovery', function () { }) }) - describe('GET /.well-known/oauth-protected-resource (RFC 9728)', function () { - let body - - before(async function () { - const response = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource' }) - response.statusCode.should.equal(200) - body = response.json() - }) - - it('advertises the MCP resource and its authorization server', function () { - body.should.have.property('resource', `${baseUrl}/mcp`) - body.authorization_servers.should.eql([baseUrl]) - }) - }) - - it('serves both documents anonymously, without a session', async function () { + it('serves the authorization server document anonymously, without a session', async function () { const authServer = await app.inject({ method: 'GET', url: '/.well-known/oauth-authorization-server' }) - const resource = await app.inject({ method: 'GET', url: '/.well-known/oauth-protected-resource' }) authServer.statusCode.should.equal(200) - resource.statusCode.should.equal(200) }) }) From d07d5056a519a44261f3539637182e45da82142e Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 10:56:06 +0200 Subject: [PATCH 05/14] fix(mcp): keep the refresh token alive after access-token expiry An AccessToken row holds both the access token and its refresh token, and getOrExpire destroyed the row when the access token expired, taking the refresh token with it so a client could never refresh (RFC 6749 1.5). Add a separate refreshTokenExpiresAt lifetime: reject an expired access token but keep the row while its refresh token is still valid. The MCP refresh token is stable rather than rotating, and concurrent refreshes coalesce through a shared cache so they reuse the most recently minted access token instead of overwriting the row. --- forge/db/controllers/AccessToken.js | 63 ++++++++++++-- ...d-refreshTokenExpiresAt-to-AccessTokens.js | 29 +++++++ forge/db/models/AccessToken.js | 1 + .../forge/db/controllers/AccessToken_spec.js | 87 +++++++++++++++++++ 4 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index 064e83d1df..47caaaa105 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -4,6 +4,15 @@ const { generateToken, generateNumericToken, sha256, randomPhrase } = require('. const DEFAULT_TOKEN_SESSION_EXPIRY = 1000 * 60 * 30 // 30 mins session - with refresh token support +const DEFAULT_REFRESH_TOKEN_EXPIRY = 1000 * 60 * 60 * 24 * 30 // 30 days - sliding refresh token lifetime + +// Concurrent refreshes of the same stable refresh token reuse the most recently +// minted access token from this shared cache (Valkey in production) rather than +// each minting a new one and overwriting the row. Re-mint once the cached token +// is within this window of expiry so a client is never handed a token about to die. +const MCP_ACCESS_TOKEN_CACHE = 'mcp-oauth-access-token' +const MCP_ACCESS_TOKEN_REMAINING_LIMIT = 1000 * 60 // 60 seconds + const DEFAULT_DEVICE_OTC_EXPIRY = 1000 * 60 * 60 * 24 // 24 hours /* @@ -269,6 +278,7 @@ module.exports = { const token = generateToken(32, 'ffpat') const refreshToken = generateToken(32, 'ffpat') const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY + const refreshTokenExpiresAt = Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY await app.db.sequelize.transaction(async (t) => { const tok = await app.db.models.AccessToken.create({ @@ -277,6 +287,7 @@ module.exports = { refreshToken, scope: '', expiresAt, + refreshTokenExpiresAt, readOnly, adminOptIn: false, ownerId: '' + userId, @@ -433,8 +444,14 @@ module.exports = { refreshToken: async function (app, refreshToken) { const existingToken = await app.db.models.AccessToken.byRefreshToken(refreshToken) - if (existingToken) { - const [prefix] = refreshToken.split('_') + if (!existingToken) { + return null + } + const [prefix] = refreshToken.split('_') + + // Tokens without their own refresh lifetime (e.g. editor sessions) rotate + // the refresh token on each use. + if (!existingToken.refreshTokenExpiresAt) { const tokenUpdates = { token: generateToken(32, prefix), refreshToken: generateToken(32, prefix), @@ -443,7 +460,35 @@ module.exports = { await app.db.models.AccessToken.update(tokenUpdates, { where: { refreshToken: existingToken.refreshToken } }) return tokenUpdates } - return null + + // A refresh token with its own lifetime is no longer valid once that + // lifetime has passed - remove it rather than issuing a new access token. + if (existingToken.refreshTokenExpiresAt.getTime() < Date.now()) { + await existingToken.destroy() + return null + } + + // These tokens keep a stable refresh token, so concurrent refreshes can + // reuse the access token most recently minted for it: the first mint + // populates the shared cache and the rest return the same token instead of + // overwriting the row. If a truly simultaneous mint slips past the cache, + // the stale side simply refreshes again with its unchanged refresh token. + const cache = app.caches?.getCache?.(MCP_ACCESS_TOKEN_CACHE, { ttl: DEFAULT_TOKEN_SESSION_EXPIRY, max: 10000 }) + const cacheKey = sha256(refreshToken) + const cached = await cache?.get(cacheKey) + if (cached && cached.expiresAt - Date.now() > MCP_ACCESS_TOKEN_REMAINING_LIMIT) { + return { token: cached.token, expiresAt: cached.expiresAt, refreshToken } + } + + const token = generateToken(32, prefix) + const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY + await app.db.models.AccessToken.update( + { token, expiresAt, refreshTokenExpiresAt: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY }, + { where: { refreshToken: existingToken.refreshToken } } + ) + await cache?.set(cacheKey, { token, expiresAt }) + // The refresh token is unchanged, so hand back the one the client presented. + return { token, expiresAt, refreshToken } }, /** @@ -465,8 +510,16 @@ module.exports = { }) if (accessToken) { if (accessToken.expiresAt && accessToken.expiresAt.getTime() < Date.now()) { - await accessToken.destroy() - accessToken = null + const refreshTokenValid = accessToken.refreshTokenExpiresAt && accessToken.refreshTokenExpiresAt.getTime() > Date.now() + if (refreshTokenValid) { + // The access token has expired but the refresh token is still + // valid. Reject the access token without destroying the row so + // the client can obtain a new one via refresh (RFC 6749 §1.5). + accessToken = null + } else { + await accessToken.destroy() + accessToken = null + } } } return accessToken diff --git a/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js b/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js new file mode 100644 index 0000000000..1962bae2a2 --- /dev/null +++ b/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js @@ -0,0 +1,29 @@ +/** + * Give refresh tokens a lifetime independent of the access token. + * + * An AccessToken row holds both the access token and its refresh token. The + * access token is short-lived (expiresAt); the refresh token is meant to + * outlive it so a client can obtain a new access token after expiry + * (RFC 6749 §1.5). Without a separate expiry the refresh token's lifetime was + * tied to the access token's, so expiring the access token also discarded the + * refresh token and made refresh impossible. + * + * refreshTokenExpiresAt - when the refresh token itself expires. Null for + * tokens that do not use a refresh lifetime, which + * keep the previous behaviour. + */ + +const { DataTypes } = require('sequelize') + +module.exports = { + up: async (context) => { + await context.addColumn('AccessTokens', 'refreshTokenExpiresAt', { + type: DataTypes.DATE, + allowNull: true, + defaultValue: null + }) + }, + down: async (context) => { + await context.removeColumn('AccessTokens', 'refreshTokenExpiresAt') + } +} diff --git a/forge/db/models/AccessToken.js b/forge/db/models/AccessToken.js index d92d15d686..f1a6c0e254 100644 --- a/forge/db/models/AccessToken.js +++ b/forge/db/models/AccessToken.js @@ -45,6 +45,7 @@ module.exports = { } } }, + refreshTokenExpiresAt: { type: DataTypes.DATE }, name: { type: DataTypes.STRING }, readOnly: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false }, adminOptIn: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false } diff --git a/test/unit/forge/db/controllers/AccessToken_spec.js b/test/unit/forge/db/controllers/AccessToken_spec.js index 1a0b273e80..d07c96d080 100644 --- a/test/unit/forge/db/controllers/AccessToken_spec.js +++ b/test/unit/forge/db/controllers/AccessToken_spec.js @@ -335,6 +335,93 @@ describe('AccessToken controller', function () { }) }) + describe('MCP OAuth Tokens', function () { + function createToken (opts = {}) { + return app.db.controllers.AccessToken.createMCPOAuthToken(TestObjects.alice.id, opts) + } + + // Move the row's access and/or refresh token expiry, so the refresh + // behaviour can be exercised without waiting for real time to pass. + async function setRowExpiry (refreshToken, { accessMs, refreshMs } = {}) { + const row = await app.db.models.AccessToken.byRefreshToken(refreshToken) + const updates = {} + if (accessMs !== undefined) { + updates.expiresAt = new Date(Date.now() + accessMs) + } + if (refreshMs !== undefined) { + updates.refreshTokenExpiresAt = new Date(Date.now() + refreshMs) + } + await app.db.models.AccessToken.update(updates, { where: { id: row.id } }) + } + + it('creates a user token with a refresh token that outlives the access token', async function () { + const result = await createToken({ readOnly: true }) + result.token.should.be.a.String().and.startWith('ffpat') + result.refreshToken.should.be.a.String().and.startWith('ffpat') + should.exist(result.expiresAt) + + const row = await app.db.models.AccessToken.byRefreshToken(result.refreshToken) + row.should.have.property('readOnly', true) + should.exist(row.refreshTokenExpiresAt) + row.refreshTokenExpiresAt.getTime().should.be.greaterThan(row.expiresAt.getTime()) + }) + + it('rejects an expired access token but keeps the row so it can still be refreshed', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { accessMs: -5000 }) + + // The access token is rejected... + should.not.exist(await app.db.controllers.AccessToken.getOrExpire(original.token)) + // ...but the row survives (RFC 6749 §1.5) so refresh still works. + ;(await app.db.models.AccessToken.count()).should.equal(1) + + const refreshed = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + should.exist(refreshed) + should.exist(await app.db.controllers.AccessToken.getOrExpire(refreshed.token)) + }) + + it('destroys the row once the refresh token has also expired', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { accessMs: -5000, refreshMs: -1000 }) + + should.not.exist(await app.db.controllers.AccessToken.getOrExpire(original.token)) + ;(await app.db.models.AccessToken.count()).should.equal(0) + }) + + it('keeps the refresh token stable and slides its expiry on refresh', async function () { + const original = await createToken() + const before = await app.db.models.AccessToken.byRefreshToken(original.refreshToken) + // Bring the refresh expiry close so the slide forward is observable. + await setRowExpiry(original.refreshToken, { accessMs: -5000, refreshMs: 60000 }) + + const refreshed = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + refreshed.token.should.not.equal(original.token) + // The refresh token is not rotated. + refreshed.refreshToken.should.equal(original.refreshToken) + + const after = await app.db.models.AccessToken.byRefreshToken(original.refreshToken) + after.refreshTokenExpiresAt.getTime().should.be.greaterThan(before.refreshTokenExpiresAt.getTime()) + }) + + it('reuses the same access token for a repeat refresh within its lifetime', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { accessMs: -5000 }) + + const first = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + const second = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + second.token.should.equal(first.token) + should.exist(await app.db.controllers.AccessToken.getOrExpire(first.token)) + }) + + it('fails to refresh once the refresh token lifetime has passed', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { refreshMs: -1000 }) + + should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken)) + ;(await app.db.models.AccessToken.count()).should.equal(0) + }) + }) + describe('getOrExpire', function () { it('does not return expired tokens', async function () { ;(await app.db.models.AccessToken.count()).should.equal(0) From 3ce33b31f2364eb11c41c933b041a2729d583a8c Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 11:35:45 +0200 Subject: [PATCH 06/14] test(mcp): make the refresh-expiry slide assertion deterministic Capture the pre-refresh expiry after lowering it, so the assertion compares against the shortened lifetime rather than the original one and no longer ties when the refresh lands in the same millisecond. --- test/unit/forge/db/controllers/AccessToken_spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/forge/db/controllers/AccessToken_spec.js b/test/unit/forge/db/controllers/AccessToken_spec.js index d07c96d080..8424a09a1a 100644 --- a/test/unit/forge/db/controllers/AccessToken_spec.js +++ b/test/unit/forge/db/controllers/AccessToken_spec.js @@ -390,9 +390,10 @@ describe('AccessToken controller', function () { it('keeps the refresh token stable and slides its expiry on refresh', async function () { const original = await createToken() - const before = await app.db.models.AccessToken.byRefreshToken(original.refreshToken) - // Bring the refresh expiry close so the slide forward is observable. + // Bring the refresh expiry close so the slide back out to the full + // lifetime is observable rather than a same-millisecond tie. await setRowExpiry(original.refreshToken, { accessMs: -5000, refreshMs: 60000 }) + const before = await app.db.models.AccessToken.byRefreshToken(original.refreshToken) const refreshed = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) refreshed.token.should.not.equal(original.token) From 55db3ec37f6bcbb3367eeefd78fcbfa75b1efb2c Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 12:36:17 +0200 Subject: [PATCH 07/14] feat(mcp): name the issued MCP token after its registered client Link the MCP OAuth token to the AuthClient it was issued to and name it after the registered client instead of the fixed 'MCP Agent', so the user's token list and later audit attribution can tell agents apart. --- forge/db/controllers/AccessToken.js | 5 ++-- ...60825-02-add-authclient-to-access-token.js | 27 +++++++++++++++++++ forge/db/models/AccessToken.js | 2 ++ forge/routes/auth/oauth.js | 4 ++- .../forge/db/controllers/AccessToken_spec.js | 26 ++++++++++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 forge/db/migrations/20260825-02-add-authclient-to-access-token.js diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index 47caaaa105..e88a1337fe 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -274,7 +274,7 @@ module.exports = { await app.settings.set('platform:stats:token', false) }, - createMCPOAuthToken: async function (app, userId, { readOnly = false, teamIds = [] } = {}) { + createMCPOAuthToken: async function (app, userId, { readOnly = false, teamIds = [], client } = {}) { const token = generateToken(32, 'ffpat') const refreshToken = generateToken(32, 'ffpat') const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY @@ -282,7 +282,8 @@ module.exports = { await app.db.sequelize.transaction(async (t) => { const tok = await app.db.models.AccessToken.create({ - name: 'MCP Agent', + name: client?.name || 'MCP Agent', + AuthClientId: client?.clientID || null, token, refreshToken, scope: '', diff --git a/forge/db/migrations/20260825-02-add-authclient-to-access-token.js b/forge/db/migrations/20260825-02-add-authclient-to-access-token.js new file mode 100644 index 0000000000..8de14688df --- /dev/null +++ b/forge/db/migrations/20260825-02-add-authclient-to-access-token.js @@ -0,0 +1,27 @@ +/** + * Link an AccessToken back to the AuthClient it was issued to. + * + * MCP OAuth tokens are minted for a dynamically-registered client (see + * AuthClient.type = 'mcp'), but until now the AccessToken row kept no + * reference to that client - only a copied-in `name`. Storing the client id + * lets the token be traced back to its client after issuance, which future + * audit attribution needs. + * + * AuthClientId - the clientID of the AuthClient this token was issued to. + * Null for tokens not tied to a client. + */ + +const { DataTypes } = require('sequelize') + +module.exports = { + up: async (context) => { + await context.addColumn('AccessTokens', 'AuthClientId', { + type: DataTypes.STRING, + allowNull: true, + defaultValue: null + }) + }, + down: async (context) => { + await context.removeColumn('AccessTokens', 'AuthClientId') + } +} diff --git a/forge/db/models/AccessToken.js b/forge/db/models/AccessToken.js index f1a6c0e254..8f056259b3 100644 --- a/forge/db/models/AccessToken.js +++ b/forge/db/models/AccessToken.js @@ -46,6 +46,7 @@ module.exports = { } }, refreshTokenExpiresAt: { type: DataTypes.DATE }, + AuthClientId: { type: DataTypes.STRING }, name: { type: DataTypes.STRING }, readOnly: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false }, adminOptIn: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false } @@ -55,6 +56,7 @@ module.exports = { this.belongsTo(M.Project, { foreignKey: 'ownerId', constraints: false }) this.belongsTo(M.Device, { foreignKey: 'ownerId', constraints: false }) this.belongsTo(M.User, { foreignKey: 'ownerId', constraints: false }) + this.belongsTo(M.AuthClient, { foreignKey: 'AuthClientId', targetKey: 'clientID', constraints: false }) this.hasMany(M.AccessTokenTeamScope) }, finders: function (M) { diff --git a/forge/routes/auth/oauth.js b/forge/routes/auth/oauth.js index bfb3a04cd6..aad578085a 100644 --- a/forge/routes/auth/oauth.js +++ b/forge/routes/auth/oauth.js @@ -524,11 +524,13 @@ module.exports = async function (app) { } reply.send(response) } else if (requestObject.mcp) { + const mcpClient = await app.db.controllers.AuthClient.getAuthClient(requestObject.client_id) const accessToken = await app.db.controllers.AccessToken.createMCPOAuthToken( requestObject.userId, { readOnly: requestObject.readOnly || false, - teamIds: requestObject.teamIds || [] + teamIds: requestObject.teamIds || [], + client: mcpClient } ) const response = { diff --git a/test/unit/forge/db/controllers/AccessToken_spec.js b/test/unit/forge/db/controllers/AccessToken_spec.js index 8424a09a1a..53562ffa97 100644 --- a/test/unit/forge/db/controllers/AccessToken_spec.js +++ b/test/unit/forge/db/controllers/AccessToken_spec.js @@ -366,6 +366,32 @@ describe('AccessToken controller', function () { row.refreshTokenExpiresAt.getTime().should.be.greaterThan(row.expiresAt.getTime()) }) + it('names the token after the registered client and links it', async function () { + const client = await app.db.models.AuthClient.create({ + clientID: 'ffmcp_test', + type: 'mcp', + name: 'Claude Test', + redirectURIs: ['http://127.0.0.1:9999/callback'], + ownerType: 'platform', + ownerId: '0' + }) + try { + const result = await createToken({ client }) + const row = await app.db.models.AccessToken.byRefreshToken(result.refreshToken) + row.should.have.property('name', client.name) + row.should.have.property('AuthClientId', client.clientID) + } finally { + await client.destroy() + } + }) + + it('falls back to a default name and no client link when no client is given', async function () { + const result = await createToken() + const row = await app.db.models.AccessToken.byRefreshToken(result.refreshToken) + row.should.have.property('name', 'MCP Agent') + should.not.exist(row.AuthClientId) + }) + it('rejects an expired access token but keeps the row so it can still be refreshed', async function () { const original = await createToken() await setRowExpiry(original.refreshToken, { accessMs: -5000 }) From 0ff89f4dc91207a1da472060e28aad20b3223d30 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 12:07:47 +0200 Subject: [PATCH 08/14] feat(mcp): rotate MCP OAuth refresh tokens with replay detection Rotate the refresh token on every MCP refresh instead of keeping it stable: each refresh issues a new refresh token and records the previous one. A rotated-out token presented within a short grace window returns the current tokens, so a concurrent or retried refresh still succeeds; presented after the window it is treated as a replay and the grant is revoked (RFC 9700 section 4.14.2). Concurrent refreshes are serialised with a compare-and-swap on the current refresh token: the update only matches while the token is still current, so exactly one refresh rotates and the loser returns the winner's tokens from the shared cache rather than a token that never reached the row. --- forge/db/controllers/AccessToken.js | 109 ++++++++++++------ ...-03-add-refresh-token-rotation-tracking.js | 36 ++++++ forge/db/models/AccessToken.js | 4 + .../forge/db/controllers/AccessToken_spec.js | 60 ++++++++-- 4 files changed, 167 insertions(+), 42 deletions(-) create mode 100644 forge/db/migrations/20260825-03-add-refresh-token-rotation-tracking.js diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index e88a1337fe..4c03c54b05 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -6,13 +6,18 @@ const DEFAULT_TOKEN_SESSION_EXPIRY = 1000 * 60 * 30 // 30 mins session - with re const DEFAULT_REFRESH_TOKEN_EXPIRY = 1000 * 60 * 60 * 24 * 30 // 30 days - sliding refresh token lifetime -// Concurrent refreshes of the same stable refresh token reuse the most recently -// minted access token from this shared cache (Valkey in production) rather than -// each minting a new one and overwriting the row. Re-mint once the cached token -// is within this window of expiry so a client is never handed a token about to die. +// Concurrent refreshes of the same refresh token converge on one rotation result +// via this shared cache (Valkey in production) rather than each minting its own +// and racing on the row. The result carries the new refresh token so every caller +// is handed the same one; a stale cached access token is re-minted once within +// this window of expiry so a client is never handed a token about to die. const MCP_ACCESS_TOKEN_CACHE = 'mcp-oauth-access-token' const MCP_ACCESS_TOKEN_REMAINING_LIMIT = 1000 * 60 // 60 seconds +// A rotated-out refresh token stays valid for this long so a concurrent or retried +// refresh still succeeds; presenting it after the window is treated as a replay. +const MCP_REFRESH_TOKEN_GRACE = 1000 * 60 // 60 seconds + const DEFAULT_DEVICE_OTC_EXPIRY = 1000 * 60 * 60 * 24 // 24 hours /* @@ -444,15 +449,12 @@ module.exports = { }, refreshToken: async function (app, refreshToken) { - const existingToken = await app.db.models.AccessToken.byRefreshToken(refreshToken) - if (!existingToken) { - return null - } const [prefix] = refreshToken.split('_') + const existingToken = await app.db.models.AccessToken.byRefreshToken(refreshToken) // Tokens without their own refresh lifetime (e.g. editor sessions) rotate - // the refresh token on each use. - if (!existingToken.refreshTokenExpiresAt) { + // the refresh token on each use and have no replay handling. + if (existingToken && !existingToken.refreshTokenExpiresAt) { const tokenUpdates = { token: generateToken(32, prefix), refreshToken: generateToken(32, prefix), @@ -462,34 +464,73 @@ module.exports = { return tokenUpdates } - // A refresh token with its own lifetime is no longer valid once that - // lifetime has passed - remove it rather than issuing a new access token. - if (existingToken.refreshTokenExpiresAt.getTime() < Date.now()) { - await existingToken.destroy() - return null - } - - // These tokens keep a stable refresh token, so concurrent refreshes can - // reuse the access token most recently minted for it: the first mint - // populates the shared cache and the rest return the same token instead of - // overwriting the row. If a truly simultaneous mint slips past the cache, - // the stale side simply refreshes again with its unchanged refresh token. const cache = app.caches?.getCache?.(MCP_ACCESS_TOKEN_CACHE, { ttl: DEFAULT_TOKEN_SESSION_EXPIRY, max: 10000 }) const cacheKey = sha256(refreshToken) - const cached = await cache?.get(cacheKey) - if (cached && cached.expiresAt - Date.now() > MCP_ACCESS_TOKEN_REMAINING_LIMIT) { - return { token: cached.token, expiresAt: cached.expiresAt, refreshToken } + + // The presented token is the current one: rotate it. + if (existingToken) { + // Once the refresh lifetime has passed the grant is over: remove it + // rather than issuing a new access token. + if (existingToken.refreshTokenExpiresAt.getTime() < Date.now()) { + await existingToken.destroy() + return null + } + // A concurrent refresh may already have rotated this token; reuse its + // result so both callers receive the same new refresh token. + const cached = await cache?.get(cacheKey) + if (cached && cached.expiresAt - Date.now() > MCP_ACCESS_TOKEN_REMAINING_LIMIT) { + return { token: cached.token, expiresAt: cached.expiresAt, refreshToken: cached.refreshToken } + } + const token = generateToken(32, prefix) + const newRefreshToken = generateToken(32, prefix) + const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY + // Compare-and-swap on the current refresh token: the update only matches + // while this token is still current, so of two simultaneous refreshes + // exactly one rotates and the other sees zero rows affected. + const [rotatedCount] = await app.db.models.AccessToken.update( + { + token, + expiresAt, + refreshToken: newRefreshToken, + refreshTokenExpiresAt: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY, + previousRefreshToken: existingToken.refreshToken, + previousRefreshTokenRotatedAt: new Date() + }, + { where: { refreshToken: existingToken.refreshToken } } + ) + if (rotatedCount === 0) { + // A concurrent refresh won the swap; return its result rather than a + // token that never made it onto the row. + const winner = await cache?.get(cacheKey) + if (winner) { + return { token: winner.token, expiresAt: winner.expiresAt, refreshToken: winner.refreshToken } + } + return null + } + await cache?.set(cacheKey, { token, expiresAt, refreshToken: newRefreshToken }) + return { token, expiresAt, refreshToken: newRefreshToken } } - const token = generateToken(32, prefix) - const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY - await app.db.models.AccessToken.update( - { token, expiresAt, refreshTokenExpiresAt: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY }, - { where: { refreshToken: existingToken.refreshToken } } - ) - await cache?.set(cacheKey, { token, expiresAt }) - // The refresh token is unchanged, so hand back the one the client presented. - return { token, expiresAt, refreshToken } + // Not the current token: it may be one just rotated out. byRefreshToken + // stores the sha256, so match the presented token's hash here too. + const rotated = await app.db.models.AccessToken.findOne({ where: { previousRefreshToken: cacheKey } }) + if (!rotated) { + return null + } + const rotatedAt = rotated.previousRefreshTokenRotatedAt ? rotated.previousRefreshTokenRotatedAt.getTime() : 0 + if (Date.now() - rotatedAt <= MCP_REFRESH_TOKEN_GRACE) { + // Within the grace window this is a retry or a lagging concurrent + // refresh: hand back the tokens the rotation produced. + const cached = await cache?.get(cacheKey) + if (cached) { + return { token: cached.token, expiresAt: cached.expiresAt, refreshToken: cached.refreshToken } + } + return null + } + // After the grace window the same token is a replay: revoke the grant so + // its access tokens stop working and the client must re-authorize. + await rotated.destroy() + return null }, /** diff --git a/forge/db/migrations/20260825-03-add-refresh-token-rotation-tracking.js b/forge/db/migrations/20260825-03-add-refresh-token-rotation-tracking.js new file mode 100644 index 0000000000..35d2e8443e --- /dev/null +++ b/forge/db/migrations/20260825-03-add-refresh-token-rotation-tracking.js @@ -0,0 +1,36 @@ +/** + * Track the previously rotated-out refresh token so rotation can tell a + * legitimate concurrent or retried refresh from a replay. + * + * On each MCP refresh the refresh token is rotated: a new one is issued and the + * presented one is recorded here. Presenting the recorded token again within a + * short grace window is treated as a retry and returns the current tokens; + * presenting it after the window is a replay and revokes the grant. + * + * previousRefreshToken - sha256 of the last rotated-out refresh token. + * previousRefreshTokenRotatedAt - when that rotation happened, used to bound + * the grace window. + * + * Both are null for tokens that do not rotate (for example editor sessions). + */ + +const { DataTypes } = require('sequelize') + +module.exports = { + up: async (context) => { + await context.addColumn('AccessTokens', 'previousRefreshToken', { + type: DataTypes.STRING, + allowNull: true, + defaultValue: null + }) + await context.addColumn('AccessTokens', 'previousRefreshTokenRotatedAt', { + type: DataTypes.DATE, + allowNull: true, + defaultValue: null + }) + }, + down: async (context) => { + await context.removeColumn('AccessTokens', 'previousRefreshToken') + await context.removeColumn('AccessTokens', 'previousRefreshTokenRotatedAt') + } +} diff --git a/forge/db/models/AccessToken.js b/forge/db/models/AccessToken.js index 8f056259b3..8863be37be 100644 --- a/forge/db/models/AccessToken.js +++ b/forge/db/models/AccessToken.js @@ -47,6 +47,10 @@ module.exports = { }, refreshTokenExpiresAt: { type: DataTypes.DATE }, AuthClientId: { type: DataTypes.STRING }, + // Holds the sha256 of the last rotated-out refresh token (set directly, + // already hashed) so rotation can distinguish a retry from a replay. + previousRefreshToken: { type: DataTypes.STRING }, + previousRefreshTokenRotatedAt: { type: DataTypes.DATE }, name: { type: DataTypes.STRING }, readOnly: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false }, adminOptIn: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false } diff --git a/test/unit/forge/db/controllers/AccessToken_spec.js b/test/unit/forge/db/controllers/AccessToken_spec.js index 53562ffa97..c4553e36a3 100644 --- a/test/unit/forge/db/controllers/AccessToken_spec.js +++ b/test/unit/forge/db/controllers/AccessToken_spec.js @@ -342,7 +342,7 @@ describe('AccessToken controller', function () { // Move the row's access and/or refresh token expiry, so the refresh // behaviour can be exercised without waiting for real time to pass. - async function setRowExpiry (refreshToken, { accessMs, refreshMs } = {}) { + async function setRowExpiry (refreshToken, { accessMs, refreshMs, rotatedMs } = {}) { const row = await app.db.models.AccessToken.byRefreshToken(refreshToken) const updates = {} if (accessMs !== undefined) { @@ -351,6 +351,9 @@ describe('AccessToken controller', function () { if (refreshMs !== undefined) { updates.refreshTokenExpiresAt = new Date(Date.now() + refreshMs) } + if (rotatedMs !== undefined) { + updates.previousRefreshTokenRotatedAt = new Date(Date.now() + rotatedMs) + } await app.db.models.AccessToken.update(updates, { where: { id: row.id } }) } @@ -414,7 +417,7 @@ describe('AccessToken controller', function () { ;(await app.db.models.AccessToken.count()).should.equal(0) }) - it('keeps the refresh token stable and slides its expiry on refresh', async function () { + it('rotates the refresh token and slides its expiry on refresh', async function () { const original = await createToken() // Bring the refresh expiry close so the slide back out to the full // lifetime is observable rather than a same-millisecond tie. @@ -423,20 +426,23 @@ describe('AccessToken controller', function () { const refreshed = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) refreshed.token.should.not.equal(original.token) - // The refresh token is not rotated. - refreshed.refreshToken.should.equal(original.refreshToken) + // The refresh token is rotated, and the new access token authenticates. + refreshed.refreshToken.should.be.a.String().and.not.equal(original.refreshToken) + should.exist(await app.db.controllers.AccessToken.getOrExpire(refreshed.token)) - const after = await app.db.models.AccessToken.byRefreshToken(original.refreshToken) + const after = await app.db.models.AccessToken.byRefreshToken(refreshed.refreshToken) after.refreshTokenExpiresAt.getTime().should.be.greaterThan(before.refreshTokenExpiresAt.getTime()) }) - it('reuses the same access token for a repeat refresh within its lifetime', async function () { + it('returns the same tokens for a retried refresh within the grace window', async function () { const original = await createToken() await setRowExpiry(original.refreshToken, { accessMs: -5000 }) const first = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) - const second = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) - second.token.should.equal(first.token) + // The rotated-out token is presented again (a retry or lagging concurrent refresh). + const retry = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + retry.token.should.equal(first.token) + retry.refreshToken.should.equal(first.refreshToken) should.exist(await app.db.controllers.AccessToken.getOrExpire(first.token)) }) @@ -447,6 +453,44 @@ describe('AccessToken controller', function () { should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken)) ;(await app.db.models.AccessToken.count()).should.equal(0) }) + + it('revokes the grant when a rotated-out token is replayed after the grace window', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { accessMs: -5000 }) + const first = await app.db.controllers.AccessToken.refreshToken(original.refreshToken) + + // Age the rotation beyond the grace window, then present the old token again. + await setRowExpiry(first.refreshToken, { rotatedMs: -120000 }) + should.not.exist(await app.db.controllers.AccessToken.refreshToken(original.refreshToken)) + // The whole grant is revoked, so the current access token stops working too. + ;(await app.db.models.AccessToken.count()).should.equal(0) + should.not.exist(await app.db.controllers.AccessToken.getOrExpire(first.token)) + }) + + it('returns null for an unknown refresh token', async function () { + should.not.exist(await app.db.controllers.AccessToken.refreshToken('ffpat_unknown')) + }) + + it('lets only one of two simultaneous refreshes rotate the token', async function () { + const original = await createToken() + await setRowExpiry(original.refreshToken, { accessMs: -5000 }) + + const [a, b] = await Promise.all([ + app.db.controllers.AccessToken.refreshToken(original.refreshToken), + app.db.controllers.AccessToken.refreshToken(original.refreshToken) + ]) + + // The grant is neither duplicated nor revoked by the race. + ;(await app.db.models.AccessToken.count()).should.equal(1) + const results = [a, b].filter(Boolean) + results.length.should.be.aboveOrEqual(1) + // Every token handed back is the single winner's: it authenticates, and + // the refresh tokens returned never diverge (no caller gets a dead token). + for (const result of results) { + should.exist(await app.db.controllers.AccessToken.getOrExpire(result.token)) + } + new Set(results.map(result => result.refreshToken)).size.should.equal(1) + }) }) describe('getOrExpire', function () { From 27d453f94cae0cc6cbdcb366b09b5f9d545329cb Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 13:14:55 +0200 Subject: [PATCH 09/14] fix(mcp): reach refresh rotation for already-rotated MCP tokens The refresh route rejected any refresh token byRefreshToken could not find. For a rotated MCP token that is the normal case: its hash has moved to previousRefreshToken, so the guard short-circuited the grace window and replay detection in refreshToken() before either could run. Defer resolution to refreshToken() for MCP clients, which handles the current-or-previous token, the grace window, and replay revocation. Add a route-level test covering rotation, grace retry, and replay through /account/token. --- forge/routes/auth/oauth.js | 15 ++++--- test/unit/forge/routes/auth/oauth_spec.js | 51 +++++++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/forge/routes/auth/oauth.js b/forge/routes/auth/oauth.js index aad578085a..db609db1b7 100644 --- a/forge/routes/auth/oauth.js +++ b/forge/routes/auth/oauth.js @@ -605,11 +605,6 @@ module.exports = async function (app) { reply.send(response) } } else if (grant_type === 'refresh_token') { - const existingToken = await app.db.models.AccessToken.byRefreshToken(refresh_token) - if (!existingToken) { - badRequest(reply, 'invalid_request', 'Invalid refresh_token') - return - } // ff-plugin and MCP clients are user-scoped; only project/device // clients need their resource ownership re-checked on refresh. let refreshAuthClient = null @@ -619,6 +614,16 @@ module.exports = async function (app) { return badRequest(reply, 'invalid_request', 'Invalid client_id') } } + const isMcpClient = refreshAuthClient?.type === 'mcp' + const existingToken = await app.db.models.AccessToken.byRefreshToken(refresh_token) + // A rotated-out MCP refresh token is no longer the row's current token, so + // byRefreshToken cannot find it. refreshToken() resolves the current-or-previous + // token, including the grace window and replay detection, so defer to it for + // MCP clients rather than rejecting an already-rotated token here. + if (!existingToken && !isMcpClient) { + badRequest(reply, 'invalid_request', 'Invalid refresh_token') + return + } if (refreshAuthClient && refreshAuthClient.type !== 'mcp') { // Check the owner of the existing session still has access to the project // this client is owned by diff --git a/test/unit/forge/routes/auth/oauth_spec.js b/test/unit/forge/routes/auth/oauth_spec.js index 2f0fbc7b20..902645b57b 100644 --- a/test/unit/forge/routes/auth/oauth_spec.js +++ b/test/unit/forge/routes/auth/oauth_spec.js @@ -460,6 +460,57 @@ describe('OAuth', async function () { refreshResponse.json().access_token.should.be.a.String().and.startWith('ffpat') }) + it('rotates the refresh token, honours the grace window, and revokes on replay', async function () { + const clientID = (await register()).json().client_id + const { verifier, challenge } = pkce() + const authResponse = await mcpApp.inject({ method: 'GET', url: authorizeURL(clientID, redirectURI, challenge), cookies: { sid } }) + const requestId = /\/account\/request\/([^/]+)\/mcp$/.exec(authResponse.headers.location)[1] + await mcpApp.inject({ method: 'PUT', url: `/account/authorize/${requestId}/consent`, payload: { readOnly: false, teamIds: [] }, cookies: { sid } }) + const completeResponse = await mcpApp.inject({ method: 'GET', url: `/account/complete/${requestId}`, cookies: { sid } }) + const authCode = new URL(completeResponse.headers.location).searchParams.get('code') + const first = (await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { grant_type: 'authorization_code', code: authCode, redirect_uri: redirectURI, client_id: clientID, code_verifier: verifier } + })).json() + + // refresh rotates: a new refresh token is issued in place of the presented one + const rotateResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { grant_type: 'refresh_token', client_id: clientID, refresh_token: first.refresh_token } + }) + rotateResponse.should.have.property('statusCode', 200) + const rotated = rotateResponse.json() + rotated.access_token.should.be.a.String().and.startWith('ffpat') + rotated.refresh_token.should.be.a.String().and.not.equal(first.refresh_token) + + // presenting the rotated-out token again within the grace window returns the + // current tokens rather than an error, so a retried or racing refresh still succeeds + const graceResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { grant_type: 'refresh_token', client_id: clientID, refresh_token: first.refresh_token } + }) + graceResponse.should.have.property('statusCode', 200) + graceResponse.json().refresh_token.should.equal(rotated.refresh_token) + + // push the rotation past the grace window so the rotated-out token reads as a replay + const row = await mcpApp.db.models.AccessToken.byRefreshToken(rotated.refresh_token) + await row.update({ previousRefreshTokenRotatedAt: new Date(Date.now() - 1000 * 60 * 60) }) + + const replayResponse = await mcpApp.inject({ + method: 'POST', + url: '/account/token', + payload: { grant_type: 'refresh_token', client_id: clientID, refresh_token: first.refresh_token } + }) + replayResponse.should.have.property('statusCode', 400) + + // the grant is revoked, so the current refresh token no longer resolves a row + const revoked = await mcpApp.db.models.AccessToken.byRefreshToken(rotated.refresh_token) + should.not.exist(revoked) + }) + it('rejects an authorize redirect_uri that was not registered', async function () { const reg = (await register(['http://localhost:9876/oauth/callback'])).json() const { challenge } = pkce() From 199d84ec9081826943a5e1a1e14c84cd19e06d3d Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 13:16:04 +0200 Subject: [PATCH 10/14] feat(mcp): attribute third-party MCP audit entries to the calling client Platform tool calls were always stamped source 'mcp:expert', so a third-party agent's actions rendered as the first-party Expert. The MCP door now records the caller's registered client name against its mcpSessionId, and the comms handler reads it to stamp source 'mcp' with that client name. The Expert path never opens that door, so it keeps the 'mcp:expert' default. The audit entry surfaces the client name. Closes #8271. --- forge/comms/platformAutomation.js | 22 ++++- forge/db/controllers/AccessToken.js | 2 + forge/db/controllers/AuditLog.js | 1 + forge/ee/routes/mcp/server.js | 13 ++- forge/routes/auth/index.js | 3 +- .../src/components/audit-log/AuditEntry.vue | 4 +- .../forge/comms/platformAutomation_spec.js | 80 ++++++++++++++++++- .../forge/db/controllers/AuditLog_spec.js | 50 ++++++++++++ 8 files changed, 167 insertions(+), 8 deletions(-) diff --git a/forge/comms/platformAutomation.js b/forge/comms/platformAutomation.js index 9a054d4f93..72b8d59dd6 100644 --- a/forge/comms/platformAutomation.js +++ b/forge/comms/platformAutomation.js @@ -4,6 +4,12 @@ const { default: z } = require('zod') +// Written by the third-party MCP door (forge/ee/routes/mcp/server.js) for a caller +// with a registered client. A hit here means the request came from a third-party +// agent rather than the first-party Expert, so the audit source and displayed +// client name below differ from the 'mcp:expert' default. +const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' + /** * Cheap, non-cryptographic fingerprint of the platform tool catalog, over each tool's * name/title/description/inputSchema/outputSchema/annotations/_meta. Sorted for stability @@ -109,6 +115,13 @@ class PlatformAutomationHandler { let result = {} this.app.log.info(`platform-automation request: userId=${userId} mcpSessionId=${mcpSessionId} command=${command} tool=${data?.name || 'n/a'}`) + // A cache hit means a third-party agent opened this session at the MCP + // door; a miss keeps the 'mcp:expert' default for the first-party Expert + // path, which never goes through that door. + const sessionSourceCache = this.app.caches?.getCache?.(MCP_SESSION_SOURCE_CACHE) + const sessionSource = mcpSessionId ? await sessionSourceCache?.get(mcpSessionId) : null + const source = sessionSource ? 'mcp' : 'mcp:expert' + switch (command) { case 'mcp-get-features': if (data?.hashOnly) { @@ -154,10 +167,11 @@ class PlatformAutomationHandler { if (user) { const { token } = await this.app.expert.mcp.getOrCreatePlatformToken(user) const inject = (opts) => { - const nonce = this.app.nonceStore.createSourceNonce({ - source: 'mcp:expert', - toolName - }) + const nonceMetadata = { source, toolName } + if (sessionSource?.clientName) { + nonceMetadata.clientName = sessionSource.clientName + } + const nonce = this.app.nonceStore.createSourceNonce(nonceMetadata) return this.app.inject({ ...opts, headers: { diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index e88a1337fe..ccd50d9254 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -507,6 +507,8 @@ module.exports = { include: [{ model: app.db.models.AccessTokenTeamScope, include: [{ model: app.db.models.Team, attributes: ['id', 'name'] }] + }, { + model: app.db.models.AuthClient }] }) if (accessToken) { diff --git a/forge/db/controllers/AuditLog.js b/forge/db/controllers/AuditLog.js index 58ffbe6979..4edf141bc4 100644 --- a/forge/db/controllers/AuditLog.js +++ b/forge/db/controllers/AuditLog.js @@ -18,6 +18,7 @@ function getSourceContext () { if (ctx.tokenId != null) { sc.tokenId = ctx.tokenId } if (ctx.toolName) { sc.toolName = ctx.toolName } if (ctx.correlationId) { sc.correlationId = ctx.correlationId } + if (ctx.clientName) { sc.clientName = ctx.clientName } return { source: ctx.source ?? null, sourceContext: Object.keys(sc).length > 0 ? sc : undefined diff --git a/forge/ee/routes/mcp/server.js b/forge/ee/routes/mcp/server.js index a11bb6ed56..52ba5e459e 100644 --- a/forge/ee/routes/mcp/server.js +++ b/forge/ee/routes/mcp/server.js @@ -1,5 +1,11 @@ const { randomUUID } = require('node:crypto') +// Maps an in-flight mcpSessionId to the registered client name of the third-party +// caller that opened it, so the comms layer can attribute audit entries to that +// client instead of the first-party Expert default. Read by forge/comms/platformAutomation.js. +const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' +const MCP_SESSION_SOURCE_CACHE_TTL = 1000 * 60 * 60 // 1 hour + /** * MCP Platform Tools Server * @@ -50,7 +56,8 @@ module.exports = async function (app) { } return { userId: request.session.User.hashid, - scope: { readOnly, teams } + scope: { readOnly, teams }, + clientName: request.session.pat?.clientName || null } } @@ -82,6 +89,10 @@ module.exports = async function (app) { } const mcpSessionId = request.headers['mcp-session-id'] || randomUUID() + if (caller.clientName) { + const cache = app.caches?.getCache?.(MCP_SESSION_SOURCE_CACHE, { ttl: MCP_SESSION_SOURCE_CACHE_TTL, max: 10000 }) + await cache?.set(mcpSessionId, { clientName: caller.clientName }) + } const route = { userId: caller.userId, mcpSessionId diff --git a/forge/routes/auth/index.js b/forge/routes/auth/index.js index fb0acaa997..a45034ef17 100644 --- a/forge/routes/auth/index.js +++ b/forge/routes/auth/index.js @@ -160,7 +160,8 @@ async function init (app, opts) { id: accessToken.id, readOnly: accessToken.readOnly, adminOptIn: accessToken.adminOptIn, - teamScopes + teamScopes, + clientName: accessToken.AuthClient?.name || null } request.session.isPAT = true diff --git a/frontend/src/components/audit-log/AuditEntry.vue b/frontend/src/components/audit-log/AuditEntry.vue index 05259e48b8..227baae142 100644 --- a/frontend/src/components/audit-log/AuditEntry.vue +++ b/frontend/src/components/audit-log/AuditEntry.vue @@ -93,7 +93,9 @@ export default { return toolName ? `via Expert (Tool name: ${toolName})` : 'via Expert' } if (this.entry.source === 'mcp') { - return toolName ? `via MCP (Tool name: ${toolName})` : 'via MCP' + const clientName = this.entry.body?.sourceContext?.clientName + const via = clientName || 'MCP' + return toolName ? `via ${via} (Tool name: ${toolName})` : `via ${via}` } if (this.entry.source === 'api') { return 'via API' diff --git a/test/unit/forge/comms/platformAutomation_spec.js b/test/unit/forge/comms/platformAutomation_spec.js index a24bfd7f3b..654cfba828 100644 --- a/test/unit/forge/comms/platformAutomation_spec.js +++ b/test/unit/forge/comms/platformAutomation_spec.js @@ -24,13 +24,14 @@ describe('PlatformAutomationHandler', function () { } } - function invokeToolCall ({ userId, toolName, args, meta, scope }) { + function invokeToolCall ({ userId, toolName, args, meta, scope, mcpSessionId }) { return new Promise((resolve) => { const onSuccess = (result) => resolve({ ok: true, result }) const onError = (message, code, err) => resolve({ ok: false, message, code, err }) handler.eventHandler( { userId, + mcpSessionId, command: 'mcp-call-tool', data: { name: toolName, input: args || {} }, meta, @@ -150,6 +151,52 @@ describe('PlatformAutomationHandler', function () { }) }) + describe('third-party client attribution', function () { + const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' + + afterEach(async function () { + const cache = app.caches.getCache(MCP_SESSION_SOURCE_CACHE) + await cache.del('session-with-client') + }) + + it('mints a source mcp nonce with the client name when the door recorded one for the session', async function () { + const cache = app.caches.getCache(MCP_SESSION_SOURCE_CACHE) + await cache.set('session-with-client', { clientName: 'Some Third Party Agent' }) + + const createSpy = sinon.spy(app.nonceStore, 'createSourceNonce') + const tool = handler.findTool('platform_list_teams') + + const res = await invokeToolCall({ + userId: app.adminUser.hashid, + toolName: 'platform_list_teams', + mcpSessionId: 'session-with-client', + meta: { toolDefinition: { annotations: tool.annotations } } + }) + + res.ok.should.be.true() + const nonceArgs = createSpy.firstCall.args[0] + nonceArgs.should.have.property('source', 'mcp') + nonceArgs.should.have.property('clientName', 'Some Third Party Agent') + }) + + it('falls back to source mcp:expert when the session has no recorded client', async function () { + const createSpy = sinon.spy(app.nonceStore, 'createSourceNonce') + const tool = handler.findTool('platform_list_teams') + + const res = await invokeToolCall({ + userId: app.adminUser.hashid, + toolName: 'platform_list_teams', + mcpSessionId: 'session-without-client', + meta: { toolDefinition: { annotations: tool.annotations } } + }) + + res.ok.should.be.true() + const nonceArgs = createSpy.firstCall.args[0] + nonceArgs.should.have.property('source', 'mcp:expert') + nonceArgs.should.not.have.property('clientName') + }) + }) + describe('platform_get_remote_instance_status', function () { afterEach(function () { delete app.comms @@ -298,5 +345,36 @@ describe('PlatformAutomationHandler', function () { const body = JSON.parse(entry.body) body.sourceContext.should.have.property('toolName', 'platform_create_application') }) + + it('tool call from a session with a recorded client produces an audit entry with source mcp and the client name', async function () { + const cache = app.caches.getCache('mcp-session-source') + await cache.set('audit-trail-third-party-session', { clientName: 'Some Third Party Agent' }) + + try { + const tool = handler.findTool('platform_create_application') + + const res = await invokeToolCall({ + userId: app.adminUser.hashid, + toolName: 'platform_create_application', + mcpSessionId: 'audit-trail-third-party-session', + args: { name: 'audit-trail-third-party-app', teamId: app.team.hashid }, + meta: { toolDefinition: { annotations: tool.annotations } } + }) + + res.ok.should.be.true() + + const entry = await app.db.models.AuditLog.findOne({ + where: { event: 'application.created' }, + order: [['createdAt', 'DESC']] + }) + should.exist(entry) + entry.source.should.equal('mcp') + const body = JSON.parse(entry.body) + body.sourceContext.should.have.property('toolName', 'platform_create_application') + body.sourceContext.should.have.property('clientName', 'Some Third Party Agent') + } finally { + await cache.del('audit-trail-third-party-session') + } + }) }) }) diff --git a/test/unit/forge/db/controllers/AuditLog_spec.js b/test/unit/forge/db/controllers/AuditLog_spec.js index 1fe4847b1b..e6e3e685d1 100644 --- a/test/unit/forge/db/controllers/AuditLog_spec.js +++ b/test/unit/forge/db/controllers/AuditLog_spec.js @@ -77,6 +77,56 @@ describe('AuditLog controller', function () { } }) + it('writes mcp source with clientName from request context', async function () { + const store = requestContext + const origGet = store.get + store.get = (key) => { + if (key === 'sourceContext') { + return { + source: 'mcp', + toolName: 'get-instance', + clientName: 'Some Third Party Agent' + } + } + return origGet.call(store, key) + } + + try { + await app.db.controllers.AuditLog.projectLog('1', null, 'test.event', {}) + const entries = await app.db.models.AuditLog.findAll() + entries.should.have.length(1) + entries[0].source.should.equal('mcp') + + const body = JSON.parse(entries[0].body) + should(body.sourceContext).be.an.Object() + body.sourceContext.clientName.should.equal('Some Third Party Agent') + } finally { + store.get = origGet + } + }) + + it('omits clientName from sourceContext when not provided', async function () { + const store = requestContext + const origGet = store.get + store.get = (key) => { + if (key === 'sourceContext') { + return { source: 'mcp:expert', toolName: 'get-instance' } + } + return origGet.call(store, key) + } + + try { + await app.db.controllers.AuditLog.projectLog('1', null, 'test.event', {}) + const entries = await app.db.models.AuditLog.findAll() + entries.should.have.length(1) + + const body = JSON.parse(entries[0].body) + should(body.sourceContext.clientName).be.undefined() + } finally { + store.get = origGet + } + }) + it('includes tokenId in sourceContext for PAT API calls', async function () { const store = requestContext const origGet = store.get From 90ffd1fafdbee80748f5b22843c7488e0651734d Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 15:18:07 +0200 Subject: [PATCH 11/14] perf(mcp): scope the AuthClient lookup to MCP tokens getOrExpire eager-loaded AuthClient on every token lookup so the client name could be attached to the session, adding a join to every Bearer request when only MCP tokens set AuthClientId. Drop the join and resolve the name with a targeted lookup that runs only when AuthClientId is set. --- forge/db/controllers/AccessToken.js | 2 -- forge/routes/auth/index.js | 14 +++++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index ccd50d9254..e88a1337fe 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -507,8 +507,6 @@ module.exports = { include: [{ model: app.db.models.AccessTokenTeamScope, include: [{ model: app.db.models.Team, attributes: ['id', 'name'] }] - }, { - model: app.db.models.AuthClient }] }) if (accessToken) { diff --git a/forge/routes/auth/index.js b/forge/routes/auth/index.js index a45034ef17..4d6d964276 100644 --- a/forge/routes/auth/index.js +++ b/forge/routes/auth/index.js @@ -156,12 +156,24 @@ async function init (app, opts) { })) } + // Only tokens issued to a registered MCP client carry an + // AuthClientId, so the client name lookup is skipped for + // every other token rather than joined on each request. + let clientName = null + if (accessToken.AuthClientId) { + const authClient = await app.db.models.AuthClient.findOne({ + where: { clientID: accessToken.AuthClientId }, + attributes: ['name'] + }) + clientName = authClient?.name || null + } + const patMetadata = { id: accessToken.id, readOnly: accessToken.readOnly, adminOptIn: accessToken.adminOptIn, teamScopes, - clientName: accessToken.AuthClient?.name || null + clientName } request.session.isPAT = true From 39e94a7f521b98c117aff8d39f4889d4afc84d82 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 15:30:58 +0200 Subject: [PATCH 12/14] fix(mcp): include token_type in the refresh token response The refresh_token grant returned access_token, expires_in and refresh_token but omitted token_type, which RFC 6749 section 5.1 requires. A spec-compliant client rejects the response and falls back to re-authorization even though the server rotated the token successfully. Return token_type: bearer to match the authorization_code response. --- forge/routes/auth/oauth.js | 1 + test/unit/forge/routes/auth/oauth_spec.js | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/forge/routes/auth/oauth.js b/forge/routes/auth/oauth.js index bfb3a04cd6..81681fc0a7 100644 --- a/forge/routes/auth/oauth.js +++ b/forge/routes/auth/oauth.js @@ -655,6 +655,7 @@ module.exports = async function (app) { const response = { access_token: accessToken.token, + token_type: 'bearer', expires_in: Math.floor((accessToken.expiresAt - Date.now()) / 1000), refresh_token: accessToken.refreshToken } diff --git a/test/unit/forge/routes/auth/oauth_spec.js b/test/unit/forge/routes/auth/oauth_spec.js index 2f0fbc7b20..847ac2dd6d 100644 --- a/test/unit/forge/routes/auth/oauth_spec.js +++ b/test/unit/forge/routes/auth/oauth_spec.js @@ -457,7 +457,10 @@ describe('OAuth', async function () { payload: { grant_type: 'refresh_token', client_id: clientID, refresh_token: token.refresh_token } }) refreshResponse.should.have.property('statusCode', 200) - refreshResponse.json().access_token.should.be.a.String().and.startWith('ffpat') + const refreshed = refreshResponse.json() + refreshed.access_token.should.be.a.String().and.startWith('ffpat') + refreshed.should.have.property('token_type', 'bearer') + refreshed.should.have.property('refresh_token') }) it('rejects an authorize redirect_uri that was not registered', async function () { From 3174417d8966d9dd48ca952467f247b97ce816a7 Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 15:43:30 +0200 Subject: [PATCH 13/14] chore(mcp): make migration down a no-op to match convention --- .../db/migrations/20260824-01-add-mcp-authclient-fields.js | 6 +----- ...20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js b/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js index df7fa920b8..21ffaff4f2 100644 --- a/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js +++ b/forge/db/migrations/20260824-01-add-mcp-authclient-fields.js @@ -31,9 +31,5 @@ module.exports = { defaultValue: null }) }, - down: async (context) => { - await context.removeColumn('AuthClients', 'type') - await context.removeColumn('AuthClients', 'name') - await context.removeColumn('AuthClients', 'redirectURIs') - } + down: async (context) => {} } diff --git a/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js b/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js index 1962bae2a2..536414ab40 100644 --- a/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js +++ b/forge/db/migrations/20260825-01-add-refreshTokenExpiresAt-to-AccessTokens.js @@ -23,7 +23,5 @@ module.exports = { defaultValue: null }) }, - down: async (context) => { - await context.removeColumn('AccessTokens', 'refreshTokenExpiresAt') - } + down: async (context) => {} } From e09546af394d1fdaeb89543cf5d64847668b906c Mon Sep 17 00:00:00 2001 From: andypalmi Date: Tue, 25 Aug 2026 19:14:35 +0200 Subject: [PATCH 14/14] refactor(mcp): attribute third-party audit via the caller's PAT Run third-party MCP platform actions under the caller's own PAT rather than a freshly minted platform token, and derive the audit source from whether the session has one. The door stashes the PAT against the mcpSessionId; the comms layer injects it and marks the entry 'mcp', or mints a token and marks 'mcp:expert' for the first-party Expert path. Removes the per-client identity plumbing (AuthClientId column, client name lookup and rendering), which is not needed to distinguish expert, third-party and api sources. --- forge/comms/platformAutomation.js | 26 ++++------ forge/db/controllers/AccessToken.js | 5 +- forge/db/controllers/AuditLog.js | 1 - ...60825-02-add-authclient-to-access-token.js | 27 ---------- forge/db/models/AccessToken.js | 2 - forge/ee/routes/mcp/server.js | 19 ++++--- forge/routes/auth/index.js | 15 +----- forge/routes/auth/oauth.js | 4 +- .../src/components/audit-log/AuditEntry.vue | 4 +- .../forge/comms/platformAutomation_spec.js | 47 +++++++++-------- .../forge/db/controllers/AccessToken_spec.js | 26 ---------- .../forge/db/controllers/AuditLog_spec.js | 50 ------------------- 12 files changed, 48 insertions(+), 178 deletions(-) delete mode 100644 forge/db/migrations/20260825-02-add-authclient-to-access-token.js diff --git a/forge/comms/platformAutomation.js b/forge/comms/platformAutomation.js index 72b8d59dd6..ea288faf03 100644 --- a/forge/comms/platformAutomation.js +++ b/forge/comms/platformAutomation.js @@ -4,11 +4,9 @@ const { default: z } = require('zod') -// Written by the third-party MCP door (forge/ee/routes/mcp/server.js) for a caller -// with a registered client. A hit here means the request came from a third-party -// agent rather than the first-party Expert, so the audit source and displayed -// client name below differ from the 'mcp:expert' default. -const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' +// Written by the third-party MCP door; a hit means a third-party caller, and the +// value is that caller's PAT. A miss is the first-party Expert path. +const MCP_SESSION_TOKEN_CACHE = 'mcp-session-token' /** * Cheap, non-cryptographic fingerprint of the platform tool catalog, over each tool's @@ -115,12 +113,9 @@ class PlatformAutomationHandler { let result = {} this.app.log.info(`platform-automation request: userId=${userId} mcpSessionId=${mcpSessionId} command=${command} tool=${data?.name || 'n/a'}`) - // A cache hit means a third-party agent opened this session at the MCP - // door; a miss keeps the 'mcp:expert' default for the first-party Expert - // path, which never goes through that door. - const sessionSourceCache = this.app.caches?.getCache?.(MCP_SESSION_SOURCE_CACHE) - const sessionSource = mcpSessionId ? await sessionSourceCache?.get(mcpSessionId) : null - const source = sessionSource ? 'mcp' : 'mcp:expert' + const sessionTokenCache = this.app.caches?.getCache?.(MCP_SESSION_TOKEN_CACHE) + const sessionToken = mcpSessionId ? await sessionTokenCache?.get(mcpSessionId) : null + const source = sessionToken ? 'mcp' : 'mcp:expert' switch (command) { case 'mcp-get-features': @@ -165,13 +160,10 @@ class PlatformAutomationHandler { const user = await this.app.db.models.User.byId(userId) if (user) { - const { token } = await this.app.expert.mcp.getOrCreatePlatformToken(user) + // Third-party runs under the caller's PAT; Expert mints a token. + const token = sessionToken || (await this.app.expert.mcp.getOrCreatePlatformToken(user)).token const inject = (opts) => { - const nonceMetadata = { source, toolName } - if (sessionSource?.clientName) { - nonceMetadata.clientName = sessionSource.clientName - } - const nonce = this.app.nonceStore.createSourceNonce(nonceMetadata) + const nonce = this.app.nonceStore.createSourceNonce({ source, toolName }) return this.app.inject({ ...opts, headers: { diff --git a/forge/db/controllers/AccessToken.js b/forge/db/controllers/AccessToken.js index e88a1337fe..47caaaa105 100644 --- a/forge/db/controllers/AccessToken.js +++ b/forge/db/controllers/AccessToken.js @@ -274,7 +274,7 @@ module.exports = { await app.settings.set('platform:stats:token', false) }, - createMCPOAuthToken: async function (app, userId, { readOnly = false, teamIds = [], client } = {}) { + createMCPOAuthToken: async function (app, userId, { readOnly = false, teamIds = [] } = {}) { const token = generateToken(32, 'ffpat') const refreshToken = generateToken(32, 'ffpat') const expiresAt = Date.now() + DEFAULT_TOKEN_SESSION_EXPIRY @@ -282,8 +282,7 @@ module.exports = { await app.db.sequelize.transaction(async (t) => { const tok = await app.db.models.AccessToken.create({ - name: client?.name || 'MCP Agent', - AuthClientId: client?.clientID || null, + name: 'MCP Agent', token, refreshToken, scope: '', diff --git a/forge/db/controllers/AuditLog.js b/forge/db/controllers/AuditLog.js index 4edf141bc4..58ffbe6979 100644 --- a/forge/db/controllers/AuditLog.js +++ b/forge/db/controllers/AuditLog.js @@ -18,7 +18,6 @@ function getSourceContext () { if (ctx.tokenId != null) { sc.tokenId = ctx.tokenId } if (ctx.toolName) { sc.toolName = ctx.toolName } if (ctx.correlationId) { sc.correlationId = ctx.correlationId } - if (ctx.clientName) { sc.clientName = ctx.clientName } return { source: ctx.source ?? null, sourceContext: Object.keys(sc).length > 0 ? sc : undefined diff --git a/forge/db/migrations/20260825-02-add-authclient-to-access-token.js b/forge/db/migrations/20260825-02-add-authclient-to-access-token.js deleted file mode 100644 index 8de14688df..0000000000 --- a/forge/db/migrations/20260825-02-add-authclient-to-access-token.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Link an AccessToken back to the AuthClient it was issued to. - * - * MCP OAuth tokens are minted for a dynamically-registered client (see - * AuthClient.type = 'mcp'), but until now the AccessToken row kept no - * reference to that client - only a copied-in `name`. Storing the client id - * lets the token be traced back to its client after issuance, which future - * audit attribution needs. - * - * AuthClientId - the clientID of the AuthClient this token was issued to. - * Null for tokens not tied to a client. - */ - -const { DataTypes } = require('sequelize') - -module.exports = { - up: async (context) => { - await context.addColumn('AccessTokens', 'AuthClientId', { - type: DataTypes.STRING, - allowNull: true, - defaultValue: null - }) - }, - down: async (context) => { - await context.removeColumn('AccessTokens', 'AuthClientId') - } -} diff --git a/forge/db/models/AccessToken.js b/forge/db/models/AccessToken.js index 8f056259b3..f1a6c0e254 100644 --- a/forge/db/models/AccessToken.js +++ b/forge/db/models/AccessToken.js @@ -46,7 +46,6 @@ module.exports = { } }, refreshTokenExpiresAt: { type: DataTypes.DATE }, - AuthClientId: { type: DataTypes.STRING }, name: { type: DataTypes.STRING }, readOnly: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false }, adminOptIn: { type: DataTypes.BOOLEAN, defaultValue: false, allowNull: false } @@ -56,7 +55,6 @@ module.exports = { this.belongsTo(M.Project, { foreignKey: 'ownerId', constraints: false }) this.belongsTo(M.Device, { foreignKey: 'ownerId', constraints: false }) this.belongsTo(M.User, { foreignKey: 'ownerId', constraints: false }) - this.belongsTo(M.AuthClient, { foreignKey: 'AuthClientId', targetKey: 'clientID', constraints: false }) this.hasMany(M.AccessTokenTeamScope) }, finders: function (M) { diff --git a/forge/ee/routes/mcp/server.js b/forge/ee/routes/mcp/server.js index 52ba5e459e..d5b50b90cb 100644 --- a/forge/ee/routes/mcp/server.js +++ b/forge/ee/routes/mcp/server.js @@ -1,10 +1,8 @@ const { randomUUID } = require('node:crypto') -// Maps an in-flight mcpSessionId to the registered client name of the third-party -// caller that opened it, so the comms layer can attribute audit entries to that -// client instead of the first-party Expert default. Read by forge/comms/platformAutomation.js. -const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' -const MCP_SESSION_SOURCE_CACHE_TTL = 1000 * 60 * 60 // 1 hour +// Maps mcpSessionId to the third-party caller's PAT, consumed by the comms layer. +const MCP_SESSION_TOKEN_CACHE = 'mcp-session-token' +const MCP_SESSION_TOKEN_CACHE_TTL = 1000 * 60 * 60 // 1 hour /** * MCP Platform Tools Server @@ -56,8 +54,7 @@ module.exports = async function (app) { } return { userId: request.session.User.hashid, - scope: { readOnly, teams }, - clientName: request.session.pat?.clientName || null + scope: { readOnly, teams } } } @@ -89,9 +86,11 @@ module.exports = async function (app) { } const mcpSessionId = request.headers['mcp-session-id'] || randomUUID() - if (caller.clientName) { - const cache = app.caches?.getCache?.(MCP_SESSION_SOURCE_CACHE, { ttl: MCP_SESSION_SOURCE_CACHE_TTL, max: 10000 }) - await cache?.set(mcpSessionId, { clientName: caller.clientName }) + const authHeader = request.headers.authorization || '' + const token = authHeader.startsWith('Bearer ') ? authHeader.slice('Bearer '.length) : null + if (token) { + const cache = app.caches?.getCache?.(MCP_SESSION_TOKEN_CACHE, { ttl: MCP_SESSION_TOKEN_CACHE_TTL, max: 10000 }) + await cache?.set(mcpSessionId, token) } const route = { userId: caller.userId, diff --git a/forge/routes/auth/index.js b/forge/routes/auth/index.js index 4d6d964276..fb0acaa997 100644 --- a/forge/routes/auth/index.js +++ b/forge/routes/auth/index.js @@ -156,24 +156,11 @@ async function init (app, opts) { })) } - // Only tokens issued to a registered MCP client carry an - // AuthClientId, so the client name lookup is skipped for - // every other token rather than joined on each request. - let clientName = null - if (accessToken.AuthClientId) { - const authClient = await app.db.models.AuthClient.findOne({ - where: { clientID: accessToken.AuthClientId }, - attributes: ['name'] - }) - clientName = authClient?.name || null - } - const patMetadata = { id: accessToken.id, readOnly: accessToken.readOnly, adminOptIn: accessToken.adminOptIn, - teamScopes, - clientName + teamScopes } request.session.isPAT = true diff --git a/forge/routes/auth/oauth.js b/forge/routes/auth/oauth.js index 450c60230f..81681fc0a7 100644 --- a/forge/routes/auth/oauth.js +++ b/forge/routes/auth/oauth.js @@ -524,13 +524,11 @@ module.exports = async function (app) { } reply.send(response) } else if (requestObject.mcp) { - const mcpClient = await app.db.controllers.AuthClient.getAuthClient(requestObject.client_id) const accessToken = await app.db.controllers.AccessToken.createMCPOAuthToken( requestObject.userId, { readOnly: requestObject.readOnly || false, - teamIds: requestObject.teamIds || [], - client: mcpClient + teamIds: requestObject.teamIds || [] } ) const response = { diff --git a/frontend/src/components/audit-log/AuditEntry.vue b/frontend/src/components/audit-log/AuditEntry.vue index 227baae142..05259e48b8 100644 --- a/frontend/src/components/audit-log/AuditEntry.vue +++ b/frontend/src/components/audit-log/AuditEntry.vue @@ -93,9 +93,7 @@ export default { return toolName ? `via Expert (Tool name: ${toolName})` : 'via Expert' } if (this.entry.source === 'mcp') { - const clientName = this.entry.body?.sourceContext?.clientName - const via = clientName || 'MCP' - return toolName ? `via ${via} (Tool name: ${toolName})` : `via ${via}` + return toolName ? `via MCP (Tool name: ${toolName})` : 'via MCP' } if (this.entry.source === 'api') { return 'via API' diff --git a/test/unit/forge/comms/platformAutomation_spec.js b/test/unit/forge/comms/platformAutomation_spec.js index 654cfba828..7e2232e0ab 100644 --- a/test/unit/forge/comms/platformAutomation_spec.js +++ b/test/unit/forge/comms/platformAutomation_spec.js @@ -151,49 +151,52 @@ describe('PlatformAutomationHandler', function () { }) }) - describe('third-party client attribution', function () { - const MCP_SESSION_SOURCE_CACHE = 'mcp-session-source' + describe('third-party session token', function () { + const MCP_SESSION_TOKEN_CACHE = 'mcp-session-token' afterEach(async function () { - const cache = app.caches.getCache(MCP_SESSION_SOURCE_CACHE) - await cache.del('session-with-client') + const cache = app.caches.getCache(MCP_SESSION_TOKEN_CACHE) + await cache.del('session-with-token') }) - it('mints a source mcp nonce with the client name when the door recorded one for the session', async function () { - const cache = app.caches.getCache(MCP_SESSION_SOURCE_CACHE) - await cache.set('session-with-client', { clientName: 'Some Third Party Agent' }) + it('injects the caller PAT and marks source mcp when the door recorded a token', async function () { + const { token: callerToken } = await app.expert.mcp.getOrCreatePlatformToken(app.adminUser) + const cache = app.caches.getCache(MCP_SESSION_TOKEN_CACHE) + await cache.set('session-with-token', callerToken) - const createSpy = sinon.spy(app.nonceStore, 'createSourceNonce') + const platformTokenSpy = sinon.spy(app.expert.mcp, 'getOrCreatePlatformToken') + const injectSpy = sinon.spy(app, 'inject') + const nonceSpy = sinon.spy(app.nonceStore, 'createSourceNonce') const tool = handler.findTool('platform_list_teams') const res = await invokeToolCall({ userId: app.adminUser.hashid, toolName: 'platform_list_teams', - mcpSessionId: 'session-with-client', + mcpSessionId: 'session-with-token', meta: { toolDefinition: { annotations: tool.annotations } } }) res.ok.should.be.true() - const nonceArgs = createSpy.firstCall.args[0] - nonceArgs.should.have.property('source', 'mcp') - nonceArgs.should.have.property('clientName', 'Some Third Party Agent') + platformTokenSpy.called.should.be.false() + injectSpy.firstCall.args[0].headers.authorization.should.equal(`Bearer ${callerToken}`) + nonceSpy.firstCall.args[0].should.have.property('source', 'mcp') }) - it('falls back to source mcp:expert when the session has no recorded client', async function () { - const createSpy = sinon.spy(app.nonceStore, 'createSourceNonce') + it('mints a platform token and marks source mcp:expert when the session has no recorded token', async function () { + const platformTokenSpy = sinon.spy(app.expert.mcp, 'getOrCreatePlatformToken') + const nonceSpy = sinon.spy(app.nonceStore, 'createSourceNonce') const tool = handler.findTool('platform_list_teams') const res = await invokeToolCall({ userId: app.adminUser.hashid, toolName: 'platform_list_teams', - mcpSessionId: 'session-without-client', + mcpSessionId: 'session-without-token', meta: { toolDefinition: { annotations: tool.annotations } } }) res.ok.should.be.true() - const nonceArgs = createSpy.firstCall.args[0] - nonceArgs.should.have.property('source', 'mcp:expert') - nonceArgs.should.not.have.property('clientName') + platformTokenSpy.calledOnce.should.be.true() + nonceSpy.firstCall.args[0].should.have.property('source', 'mcp:expert') }) }) @@ -346,9 +349,10 @@ describe('PlatformAutomationHandler', function () { body.sourceContext.should.have.property('toolName', 'platform_create_application') }) - it('tool call from a session with a recorded client produces an audit entry with source mcp and the client name', async function () { - const cache = app.caches.getCache('mcp-session-source') - await cache.set('audit-trail-third-party-session', { clientName: 'Some Third Party Agent' }) + it('tool call from a session with a recorded token produces an audit entry with source mcp', async function () { + const { token: callerToken } = await app.expert.mcp.getOrCreatePlatformToken(app.adminUser) + const cache = app.caches.getCache('mcp-session-token') + await cache.set('audit-trail-third-party-session', callerToken) try { const tool = handler.findTool('platform_create_application') @@ -371,7 +375,6 @@ describe('PlatformAutomationHandler', function () { entry.source.should.equal('mcp') const body = JSON.parse(entry.body) body.sourceContext.should.have.property('toolName', 'platform_create_application') - body.sourceContext.should.have.property('clientName', 'Some Third Party Agent') } finally { await cache.del('audit-trail-third-party-session') } diff --git a/test/unit/forge/db/controllers/AccessToken_spec.js b/test/unit/forge/db/controllers/AccessToken_spec.js index 53562ffa97..8424a09a1a 100644 --- a/test/unit/forge/db/controllers/AccessToken_spec.js +++ b/test/unit/forge/db/controllers/AccessToken_spec.js @@ -366,32 +366,6 @@ describe('AccessToken controller', function () { row.refreshTokenExpiresAt.getTime().should.be.greaterThan(row.expiresAt.getTime()) }) - it('names the token after the registered client and links it', async function () { - const client = await app.db.models.AuthClient.create({ - clientID: 'ffmcp_test', - type: 'mcp', - name: 'Claude Test', - redirectURIs: ['http://127.0.0.1:9999/callback'], - ownerType: 'platform', - ownerId: '0' - }) - try { - const result = await createToken({ client }) - const row = await app.db.models.AccessToken.byRefreshToken(result.refreshToken) - row.should.have.property('name', client.name) - row.should.have.property('AuthClientId', client.clientID) - } finally { - await client.destroy() - } - }) - - it('falls back to a default name and no client link when no client is given', async function () { - const result = await createToken() - const row = await app.db.models.AccessToken.byRefreshToken(result.refreshToken) - row.should.have.property('name', 'MCP Agent') - should.not.exist(row.AuthClientId) - }) - it('rejects an expired access token but keeps the row so it can still be refreshed', async function () { const original = await createToken() await setRowExpiry(original.refreshToken, { accessMs: -5000 }) diff --git a/test/unit/forge/db/controllers/AuditLog_spec.js b/test/unit/forge/db/controllers/AuditLog_spec.js index e6e3e685d1..1fe4847b1b 100644 --- a/test/unit/forge/db/controllers/AuditLog_spec.js +++ b/test/unit/forge/db/controllers/AuditLog_spec.js @@ -77,56 +77,6 @@ describe('AuditLog controller', function () { } }) - it('writes mcp source with clientName from request context', async function () { - const store = requestContext - const origGet = store.get - store.get = (key) => { - if (key === 'sourceContext') { - return { - source: 'mcp', - toolName: 'get-instance', - clientName: 'Some Third Party Agent' - } - } - return origGet.call(store, key) - } - - try { - await app.db.controllers.AuditLog.projectLog('1', null, 'test.event', {}) - const entries = await app.db.models.AuditLog.findAll() - entries.should.have.length(1) - entries[0].source.should.equal('mcp') - - const body = JSON.parse(entries[0].body) - should(body.sourceContext).be.an.Object() - body.sourceContext.clientName.should.equal('Some Third Party Agent') - } finally { - store.get = origGet - } - }) - - it('omits clientName from sourceContext when not provided', async function () { - const store = requestContext - const origGet = store.get - store.get = (key) => { - if (key === 'sourceContext') { - return { source: 'mcp:expert', toolName: 'get-instance' } - } - return origGet.call(store, key) - } - - try { - await app.db.controllers.AuditLog.projectLog('1', null, 'test.event', {}) - const entries = await app.db.models.AuditLog.findAll() - entries.should.have.length(1) - - const body = JSON.parse(entries[0].body) - should(body.sourceContext.clientName).be.undefined() - } finally { - store.get = origGet - } - }) - it('includes tokenId in sourceContext for PAT API calls', async function () { const store = requestContext const origGet = store.get