diff --git a/src/device-api/actions.ts b/src/device-api/actions.ts index 6fbd53d1..bccb5f16 100644 --- a/src/device-api/actions.ts +++ b/src/device-api/actions.ts @@ -6,6 +6,7 @@ import * as messages from './messages'; import * as eventTracker from '../event-tracker'; import * as deviceState from '../device-state'; import * as logger from '../logger'; +import * as config from '../config'; import { App } from '../compose/app'; import * as applicationManager from '../compose/application-manager'; import * as serviceManager from '../compose/service-manager'; @@ -14,11 +15,13 @@ import { CompositionStepAction, generateStep, } from '../compose/composition-steps'; +import * as commitStore from '../compose/commit'; import { getApp } from '../device-state/db-format'; import * as TargetState from '../device-state/target-state'; import log from '../lib/supervisor-console'; import blink = require('../lib/blink'); import { lock } from '../lib/update-lock'; +import * as constants from '../lib/constants'; import { InternalInconsistencyError, NotFoundError, @@ -416,3 +419,37 @@ export const updateTarget = async (force: boolean = false) => { ); return false; }; + +/** + * Get application information for a single-container app, throwing if multicontainer + * Used by: + * - GET /v1/apps/:appId + */ +export const getSingleContainerApp = async (appId: number) => { + eventTracker.track('GET app (v1)', { appId }); + const apps = await applicationManager.getCurrentApps(); + const app = apps[appId]; + const service = app?.services?.[0]; + if (service == null) { + // This should return a 404 Not Found, but we can't change the interface now so keep it as a 400 + throw new BadRequestError('App not found'); + } + if (app.services.length > 1) { + throw new BadRequestError( + 'Some v1 endpoints are only allowed on single-container apps', + ); + } + + // Because we only have a single app, we can fetch the commit for that + // app, and maintain backwards compatability + const commit = await commitStore.getCommitForApp(appId); + + return { + appId, + commit, + containerId: service.containerId, + env: _.omit(service.config.environment, constants.privateAppEnvVars), + imageId: service.config.image, + releaseId: service.releaseId, + }; +}; diff --git a/src/device-api/v1.ts b/src/device-api/v1.ts index 8869d84a..aaba5ddd 100644 --- a/src/device-api/v1.ts +++ b/src/device-api/v1.ts @@ -17,9 +17,7 @@ import { isBadRequestError, } from '../lib/errors'; import * as hostConfig from '../host-config'; -import * as applicationManager from '../compose/application-manager'; import { CompositionStepAction } from '../compose/composition-steps'; -import * as commitStore from '../compose/commit'; const disallowedHostConfigPatchFields = ['local_ip', 'local_port']; @@ -107,49 +105,25 @@ router.post('/v1/shutdown', handleDeviceAction('shutdown')); router.get('/v1/apps/:appId', async (req: AuthorizedRequest, res, next) => { const appId = checkInt(req.params.appId); - eventTracker.track('GET app (v1)', { appId }); if (appId == null) { return res.status(400).send('Missing app id'); } + // handle the case where the appId is out of scope + if (!req.auth.isScoped({ apps: [appId] })) { + return res.status(401).json({ + status: 'failed', + message: 'Application is not available', + }); + } + try { - const apps = await applicationManager.getCurrentApps(); - const app = apps[appId]; - const service = app?.services?.[0]; - if (service == null) { - return res.status(400).send('App not found'); + const app = await actions.getSingleContainerApp(appId); + return res.json(app); + } catch (e: unknown) { + if (isBadRequestError(e) || isNotFoundError(e)) { + return res.status(e.statusCode).send(e.statusMessage); } - - // handle the case where the appId is out of scope - if (!req.auth.isScoped({ apps: [app.appId] })) { - return res.status(401).json({ - status: 'failed', - message: 'Unauthorized', - }); - } - - if (app.services.length > 1) { - return res - .status(400) - .send('Some v1 endpoints are only allowed on single-container apps'); - } - - // Because we only have a single app, we can fetch the commit for that - // app, and maintain backwards compatability - const commit = await commitStore.getCommitForApp(appId); - - // Don't return data that will be of no use to the user - const appToSend = { - appId, - commit, - containerId: service.containerId, - env: _.omit(service.config.environment, constants.privateAppEnvVars), - imageId: service.config.image, - releaseId: service.releaseId, - }; - - return res.json(appToSend); - } catch (e) { next(e); } }); diff --git a/test/integration/device-api/actions.spec.ts b/test/integration/device-api/actions.spec.ts index bb541b9e..a6afea0d 100644 --- a/test/integration/device-api/actions.spec.ts +++ b/test/integration/device-api/actions.spec.ts @@ -5,6 +5,7 @@ import * as request from 'supertest'; import { setTimeout } from 'timers/promises'; import * as deviceState from '~/src/device-state'; +import * as config from '~/src/config'; import * as deviceApi from '~/src/device-api'; import * as actions from '~/src/device-api/actions'; import * as TargetState from '~/src/device-state/target-state'; @@ -357,6 +358,26 @@ describe('manages application lifecycle', () => { }); }); + it('should return information about a single-container app', async () => { + containers = await waitForSetup(targetState); + const containerId = containers[0].Id; + const imageHash = containers[0].Config.Image; + + // Calling actions.getSingleContainerApp doesn't work because + // the action queries the database + const { body } = await request(BALENA_SUPERVISOR_ADDRESS).get( + '/v1/apps/1', + ); + + expect(body).to.have.property('appId', APP_ID); + expect(body).to.have.property('containerId', containerId); + expect(body).to.have.property('imageId', imageHash); + expect(body).to.have.property('releaseId', 1); + // Should return the environment of the single service + expect(body.env).to.have.property('BALENA_APP_ID', String(APP_ID)); + expect(body.env).to.have.property('BALENA_SERVICE_NAME', serviceNames[0]); + }); + // This test should be ordered last in this `describe` block, because the test compares // the `CreatedAt` timestamps of volumes to determine whether purge was successful. Thus, // ordering the assertion last will ensure some time has passed between the first `CreatedAt` diff --git a/test/integration/device-api/v1.spec.ts b/test/integration/device-api/v1.spec.ts index 2523a561..a98feaa8 100644 --- a/test/integration/device-api/v1.spec.ts +++ b/test/integration/device-api/v1.spec.ts @@ -684,4 +684,64 @@ describe('device-api/v1', () => { .expect(202); }); }); + + describe('GET /v1/apps/:appId', () => { + let getSingleContainerAppStub: SinonStub; + beforeEach(() => { + getSingleContainerAppStub = stub( + actions, + 'getSingleContainerApp', + ).resolves({} as any); + }); + afterEach(async () => { + getSingleContainerAppStub.restore(); + // Remove all scoped API keys between tests + await db.models('apiSecret').whereNot({ appId: 0 }).del(); + }); + + it('validates data from request body', async () => { + await request(api) + .get('/v1/apps/1234567') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`); + expect(getSingleContainerAppStub).to.have.been.calledWith(1234567); + }); + + it('responds with 200 if request successful', async () => { + await request(api) + .get('/v1/apps/1234567') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(200, {}); + }); + + it('responds with 400 if invalid appId parameter', async () => { + await request(api) + .get('/v1/apps/badAppId') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(400); + }); + + it('responds with 400 if action throws BadRequestError', async () => { + getSingleContainerAppStub.throws(new BadRequestError()); + await request(api) + .get('/v1/apps/1234567') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(400); + }); + + it("responds with 401 if caller's API key is not in scope of appId", async () => { + const scopedKey = await deviceApi.generateScopedKey(7654321, 'main'); + await request(api) + .get('/v1/apps/1234567') + .set('Authorization', `Bearer ${scopedKey}`) + .expect(401); + }); + + it('responds with 503 for other errors that occur during request', async () => { + getSingleContainerAppStub.throws(new Error()); + await request(api) + .get('/v1/apps/1234567') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(503); + }); + }); }); diff --git a/test/legacy/41-device-api-v1.spec.ts b/test/legacy/41-device-api-v1.spec.ts index 974f29dd..67d7ed0c 100644 --- a/test/legacy/41-device-api-v1.spec.ts +++ b/test/legacy/41-device-api-v1.spec.ts @@ -107,50 +107,6 @@ describe('SupervisorAPI [V1 Endpoints]', () => { loggerStub.restore(); }); - describe('GET /v1/apps/:appId', () => { - it('does not return information for an application when there is more than 1 container', async () => { - await request - .get('/v1/apps/2') - .set('Accept', 'application/json') - .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) - .expect( - sampleResponses.V1.GET['/apps/2 [Multiple containers running]'] - .statusCode, - ); - }); - - it('returns information about a specific application', async () => { - // Setup single container application - const container = mockedAPI.mockService({ - containerId: 'abc123', - appId: 2, - releaseId: 77777, - }); - const image = mockedAPI.mockImage({ - appId: 2, - }); - appMock.mockManagers([container], [], []); - appMock.mockImages([], false, [image]); - await mockedDockerode.testWithData( - { containers: [container], images: [image] }, - async () => { - // Make request - await request - .get('/v1/apps/2') - .set('Accept', 'application/json') - .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) - .expect(sampleResponses.V1.GET['/apps/2'].statusCode) - .expect('Content-Type', /json/) - .then((response) => { - expect(response.body).to.deep.equal( - sampleResponses.V1.GET['/apps/2'].body, - ); - }); - }, - ); - }); - }); - describe('GET /v1/device', () => { it('returns MAC address', async () => { const response = await request