Skip to content

Commit

Permalink
Use the graceful deny mechanism for device state PATCH v3
Browse files Browse the repository at this point in the history
Change-type: major
  • Loading branch information
thgreasi committed Jul 12, 2023
1 parent 3dfba99 commit 87ecf8b
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 21 deletions.
15 changes: 13 additions & 2 deletions src/features/device-state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import { resolveOrDenyDevicesWithStatus } from './middleware';
import { stateV2 } from './routes/state-get-v2';
import { stateV3 } from './routes/state-get-v3';
import { statePatchV2 } from './routes/state-patch-v2';
import { statePatchV3 } from './routes/state-patch-v3';
import {
StatePatchV3Body,
resolveDeviceUuids,
statePatchV3,
} from './routes/state-patch-v3';
import { fleetStateV3 } from './routes/fleet-state-get-v3';
import { Device } from '../../balena-model';

Expand Down Expand Up @@ -49,7 +53,14 @@ export const setup = (app: Application) => {
middleware.authenticatedApiKey,
statePatchV2,
);
app.patch('/device/v3/state', middleware.authenticatedApiKey, statePatchV3);
app.patch(
'/device/v3/state',
resolveOrDenyDevicesWithStatus(401, (req) =>
resolveDeviceUuids(req.body as StatePatchV3Body),
),
middleware.authenticatedApiKey,
statePatchV3,
);
app.get(
'/device/v3/fleet/:fleetUuid/state',
middleware.authenticated,
Expand Down
36 changes: 26 additions & 10 deletions src/features/device-state/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Device } from '../../balena-model';

import { sbvrUtils, permissions } from '@balena/pinejs';
import { DEVICE_EXISTS_CACHE_TIMEOUT } from '../../lib/config';
import type { Request } from 'express-serve-static-core';

const { api } = sbvrUtils;

Expand Down Expand Up @@ -39,35 +40,50 @@ export const checkDeviceExistsIsFrozen = multiCacheMemoizee(
);

export interface ResolveDeviceInfoCustomObject {
resolvedDevice: Device['id'];
resolvedDeviceIds: Array<Device['id']>;
}

const requestParamsUuidResolver = (req: Request) => [req.params.uuid];

/**
* This checks if a device is deleted or frozen and responds according to the passed statusCode(s)
*/
export const resolveOrDenyDevicesWithStatus = (
statusCode: number | { deleted: number; frozen: number },
uuidResolver: (req: Request) => string[] = requestParamsUuidResolver,
): RequestHandler => {
const deletedStatusCode =
typeof statusCode === 'number' ? statusCode : statusCode.deleted;
const frozenStatusCode =
typeof statusCode === 'number' ? statusCode : statusCode.frozen;

return async (req, res, next) => {
const device = await checkDeviceExistsIsFrozen(req.params.uuid);
if (device == null) {
// Gracefully deny deleted devices
const uuids = uuidResolver(req);
if (!uuids.length) {
res.status(deletedStatusCode).end();
return;
}
if (device.is_frozen) {
// Gracefully deny frozen devices
res.status(frozenStatusCode).end();
return;
const deviceIds: number[] = [];
for (const uuid of uuids) {
const device = await checkDeviceExistsIsFrozen(uuid);
// Heads-up: if any of the provided devices is deleted/frozen
// then the whole request is rejected! We should revisit this
// if we later add again support for handling multiple devices
// per request.
if (device == null) {
// Gracefully deny deleted devices
res.status(deletedStatusCode).end();
return;
}
if (device.is_frozen) {
// Gracefully deny frozen devices
res.status(frozenStatusCode).end();
return;
}
deviceIds.push(device.id);
}

req.custom ??= {};
(req.custom as ResolveDeviceInfoCustomObject).resolvedDevice = device.id;
(req.custom as ResolveDeviceInfoCustomObject).resolvedDeviceIds = deviceIds;
next();
};
};
3 changes: 2 additions & 1 deletion src/features/device-state/routes/state-get-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ const stateQuery = _.once(() =>
);

const getStateV2 = async (req: Request, uuid: string): Promise<StateV2> => {
const deviceId = (req.custom as ResolveDeviceInfoCustomObject).resolvedDevice;
const [deviceId] = (req.custom as ResolveDeviceInfoCustomObject)
.resolvedDeviceIds;

const device = await getDevice(req, uuid);
const config = getConfig(device);
Expand Down
3 changes: 2 additions & 1 deletion src/features/device-state/routes/state-get-v3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ const stateQuery = _.once(() =>
);

const getStateV3 = async (req: Request, uuid: string): Promise<StateV3> => {
const deviceId = (req.custom as ResolveDeviceInfoCustomObject).resolvedDevice;
const [deviceId] = (req.custom as ResolveDeviceInfoCustomObject)
.resolvedDeviceIds;

const device = await getDevice(req, uuid);
const config = getConfig(device);
Expand Down
5 changes: 3 additions & 2 deletions src/features/device-state/routes/state-patch-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ export const statePatchV2: RequestHandler = async (req, res) => {
if (!uuid) {
throw new BadRequestError();
}
const { resolvedDevice: deviceId } =
req.custom as ResolveDeviceInfoCustomObject;
const {
resolvedDeviceIds: [deviceId],
} = req.custom as ResolveDeviceInfoCustomObject;
if (deviceId == null) {
// We are supposed to have already checked this.
throw new UnauthorizedError();
Expand Down
21 changes: 16 additions & 5 deletions src/features/device-state/routes/state-patch-v3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
shouldUpdateMetrics,
truncateShortTextFields,
} from '../state-patch-utils';
import { ResolveDeviceInfoCustomObject } from '../middleware';

const { BadRequestError, UnauthorizedError, InternalRequestError } = errors;
const { api } = sbvrUtils;
Expand Down Expand Up @@ -80,7 +81,7 @@ export type StatePatchV3Body = {
const fetchData = async (
req: Express.Request,
custom: AnyObject,
uuids: string[],
deviceIds: number[],
appReleaseUuids: {
[appUuid: string]: {
releaseUuids: Set<string>;
Expand All @@ -97,7 +98,7 @@ const fetchData = async (
options: {
$select: ['id', 'uuid'],
$filter: {
uuid: { $in: uuids },
id: { $in: deviceIds },
},
$expand: {
belongs_to__application: {
Expand All @@ -110,7 +111,7 @@ const fetchData = async (
belongs_to__application: Array<Pick<Application, 'uuid'>>;
}
>;
if (devices.length !== uuids.length) {
if (devices.length !== deviceIds.length) {
throw new UnauthorizedError();
}

Expand Down Expand Up @@ -205,6 +206,9 @@ const fetchData = async (
return { devicesByUuid, images, releasesByAppUuid };
});

export const resolveDeviceUuids = (body: StatePatchV3Body) =>
Object.keys(body).filter((uuid) => body[uuid] != null);

export const statePatchV3: RequestHandler = async (req, res) => {
try {
const body = req.body as StatePatchV3Body;
Expand All @@ -215,7 +219,7 @@ export const statePatchV3: RequestHandler = async (req, res) => {
custom.ipAddress = getIP(req);
}

const uuids = Object.keys(body).filter((uuid) => body[uuid] != null);
const uuids = resolveDeviceUuids(body);
if (uuids.length === 0) {
throw new BadRequestError();
}
Expand Down Expand Up @@ -298,8 +302,15 @@ export const statePatchV3: RequestHandler = async (req, res) => {
}

if (apps != null || Object.keys(deviceBody).length > 0) {
const { resolvedDeviceIds } =
req.custom as ResolveDeviceInfoCustomObject;
// We lazily fetch the necessary data only if we absolutely must to avoid unnecessary work if it turns out we don't need it
data ??= await fetchData(req, custom, uuids, appReleasesCriteria);
data ??= await fetchData(
req,
custom,
resolvedDeviceIds,
appReleasesCriteria,
);
const { images, releasesByAppUuid } = data;
const device = data.devicesByUuid[uuid];

Expand Down

0 comments on commit 87ecf8b

Please sign in to comment.