diff --git a/forge/ee/db/models/MCPRegistration.js b/forge/ee/db/models/MCPRegistration.js index 96e96463db..402d700414 100644 --- a/forge/ee/db/models/MCPRegistration.js +++ b/forge/ee/db/models/MCPRegistration.js @@ -91,14 +91,16 @@ module.exports = { include }) }, - byTypeAndIDs: async (targetType, targetId, nodeId) => { - return this.findOne({ - where: { - targetType, - targetId, - nodeId - } - }) + byTypeAndIDs: async (targetType, targetId, nodeId, teamId) => { + const where = { + targetType, + targetId, + nodeId + } + if (teamId !== undefined) { + where.TeamId = teamId + } + return this.findOne({ where }) }, byId: async (idOrHash, { includeAssociations = false } = {}) => { let id = idOrHash diff --git a/forge/ee/routes/mcp/registrations.js b/forge/ee/routes/mcp/registrations.js index ed8da3b08e..74f4811a04 100644 --- a/forge/ee/routes/mcp/registrations.js +++ b/forge/ee/routes/mcp/registrations.js @@ -1,3 +1,44 @@ +// Resolves the device/instance named by request.params.type/typeId, checks it +// belongs to request.team, and that it matches the session's own owner id. +// Replies with the appropriate status code and returns null on any failure, +// otherwise returns the resolved (string) typeId - shared by POST and DELETE +// so both report the same status code for the same failure. +async function resolveTarget (app, request, reply) { + let typeId = request.params.typeId + if (request.params.type === 'device') { + const device = await app.db.models.Device.byId(request.params.typeId) + if (!device) { + reply.code(404).send({ code: 'not_found', error: 'Device not found' }) + return null + } + typeId = '' + device.id + if (device.Team.id !== request.team.id) { + reply.code(403).send({ code: 'unauthorized', error: 'Unauthorized' }) + return null + } + } else if (request.params.type === 'instance') { + const project = await app.db.models.Project.byId(request.params.typeId) + if (!project) { + reply.code(404).send({ code: 'not_found', error: 'Instance not found' }) + return null + } + if (project.Team.id !== request.team.id) { + reply.code(403).send({ code: 'unauthorized', error: 'Unauthorized' }) + return null + } + } else { + reply.code(400).send({ code: 'invalid_request', error: `Unknown MCP target type '${request.params.type}'` }) + return null + } + + if (String(typeId) !== String(request.session.ownerId)) { + reply.code(403).send({ code: 'unauthorized', error: 'Unauthorized' }) + return null + } + + return typeId +} + module.exports = async function (app) { app.addHook('preHandler', async (request, reply) => { if (request.params.teamId !== undefined || request.params.teamSlug !== undefined) { @@ -103,29 +144,21 @@ module.exports = async function (app) { 200: { type: 'object' }, + '4xx': { + $ref: 'APIError' + }, 500: { $ref: 'APIError' } } } }, async (request, reply) => { - try { - let typeId = request.params.typeId - if (request.params.type === 'device') { - const device = await app.db.models.Device.byId(request.params.typeId) - if (!device) { - throw new Error(`Device '${request.params.typeId}' not found`) - } - typeId = device.id - } else if (request.params.type === 'instance') { - const project = await app.db.models.Project.byId(request.params.typeId) - if (!project) { - throw new Error(`Instance '${request.params.typeId}' not found`) - } - } else { - throw new Error(`Unknown MCP target type '${request.params.type}'`) - } + const typeId = await resolveTarget(app, request, reply) + if (typeId === null) { + return + } + try { await app.db.models.MCPRegistration.upsert({ targetType: request.params.type, targetId: typeId, @@ -182,8 +215,13 @@ module.exports = async function (app) { } } }, async (request, reply) => { + const typeId = await resolveTarget(app, request, reply) + if (typeId === null) { + return + } + try { - const mcpServer = await app.db.models.MCPRegistration.byTypeAndIDs(request.params.type, request.params.typeId, request.params.nodeId) + const mcpServer = await app.db.models.MCPRegistration.byTypeAndIDs(request.params.type, typeId, request.params.nodeId, request.team.id) if (mcpServer) { await mcpServer.destroy() reply.send({}) diff --git a/frontend/src/types/generated.ts b/frontend/src/types/generated.ts index 14fdab1d89..bea90fcf74 100644 --- a/frontend/src/types/generated.ts +++ b/frontend/src/types/generated.ts @@ -10041,6 +10041,15 @@ export interface paths { "application/json": components["schemas"]["APIError"]; }; }; + /** @description Default Response */ + "4XX": { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; }; }; delete: { diff --git a/test/unit/forge/ee/routes/mcp/index_spec.js b/test/unit/forge/ee/routes/mcp/index_spec.js index acd2c9c2ad..40b8f305f7 100644 --- a/test/unit/forge/ee/routes/mcp/index_spec.js +++ b/test/unit/forge/ee/routes/mcp/index_spec.js @@ -143,10 +143,8 @@ describe('MCP Server Registration', function () { const mcpServer = await app.db.models.MCPRegistration.byTypeAndIDs('instance', app.instance.id, 'abcde') should.not.exist(mcpServer) }) - it('should return 500 and log error for unknown device', async function () { + it('should return 404 for unknown device', async function () { const { token } = await app.instance.refreshAuthTokens() - // stub app.log to capture error message - const appLogStub = sinon.stub(app.log, 'error') const response = await app.inject({ method: 'POST', @@ -164,19 +162,14 @@ describe('MCP Server Registration', function () { description: 'Test MCP registration entry' } }) - response.statusCode.should.equal(500) + response.statusCode.should.equal(404) const result = response.json() result.should.be.an.Object() - result.should.have.property('code', 'unexpected_error') - result.should.have.property('error', 'Failed to create mcp entry') - appLogStub.calledOnce.should.be.true() - const errMsg = appLogStub.getCall(0).args[0] - errMsg.should.match(/Device '99999' not found/) + result.should.have.property('code', 'not_found') + result.should.have.property('error', 'Device not found') }) - it('should return 500 and log error for unknown instance', async function () { + it('should return 404 for unknown instance', async function () { const { token } = await app.instance.refreshAuthTokens() - // stub app.log to capture error message - const appLogStub = sinon.stub(app.log, 'error') const randomId = uuidv4() const response = await app.inject({ method: 'POST', @@ -194,19 +187,14 @@ describe('MCP Server Registration', function () { description: 'Test MCP registration entry' } }) - response.statusCode.should.equal(500) + response.statusCode.should.equal(404) const result = response.json() result.should.be.an.Object() - result.should.have.property('code', 'unexpected_error') - result.should.have.property('error', 'Failed to create mcp entry') - appLogStub.calledOnce.should.be.true() - const errMsg = appLogStub.getCall(0).args[0] - errMsg.should.match(new RegExp(`Instance '${randomId}' not found`)) + result.should.have.property('code', 'not_found') + result.should.have.property('error', 'Instance not found') }) - it('should return 500 and log error for unknown type', async function () { + it('should return 400 for unknown type', async function () { const { token } = await app.instance.refreshAuthTokens() - // stub app.log to capture error message - const appLogStub = sinon.stub(app.log, 'error') const response = await app.inject({ method: 'POST', @@ -224,13 +212,168 @@ describe('MCP Server Registration', function () { description: 'Test MCP registration entry' } }) - response.statusCode.should.equal(500) + response.statusCode.should.equal(400) const result = response.json() result.should.be.an.Object() - result.should.have.property('code', 'unexpected_error') - result.should.have.property('error', 'Failed to create mcp entry') - appLogStub.calledOnce.should.be.true() - const errMsg = appLogStub.getCall(0).args[0] - errMsg.should.match(/Unknown MCP target type 'blah'/) + result.should.have.property('code', 'invalid_request') + result.should.have.property('error', "Unknown MCP target type 'blah'") + }) + it('should not create mcp endpoint for non team asset', async function () { + // create second team + const bTeam = await app.factory.createTeam({ name: 'BTeam' }) + const bApplicaiton = await app.factory.createApplication({ name: 'application-2' }, bTeam) + // create instance in BTeam + const bInstance = await app.factory.createInstance( + { name: 'projectb' }, + bApplicaiton, + app.stack, + app.template, + app.projetType, + { start: false } + ) + const { token } = await bInstance.refreshAuthTokens() + + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/mcp/instance/${app.instance.id}/abcde`, + headers: { + Authorization: `Bearer ${token}`, // Use token from instance in team b + 'Content-Type': 'application/json' + }, + body: { + name: 'foo', + protocol: 'http', + endpointRoute: '/mcp', + title: 'FlowFuse MCP', + version: '1.2.3', + description: 'Test MCP registration entry' + } + }) + response.statusCode.should.equal(403) + }) + it('should delete MCP entry', async function () { + const { token } = await app.instance.refreshAuthTokens() + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/mcp/instance/${app.instance.id}/abcde`, + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json' + }, + body: { + name: 'foo', + protocol: 'http', + endpointRoute: '/mcp', + title: 'FlowFuse MCP', + version: '1.2.3', + description: 'Test MCP registration entry' + } + }) + response.statusCode.should.equal(200) + + // create second team + const cTeam = await app.factory.createTeam({ name: 'CTeam' }) + const cApplicaiton = await app.factory.createApplication({ name: 'application-2' }, cTeam) + // create instance in BTeam + const cInstance = await app.factory.createInstance( + { name: 'projectc' }, + cApplicaiton, + app.stack, + app.template, + app.projetType, + { start: false } + ) + const cToken = (await cInstance.refreshAuthTokens()).token + + const deleteResponse = await app.inject({ + method: 'DELETE', + url: `/api/v1/teams/${app.team.hashid}/mcp/instance/${app.instance.id}/abcde`, + headers: { + Authorization: `Bearer ${cToken}` + } + }) + deleteResponse.statusCode.should.equal(403) + }) + it('should let a device register and delete its own MCP entry', async function () { + const created = await app.factory.createDevice({ name: 'mcp-device' }, app.team) + // reload so the Team association is hydrated for token generation + const device = await app.db.models.Device.byId(created.id) + const { token } = await device.refreshAuthTokens() + + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/mcp/device/${device.hashid}/nodeD`, + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json' + }, + body: { + name: 'device-mcp', + protocol: 'http', + endpointRoute: '/mcp' + } + }) + response.statusCode.should.equal(200) + // stored against the numeric device id, not the hashid + const stored = await app.db.models.MCPRegistration.byTypeAndIDs('device', '' + device.id, 'nodeD') + should.exist(stored) + + const deleteResponse = await app.inject({ + method: 'DELETE', + url: `/api/v1/teams/${app.team.hashid}/mcp/device/${device.hashid}/nodeD`, + headers: { + Authorization: `Bearer ${token}` + } + }) + deleteResponse.statusCode.should.equal(200) + const afterDelete = await app.db.models.MCPRegistration.byTypeAndIDs('device', '' + device.id, 'nodeD') + should.not.exist(afterDelete) + }) + it('should not create MCP entry when teamId does not match the asset team', async function () { + // second team, used only for the URL teamId + const bTeam = await app.factory.createTeam({ name: 'BTeamMismatch' }) + const { token } = await app.instance.refreshAuthTokens() + + // caller's own token and own instance id, but a foreign team in the URL + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${bTeam.hashid}/mcp/instance/${app.instance.id}/abcde`, + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json' + }, + body: { + name: 'foo', + protocol: 'http', + endpointRoute: '/mcp' + } + }) + response.statusCode.should.equal(403) + }) + it('should not delete MCP entry when teamId does not match the asset team', async function () { + const bTeam = await app.factory.createTeam({ name: 'BTeamMismatchDelete' }) + const { token } = await app.instance.refreshAuthTokens() + + const deleteResponse = await app.inject({ + method: 'DELETE', + url: `/api/v1/teams/${bTeam.hashid}/mcp/instance/${app.instance.id}/abcde`, + headers: { + Authorization: `Bearer ${token}` + } + }) + deleteResponse.statusCode.should.equal(403) + }) + it('should return 404 when deleting MCP entry for a missing instance', async function () { + const { token } = await app.instance.refreshAuthTokens() + const randomId = uuidv4() + + const deleteResponse = await app.inject({ + method: 'DELETE', + url: `/api/v1/teams/${app.team.hashid}/mcp/instance/${randomId}/abcde`, + headers: { + Authorization: `Bearer ${token}` + } + }) + deleteResponse.statusCode.should.equal(404) }) })