Use regenerateKey action for POST /v1/regenerate-api-key

This also adds a 500 response with the old key if the API key
refresh was unsuccessful. Previously, if the key refresh was
unsuccessful, this would result in an UnhandledPromiseRejection.
This is a new interface.

Signed-off-by: Christina Ying Wang <christina@balena.io>

src/device-api/actions.ts

@@ -1,4 +1,6 @@
+import { getGlobalApiKey, refreshKey } from '.';
 import * as eventTracker from '../event-tracker';
+import * as deviceState from '../device-state';
 import log from '../lib/supervisor-console';
 import blink = require('../lib/blink');
 
@@ -36,3 +38,24 @@ export const identify = (ms: number = DEFAULT_IDENTIFY_DURATION) => {
 	blink.pattern.start();
 	setTimeout(blink.pattern.stop, ms);
 };
+
+/**
+ * Expires the supervisor's API key and generates a new one.
+ * Also communicates the new key to the balena API, if it's a key
+ * with global scope. The backend uses the global key to communicate
+ * with the Supervisor.
+ * Used by:
+ * - POST /v1/regenerate-api-key
+ */
+export const regenerateKey = async (oldKey: string) => {
+	const shouldReportUpdatedKey = oldKey === (await getGlobalApiKey());
+	const newKey = await refreshKey(oldKey);
+
+	if (shouldReportUpdatedKey) {
+		deviceState.reportCurrentState({
+			api_secret: newKey,
+		});
+	}
+
+	return newKey;
+};

src/device-api/index.ts

@@ -1,10 +1,8 @@
 import * as express from 'express';
-import * as _ from 'lodash';
 
 import * as middleware from './middleware';
 import * as apiKeys from './api-keys';
 import * as actions from './actions';
-import { reportCurrentState } from '../device-state';
 import proxyvisor from '../proxyvisor';
 import log from '../lib/supervisor-console';
 
@@ -59,30 +57,16 @@ export class SupervisorAPI {
 			return res.sendStatus(200);
 		});
 
-		// Expires the supervisor's API key and generates a new one.
-		// It also communicates the new key to the balena API.
 		this.api.post(
 			'/v1/regenerate-api-key',
-			async (req: apiKeys.AuthorizedRequest, res) => {
+			async (req: apiKeys.AuthorizedRequest, res, next) => {
-				await apiKeys.initialized();
+				const { apiKey: oldKey } = req.auth;
-
+				try {
-				// check if we're updating the cloud API key
+					const newKey = await actions.regenerateKey(oldKey);
-				const shouldUpdateCloudKey =
+					return res.status(200).send(newKey);
-					req.auth.apiKey === (await getGlobalApiKey());
+				} catch (e: unknown) {
-
+					next(e);
-				// regenerate the key...
-				const newKey = await apiKeys.refreshKey(req.auth.apiKey);
-
-				// if we need to update the cloud API with our new key
-				if (shouldUpdateCloudKey) {
-					// report the new key to the cloud API
-					reportCurrentState({
-						api_secret: newKey,
-					});
 				}
-
-				// return the value of the new key to the caller
-				res.status(200).send(newKey);
 			},
 		);
 

test/integration/device-api/actions.spec.ts

@@ -0,0 +1,34 @@
+import { expect } from 'chai';
+import { stub, SinonStub } from 'sinon';
+
+import * as deviceState from '~/src/device-state';
+import { getGlobalApiKey, generateScopedKey } from '~/src/device-api';
+import * as actions from '~/src/device-api/actions';
+
+describe('regenerates API keys', () => {
+	// Stub external dependency - current state report should be tested separately.
+	// api-key.ts methods are also tested separately.
+	beforeEach(() => stub(deviceState, 'reportCurrentState'));
+	afterEach(() => (deviceState.reportCurrentState as SinonStub).restore());
+
+	it("communicates new key to cloud if it's a global key", async () => {
+		const originalGlobalKey = await getGlobalApiKey();
+		const newKey = await actions.regenerateKey(originalGlobalKey);
+		expect(originalGlobalKey).to.not.equal(newKey);
+		expect(newKey).to.equal(await getGlobalApiKey());
+		expect(deviceState.reportCurrentState as SinonStub).to.have.been.calledOnce;
+		expect(
+			(deviceState.reportCurrentState as SinonStub).firstCall.args[0],
+		).to.deep.equal({
+			api_secret: newKey,
+		});
+	});
+
+	it("doesn't communicate new key if it's a service key", async () => {
+		const originalScopedKey = await generateScopedKey(111, 'main');
+		const newKey = await actions.regenerateKey(originalScopedKey);
+		expect(originalScopedKey).to.not.equal(newKey);
+		expect(newKey).to.not.equal(await getGlobalApiKey());
+		expect(deviceState.reportCurrentState as SinonStub).to.not.have.been.called;
+	});
+});

test/integration/device-api/v1.spec.ts

@@ -1,3 +1,4 @@
+import { expect } from 'chai';
 import * as express from 'express';
 import { SinonStub, stub } from 'sinon';
 import * as request from 'supertest';
@@ -63,4 +64,34 @@ describe('device-api/v1', () => {
 				.expect(200);
 		});
 	});
+
+	describe('POST /v1/regenerate-api-key', () => {
+		// Actions are tested elsewhere so we can stub the dependency here
+		beforeEach(() => stub(actions, 'regenerateKey'));
+		afterEach(() => (actions.regenerateKey as SinonStub).restore());
+
+		it('responds with 200 and valid new API key', async () => {
+			const oldKey = await deviceApi.getGlobalApiKey();
+			const newKey = 'my_new_key';
+			(actions.regenerateKey as SinonStub).resolves(newKey);
+
+			await request(api)
+				.post('/v1/regenerate-api-key')
+				.set('Authorization', `Bearer ${oldKey}`)
+				.expect(200)
+				.then((response) => {
+					expect(response.text).to.match(new RegExp(newKey));
+				});
+		});
+
+		it('responds with 503 if regenerate was unsuccessful', async () => {
+			const oldKey = await deviceApi.getGlobalApiKey();
+			(actions.regenerateKey as SinonStub).throws(new Error());
+
+			await request(api)
+				.post('/v1/regenerate-api-key')
+				.set('Authorization', `Bearer ${oldKey}`)
+				.expect(503);
+		});
+	});
 });

test/legacy/41-device-api-v1.spec.ts

@@ -16,7 +16,6 @@ import SupervisorAPI from '~/src/device-api';
 import * as deviceApi from '~/src/device-api';
 import * as apiBinder from '~/src/api-binder';
 import * as deviceState from '~/src/device-state';
-import * as apiKeys from '~/src/device-api/api-keys';
 import * as dbus from '~/lib/dbus';
 import * as updateLock from '~/lib/update-lock';
 import * as TargetState from '~/src/device-state/target-state';
@@ -672,72 +671,6 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 		});
 	});
 
-	describe('POST /v1/regenerate-api-key', () => {
-		it('returns a valid new API key', async () => {
-			const refreshKeySpy: SinonSpy = spy(apiKeys, 'refreshKey');
-
-			let newKey: string = '';
-
-			await request
-				.post('/v1/regenerate-api-key')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.expect(sampleResponses.V1.POST['/regenerate-api-key'].statusCode)
-				.then((response) =>
-					Promise.all([response, deviceApi.getGlobalApiKey()]),
-				)
-				.then(([response, globalApiKey]) => {
-					expect(response.body).to.deep.equal(
-						sampleResponses.V1.POST['/regenerate-api-key'].body,
-					);
-					expect(response.text).to.equal(globalApiKey);
-					newKey = response.text;
-					expect(refreshKeySpy.callCount).to.equal(1);
-				});
-
-			// Ensure persistence with future calls
-			await request
-				.post('/v1/blink')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${newKey}`)
-				.expect(sampleResponses.V1.POST['/blink'].statusCode);
-
-			refreshKeySpy.restore();
-		});
-
-		it('expires old API key after generating new key', async () => {
-			const oldKey: string = await deviceApi.getGlobalApiKey();
-
-			await request
-				.post('/v1/regenerate-api-key')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${oldKey}`)
-				.expect(sampleResponses.V1.POST['/regenerate-api-key'].statusCode);
-
-			await request
-				.post('/v1/restart')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${oldKey}`)
-				.expect(401);
-		});
-
-		it('communicates the new API key to balena API', async () => {
-			const reportStateSpy: SinonSpy = spy(deviceState, 'reportCurrentState');
-
-			await request
-				.post('/v1/regenerate-api-key')
-				.set('Accept', 'application/json')
-				.set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`)
-				.then(() => {
-					expect(reportStateSpy.callCount).to.equal(1);
-					// Further reportCurrentState tests should be in 05-device-state.spec.ts,
-					// but its test case seems to currently be skipped until interface redesign
-				});
-
-			reportStateSpy.restore();
-		});
-	});
-
 	describe('/v1/device/host-config', () => {
 		// Wrap GET and PATCH /v1/device/host-config tests in the same block to share
 		// common scoped variables, namely file paths and file content