123 f6 cross team mcp registration

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

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({})

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: {

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)
     })
 })