Fix device group snapshot assignment (F5)

forge/ee/db/controllers/DeviceGroup.js

@@ -104,6 +104,23 @@ module.exports = {
                 if (!snapshot) {
                     throw new ValidationError('Snapshot does not exist')
                 }
+                // ensure the snapshot belongs to the same application as the device
+                // group - never trust a payload-supplied snapshot id to be in-scope.
+                // Scoped to application (not just team) because granular RBAC can
+                // authorize a caller for this application without authorizing them
+                // for another application in the same team.
+                const application = await deviceGroup.getApplication()
+                let snapshotApplicationId = null
+                if (snapshot.ownerType === 'instance') {
+                    const project = await snapshot.getProject()
+                    snapshotApplicationId = project?.ApplicationId
+                } else if (snapshot.ownerType === 'device') {
+                    const device = await snapshot.getDevice()
+                    snapshotApplicationId = device?.ApplicationId || (device?.ProjectId ? (await device.getProject())?.ApplicationId : null)
+                }
+                if (!application || !snapshotApplicationId || snapshotApplicationId !== application.id) {
+                    throw new ValidationError('Snapshot does not belong to the same application')
+                }
                 snapshotId = snapshot.id
             }
 

test/unit/forge/ee/routes/api/applicationDeviceGroups_spec.js

@@ -442,10 +442,13 @@ describe('Application Device Groups API', function () {
 
             // first lets verify the devices have the snapshot set as target
             device1of2.should.have.property('targetSnapshotId', snapshot.id)
-            device1of2.should.have.property('targetSnapshotId', snapshot.id)
+            device2of2.should.have.property('targetSnapshotId', snapshot.id)
 
-            // create a new snapshot
-            const newSnapshot = await factory.createSnapshot({ name: generateName('snapshot') }, TestObjects.instance, TestObjects.bob)
+            // create a new snapshot on an instance in the group's own application -
+            // TestObjects.instance lives in a different BTeam application and would
+            // now be rejected by the application-scope check
+            const ownInstance = await factory.createInstance({ name: generateName('instance') }, application, app.stack, app.template, app.projectType, { start: false })
+            const newSnapshot = await factory.createSnapshot({ name: generateName('snapshot') }, ownInstance, TestObjects.bob)
 
             // now call the API to update targetSnapshot only
             const response = await callUpdate(sid, application, deviceGroup, {
@@ -529,6 +532,84 @@ describe('Application Device Groups API', function () {
             app.comms.devices.sendCommand.callCount.should.equal(0) // no devices should have been sent an update
         })
 
+        // Bob (BTeam owner) should not be able to point his BTeam device group at an ATeam snapshot.
+        it('Rejects a targetSnapshot that belongs to another team (400)', async function () {
+            const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare()
+
+            // Create an application, instance and snapshot owned by ATeam (foreign to bob's BTeam group)
+            const foreignApplication = await factory.createApplication({ name: generateName('foreign-app') }, TestObjects.ATeam)
+            const foreignInstance = await factory.createInstance({ name: generateName('foreign-instance') }, foreignApplication, app.stack, app.template, app.projectType, { start: false })
+            const foreignSnapshot = await factory.createSnapshot({ name: generateName('foreign-snapshot') }, foreignInstance, TestObjects.alice)
+
+            // now call the API to point the group at the foreign snapshot
+            const response = await callUpdate(sid, application, deviceGroup, {
+                targetSnapshotId: foreignSnapshot.hashid
+            })
+
+            // should fail
+            response.statusCode.should.equal(400)
+            response.json().should.have.property('code', 'invalid_input')
+
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+
+            // should not have updated the group or devices - still the original in-team snapshot
+            updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice1.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice2.should.have.property('targetSnapshotId', snapshot.id)
+
+            // check no devices got an update command
+            app.comms.devices.sendCommand.callCount.should.equal(0)
+        })
+
+        // Bob (BTeam) should not be able to point his BTeam device group at a same-team
+        // snapshot that belongs to a different BTeam application.
+        it('Rejects a targetSnapshot that belongs to a different application in the same team (400)', async function () {
+            const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare()
+            // Create a second BTeam application (same team, different application) with its own instance snapshot
+            const otherApplication = await factory.createApplication({ name: generateName('other-app') }, TestObjects.BTeam)
+            const otherInstance = await factory.createInstance({ name: generateName('other-instance') }, otherApplication, app.stack, app.template, app.projectType, { start: false })
+            const otherInstanceSnapshot = await factory.createSnapshot({ name: generateName('other-instance-snapshot') }, otherInstance, TestObjects.bob)
+            const response = await callUpdate(sid, application, deviceGroup, {
+                targetSnapshotId: otherInstanceSnapshot.hashid
+            })
+            response.statusCode.should.equal(400)
+            response.json().should.have.property('code', 'invalid_input')
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+            updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice1.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice2.should.have.property('targetSnapshotId', snapshot.id)
+            app.comms.devices.sendCommand.callCount.should.equal(0)
+        })
+        // Same scenario, but the foreign snapshot belongs to an application-owned device
+        // rather than an instance - exercises the device-owned branch of the fix.
+        it('Rejects a device-owned targetSnapshot that belongs to a different application in the same team (400)', async function () {
+            const { sid, application, deviceGroup, device1of2, device2of2, snapshot } = await prepare()
+            const otherApplication = await factory.createApplication({ name: generateName('other-app') }, TestObjects.BTeam)
+            const otherDevice = await factory.createDevice({ name: generateName('other-device') }, TestObjects.BTeam, null, otherApplication)
+            const otherDeviceSnapshot = await app.db.models.ProjectSnapshot.create({
+                name: generateName('other-device-snapshot'),
+                DeviceId: otherDevice.id,
+                settings: {},
+                flows: {}
+            })
+            const response = await callUpdate(sid, application, deviceGroup, {
+                targetSnapshotId: otherDeviceSnapshot.hashid
+            })
+            response.statusCode.should.equal(400)
+            response.json().should.have.property('code', 'invalid_input')
+            const updatedDeviceGroup = await app.db.models.DeviceGroup.byId(deviceGroup.hashid)
+            const updatedDevice1 = await app.db.models.Device.byId(device1of2.hashid)
+            const updatedDevice2 = await app.db.models.Device.byId(device2of2.hashid)
+            updatedDeviceGroup.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice1.should.have.property('targetSnapshotId', snapshot.id)
+            updatedDevice2.should.have.property('targetSnapshotId', snapshot.id)
+            app.comms.devices.sendCommand.callCount.should.equal(0)
+        })
+
         it('Cannot update a device group with empty name', async function () {
             const sid = await login('bob', 'bbPassword')
             const application = await factory.createApplication({ name: generateName('app') }, TestObjects.BTeam)