diff --git a/lib/db/models/all/model.js b/lib/db/models/all/model.js index 958226e6b4..658ca680ec 100644 --- a/lib/db/models/all/model.js +++ b/lib/db/models/all/model.js @@ -188,13 +188,14 @@ export const lockDeviceByCurrent = function(groups, serial) { 'group.lock': { $cond: [ { + // A device without the field is not locked $and: [ - {$eq: ['$group?.lock', false]}, - {$not: [{$eq: [{$setIntersection: [groups, ['$group?.id']]}, []]}]} + {$ne: ['$group.lock', true]}, + {$in: ['$group.id', groups]} ] }, true, - '$group?.lock' + '$group.lock' ] } } @@ -231,13 +232,14 @@ export const lockDeviceByOrigin = function(groups, serial) { 'group.lock': { $cond: [ { + // A device without the field is not locked $and: [ - {$eq: ['$group?.lock', false]}, - {$not: [{$eq: [{$setIntersection: [groups, ['$group?.origin']]}, []]}]} + {$ne: ['$group.lock', true]}, + {$in: ['$group.origin', groups]} ] }, true, - '$group?.lock' + '$group.lock' ] } } @@ -1013,7 +1015,9 @@ export const saveUserAccessToken = function(email, token) { email: email, id: token.id, title: token.title, - jwt: token.jwt + jwt: token.jwt, + // Expiration time in ms, null if it never expires + expiresAt: token.expiresAt ?? null }).then(function(result) { if (result.insertedId) { return tokenId @@ -1346,16 +1350,16 @@ export const updateDeviceGroupName = async(serial, group) => { $eq: [group?.isActive, true] }, group?.name, - '$group?.name' + '$group.name' ] }, { $cond: [ { - $eq: ['$group?.origin', '$group?.id'] + $eq: ['$group.origin', '$group.id'] }, group?.name, - '$group?.name' + '$group.name' ] } ] @@ -1366,7 +1370,7 @@ export const updateDeviceGroupName = async(serial, group) => { $eq: [apiutil.isOriginGroup(group?.class), true] }, group?.name, - '$group?.originName' + '$group.originName' ] } } diff --git a/lib/db/models/user/model.js b/lib/db/models/user/model.js index 7ec2d2c3a5..984f48403f 100644 --- a/lib/db/models/user/model.js +++ b/lib/db/models/user/model.js @@ -25,13 +25,6 @@ export const loadUser = function(email) { return db.users.findOne({email: email}) } -// dbapi.lookupUsersByAdbKey = function(fingerprint) { -export const lookupUsersByAdbKey = function(fingerprint) { - return db.users.find({ - adbKeys: fingerprint - }).toArray() -} - // dbapi.lookupUserByAdbFingerprint = function(fingerprint) { export const lookupUserByAdbFingerprint = function(fingerprint) { return db.users.find( @@ -193,19 +186,16 @@ export const updateUserSettings = async function(email, changes) { } // dbapi.insertUserAdbKey = function(email, key) { -export const insertUserAdbKey = function(email, key) { - let data = { - title: key.title, - fingerprint: key.fingerprint - } - return db.users.findOne({email: email}).then(user => { - let adbKeys = user?.adbKeys ? user?.adbKeys : [] - adbKeys.push(data) - return db.users.updateOne( - {email: email} - , {$set: {adbKeys: user?.adbKeys ? adbKeys : [data]}} - ) - }) +/** + * One atomic update, so concurrent additions do not overwrite each other. + * @returns {Promise} false if the user already has a key with this fingerprint + */ +export const insertUserAdbKey = async function(email, key) { + const result = await db.users.updateOne( + {email: email, 'adbKeys.fingerprint': {$ne: key.fingerprint}} + , {$push: {adbKeys: {title: key.title, fingerprint: key.fingerprint}}} + ) + return result.modifiedCount > 0 } // dbapi.grantAdmin = function(email) { diff --git a/lib/units/api/controllers/user.js b/lib/units/api/controllers/user.js index 4274ff5565..3874993d5b 100644 --- a/lib/units/api/controllers/user.js +++ b/lib/units/api/controllers/user.js @@ -9,7 +9,7 @@ import * as apiutil from '../../../util/apiutil.js' import * as lockutil from '../../../util/lockutil.js' import * as Sentry from '@sentry/node' import generateToken from '../helpers/generateToken.js' -import {AdbKeysUpdatedMessage, ConnectStartMessage, ConnectStopMessage, GroupMessage, OwnerMessage, UpdateAccessTokenMessage, UngroupMessage} from '../../../wire/wire.js' +import {ConnectStartMessage, ConnectStopMessage, GroupMessage, OwnerMessage, UpdateAccessTokenMessage, UngroupMessage} from '../../../wire/wire.js' let log = logger.createLogger('api:controllers:user') function getUser(req, res) { @@ -118,7 +118,8 @@ function addUserDevice(req, res) { }), usage, timeout: timeout || undefined, - keys: req.user.adbKeys.map(key => key.fingerprint) + // Absent for a user who never added an ADB key + keys: (req.user.adbKeys ?? []).map(key => key.fingerprint) } ) log.info(device.serial + ' added to user group ' + req.user.email) @@ -128,6 +129,7 @@ function addUserDevice(req, res) { }) } catch (err) { + log.error('Failed to add device "%s" to user group %s: %s', device.serial, req.user.email, err?.message ?? err) return apiutil.respond(res, 504, 'Device is not responding') } } @@ -183,16 +185,31 @@ function deleteUserDeviceBySerial(req, res) { } } - await req.options.txmanager.runTransaction( - device.provider.name, - device.serial, - UngroupMessage, - { - requirements: wireutil.toDeviceRequirements({ - serial: {value: serial, match: 'exact'} - }) - } - ) + try { + await req.options.txmanager.runTransaction( + device.provider.name, + device.serial, + UngroupMessage, + { + requirements: wireutil.toDeviceRequirements({ + serial: {value: serial, match: 'exact'} + }) + } + ) + } + catch (err) { + log.error('Failed to release device "%s": %s', serial, err?.message ?? err) + return isInternal ? false : apiutil.respond(res, 504, 'Device is not responding') + } + + // An internal call releases the devices of another request, which answers on its own + if (isInternal) { + return true + } + return res.json({ + success: true, + description: 'Device successfully removed' + }) }) .catch(function(err) { let errSerial @@ -207,7 +224,7 @@ function deleteUserDeviceBySerial(req, res) { return false } else { - apiutil.respond(res, 500, 'Internal Server Error' + err.message ? `: ${err.message}` : '') + apiutil.respond(res, 500, 'Internal Server Error' + (err.message ? `: ${err.message}` : '')) } }) } @@ -339,50 +356,24 @@ function getUserAccessTokens(req, res) { async function addAdbPublicKey(req, res) { const data = req.body + let key try { - // Parse the public key - const key = await Adb.util.parsePublicKey(data.publickey) - - // Look up users by fingerprint - const adbKeys = await dbapi.lookupUsersByAdbKey(key.fingerprint) - - const responseData = { - key: { - title: data.title || key.comment, - fingerprint: key.fingerprint - }, - users: adbKeys - } - - if (responseData.users.length) { - res.json({ - success: true, - fingerprint: responseData.key.fingerprint - }) - } - else { - try { - await dbapi.insertUserAdbKey(req.user.email, responseData.key) - - res.json({ - success: true, - fingerprint: responseData.key.fingerprint - }) - } - catch (err) { - if (err instanceof dbapi.DuplicateSecondaryIndexError) { - return res.status(208).json({ - success: true, - message: 'Key was already added' - }) - } - throw err // rethrow so it goes to the outer catch - } - } - - // Broadcast the update; the websocket unit fans it to interested clients. - req.options.transport.sendBroadcast(wireutil.pack(AdbKeysUpdatedMessage, {})) + key = await Adb.util.parsePublicKey(data?.publickey) + } + catch { + return apiutil.respond(res, 400, 'Invalid ADB public key') + } + try { + // Adding a key the user already has changes nothing + await dbapi.insertUserAdbKey(req.user.email, { + title: data.title || key.comment, + fingerprint: key.fingerprint + }) + res.json({ + success: true, + fingerprint: key.fingerprint + }) } catch (err) { log.error('Failed to insert a new ADB key fingerprint: ', err.stack) @@ -464,11 +455,12 @@ function getAccessTokens(req, res) { function createAccessToken(req, res) { const title = req.query.title - const token = generateToken(req.user, req.options.secret) + const token = generateToken(req.user, req.options.secret, Number(req.query.lifetime ?? 0)) dbapi.saveUserAccessToken(req.user.email, { title: title, id: token.id, - jwt: token.jwt + jwt: token.jwt, + expiresAt: token.expiresAt }) .then(function() { req.options.transport.sendBroadcast(wireutil.pack(UpdateAccessTokenMessage, {})) diff --git a/lib/units/api/helpers/generateToken.js b/lib/units/api/helpers/generateToken.js index 522b7781ad..69a252faa9 100644 --- a/lib/units/api/helpers/generateToken.js +++ b/lib/units/api/helpers/generateToken.js @@ -1,17 +1,29 @@ import * as jwtutil from '../../../util/jwtutil.js' +import {ONE_MONTH} from '../../../util/apiutil.js' import util from 'util' import {v4 as uuidv4} from 'uuid' -const generateToken = (/** @type {{ email: string; name: string; }} */ user, /** @type {string} */ secret) => { +/* Access token lifetimes in months; 0 never expires */ +export const ACCESS_TOKEN_LIFETIMES = [0, 1, 2, 3] + +const generateToken = (/** @type {{ email: string; name: string; }} */ user, /** @type {string} */ secret, lifetime = 0) => { + if (!ACCESS_TOKEN_LIFETIMES.includes(lifetime)) { + throw new Error(`Invalid access token lifetime: ${lifetime}`) + } + + const id = util.format('%s-%s', uuidv4(), uuidv4()).replace(/-/g, '') + const expiresAt = lifetime ? Date.now() + lifetime * ONE_MONTH : null const jwt = jwtutil.encode({ payload: { email: user.email, - name: user.name + name: user.name, + // Lets the token be revoked by deleting it + jti: id }, - secret: secret + secret: secret, + exp: expiresAt }) - const id = util.format('%s-%s', uuidv4(), uuidv4()).replace(/-/g, '') - return {id: id, jwt: jwt} + return {id: id, jwt: jwt, expiresAt: expiresAt} } export default generateToken diff --git a/lib/units/api/helpers/securityHandlers.js b/lib/units/api/helpers/securityHandlers.js index f497b6809f..a2318769e8 100644 --- a/lib/units/api/helpers/securityHandlers.js +++ b/lib/units/api/helpers/securityHandlers.js @@ -3,10 +3,13 @@ import dbapi from '../../../db/api.js' import * as jwtutil from '../../../util/jwtutil.js' import logger from '../../../util/logger.js' import * as apiutil from '../../../util/apiutil.js' +import {createAccessTokenCheck} from '../../../util/access-token-check.js' const log = logger.createLogger('api:helpers:securityHandlers') let upsertQueue +const accessTokenExists = createAccessTokenCheck(id => dbapi.loadAccessTokenById(id).then(Boolean)) + // Specifications: https://tools.ietf.org/html/rfc6750#section-2.1 async function accessTokenAuth(req, res, next) { let operationTag @@ -46,6 +49,7 @@ async function accessTokenAuth(req, res, next) { } let data + let loaded = false try { data = jwtutil.decode(tokenId, req.options.secret) } @@ -58,6 +62,7 @@ async function accessTokenAuth(req, res, next) { } } data = jwtutil.decode(token.jwt, req.options.secret) + loaded = true } if (!data?.email) { @@ -67,6 +72,13 @@ async function accessTokenAuth(req, res, next) { } } + if (data.jti && !loaded && !await accessTokenExists(data.jti)) { + throw { + status: 401, + message: 'Revoked token' + } + } + await upsertQueue const user = await dbapi.loadUser(data.email) @@ -130,7 +142,7 @@ async function accessTokenAuth(req, res, next) { } } try { - const user = dbapi.loadUser(data.email) + const user = await dbapi.loadUser(data.email) if (user) { if (isNotAllowed(user)) { throw forbidden diff --git a/lib/units/api/swagger/api_v1.yaml b/lib/units/api/swagger/api_v1.yaml index 13d7877277..8f67d531f7 100644 --- a/lib/units/api/swagger/api_v1.yaml +++ b/lib/units/api/swagger/api_v1.yaml @@ -1702,6 +1702,14 @@ paths: required: true schema: type: string + - name: lifetime + in: query + description: Access token lifetime in months (30 days each). 0 never expires + required: false + schema: + type: integer + enum: [0, 1, 2, 3] + default: 0 responses: "200": description: Access token information @@ -2153,6 +2161,14 @@ paths: required: true schema: type: string + - name: lifetime + in: query + description: Access token lifetime in months (30 days each). 0 never expires + required: false + schema: + type: integer + enum: [0, 1, 2, 3] + default: 0 responses: "201": description: Access token information @@ -4651,6 +4667,10 @@ components: type: string jwt: type: string + expiresAt: + type: number + nullable: true + description: Expiration time (ms since epoch), null if the token never expires UserAccessTokenResponse: required: - description diff --git a/lib/units/auth/oauth2/index.js b/lib/units/auth/oauth2/index.js index f613f7bf42..12f7f4998e 100644 --- a/lib/units/auth/oauth2/index.js +++ b/lib/units/auth/oauth2/index.js @@ -9,12 +9,10 @@ import rateLimitConfig from '../../ratelimit/index.js' import db from '../../../db/index.js' import dbapi from '../../../db/api.js' import {ONE_DAY} from '../../../util/apiutil.js' -export default (async function(options) { - var log = logger.createLogger('auth-oauth2') - var app = express() - var server = http.createServer(app) +const log = logger.createLogger('auth-oauth2') - await db.connect() +export const createApp = function(options) { + var app = express() app.set('strict routing', true) app.set('case sensitive routing', true) @@ -35,12 +33,20 @@ export default (async function(options) { }) }) }) + app.get('/auth/docs', function(req, res) { + res.status(200) + .json({ + success: true, + docsUrl: options.docsUrl + }) + }) function verify(accessToken, refreshToken, profile, done) { done(null, profile) } passport.use(new Strategy(options.oauth, verify)) app.use(rateLimitConfig) app.use(passport.initialize()) + // Every route below starts or completes a login: plain requests such as /auth/docs go above app.use(passport.authenticate('oauth2', { failureRedirect: '/auth/oauth/', session: false @@ -74,6 +80,13 @@ export default (async function(options) { res.render('rejected-email') } }) + return app +} + +export default (async function(options) { + await db.connect() + + const server = http.createServer(createApp(options)) server.listen(options.port) log.info('Listening on port %d', options.port) }) diff --git a/lib/units/base-device/plugins/group.ts b/lib/units/base-device/plugins/group.ts index ad6967add8..392650c996 100755 --- a/lib/units/base-device/plugins/group.ts +++ b/lib/units/base-device/plugins/group.ts @@ -36,11 +36,9 @@ interface GroupEvents { autojoin: [ADBKey, Joined] error: [string] timeout: [] - keys: [ADBKey[]] } export class GroupManager extends EventEmitter { - updateKeys(keys: ADBKey[]) { this.emit('keys', keys) } private currentOwner: OwnerState | null = null constructor(private inactivity: InactivityMonitor) { super() } diff --git a/lib/units/base-device/support/screen-ws-url.ts b/lib/units/base-device/support/screen-ws-url.ts new file mode 100644 index 0000000000..ea366a524b --- /dev/null +++ b/lib/units/base-device/support/screen-ws-url.ts @@ -0,0 +1,13 @@ +import _ from 'lodash' + +export interface ScreenWsUrlOptions { + screenWsUrlPattern: string + publicIp: string + screenPort: number + serial: string + provider: string +} + +/* The URL clients open the screen WebSocket at, from the --screen-ws-url-pattern template */ +export const screenWsUrl = ({screenWsUrlPattern, publicIp, screenPort, serial, provider}: ScreenWsUrlOptions): string => + _.template(screenWsUrlPattern)({publicIp, publicPort: screenPort, serial, provider}) diff --git a/lib/units/base-device/support/silent-runtime.ts b/lib/units/base-device/support/silent-runtime.ts index c60a9e5088..cba1106fc8 100644 --- a/lib/units/base-device/support/silent-runtime.ts +++ b/lib/units/base-device/support/silent-runtime.ts @@ -9,7 +9,6 @@ import * as wire from '../../../wire/wire.js' export interface SilentGroup { joinExclusive(owner: {email: string; name: string; group: string}, keys: string[], timeout: number): Promise leaveExclusive(reason: string): Promise - updateKeys?(keys: string[]): void } export class SilentError extends Error { @@ -111,7 +110,6 @@ export class SilentDeviceRuntime { } if (this.phase === 'owned') { - this.group?.updateKeys?.(actor.adbKeys) this.inactivity.keepalive() return this.acquired(actor) } diff --git a/lib/units/device/plugins/connect.ts b/lib/units/device/plugins/connect.ts index 71dd4a5c3d..a5aa6a9fe7 100644 --- a/lib/units/device/plugins/connect.ts +++ b/lib/units/device/plugins/connect.ts @@ -6,7 +6,6 @@ import * as streamutil from '../../../util/streamutil.js' import adb from '../support/adb.js' import connector, {DEVICE_TYPE} from '../../base-device/support/connector.js' import transport from '../../base-device/support/transport.js' -import router from '../../base-device/support/router.js' import group from './group.js' import solo from './solo.js' import urlformat from '../../base-device/support/urlformat.js' @@ -14,7 +13,6 @@ import identity from './util/identity.js' import data from './util/data.js' import minirev from '../resources/minirev.js' import type TcpUsbServer from '@u4/adbkit/dist/adb/tcpusb/server.d.ts' -import {AdbKeysUpdatedMessage} from '../../../wire/wire.js' import {attachReverse, type ReverseSession} from './adb-reverse/session.ts' interface Key { @@ -24,7 +22,6 @@ interface Key { export default syrup.serial() .dependency(adb) - .dependency(router) .dependency(transport) .dependency(group) .dependency(solo) @@ -33,7 +30,7 @@ export default syrup.serial() .dependency(identity) .dependency(data) .dependency(minirev) - .define(async(options, adb, router, transport, group, solo, urlformat, connector, identity, data, minirev) => { + .define(async(options, adb, transport, group, solo, urlformat, connector, identity, data, minirev) => { const log = logger.createLogger('device:plugins:connect') let activeServer: TcpUsbServer | null = null const activeSessions = new Set() @@ -138,9 +135,6 @@ export default syrup.serial() log.info('Stop connect plugin') - // TODO: Not ideal way, need WireRouter.once - router.removeAllListeners(AdbKeysUpdatedMessage) - const waitServerClose = new Promise((resolve) => { activeServer!.on('close', () => { resolve() @@ -177,7 +171,6 @@ export default syrup.serial() } } group.on('join', e => setAllowedKeys(e.adbKeys)) - if (options.silent) group.on('keys', setAllowedKeys) group.on('leave', () => { for (const session of activeSessions) { diff --git a/lib/units/device/plugins/screen/options.js b/lib/units/device/plugins/screen/options.js index 50853dfef7..729ddc2f1d 100644 --- a/lib/units/device/plugins/screen/options.js +++ b/lib/units/device/plugins/screen/options.js @@ -1,14 +1,10 @@ import syrup from '@devicefarmer/stf-syrup' -import _ from 'lodash' +import {screenWsUrl} from '../../../base-device/support/screen-ws-url.js' export default syrup.serial() .define(function(options) { var plugin = Object.create(null) plugin.devicePort = 9002 plugin.publicPort = options.screenPort - plugin.publicUrl = _.template(options.screenWsUrlPattern)({ - publicIp: options.publicIp, - publicPort: plugin.publicPort, - serial: options.serial - }) + plugin.publicUrl = screenWsUrl(options) return plugin }) diff --git a/lib/units/device/support/adb.js b/lib/units/device/support/adb.js index 0255a91734..ae125164f3 100644 --- a/lib/units/device/support/adb.js +++ b/lib/units/device/support/adb.js @@ -2,6 +2,32 @@ import syrup from '@devicefarmer/stf-syrup' import {createClient} from '@u4/adbkit' import logger from '../../../util/logger.js' import * as promiseutil from '../../../util/promiseutil.js' + +/* + * The provider owns the ADB server (see its --adb-autostart). By default adbkit + * runs `adb start-server` whenever a connection is refused: every worker would + * then start its own server along with the provider, and the losers of that race + * would linger or, with another adb version in PATH, kill the winner. + */ +export const createWorkerAdbClient = ({adbHost, adbPort}) => { + const client = createClient({ + host: adbHost, + port: adbPort, + bin: '' + }) + + // Without a binary adbkit reports a refused connection as "No bin specified" + const connection = client.connection.bind(client) + client.connection = () => connection().catch(err => { + if (err?.message === 'No bin specified') { + throw new Error(`ADB server ${adbHost}:${adbPort} is not running`) + } + throw err + }) + + return client +} + export default syrup.serial() .define( @@ -11,10 +37,7 @@ export default syrup.serial() function adbmodule(options) { var log = logger.createLogger('device:support:adb') - const adb = createClient({ - host: options.adbHost, - port: options.adbPort - }) + const adb = createWorkerAdbClient(options) function ensureBootComplete() { return promiseutil.periodicNotify(adb.getDevice(options.serial).waitBootComplete(), 1000) .progressed(function() { diff --git a/lib/units/ios-device/plugins/info.ts b/lib/units/ios-device/plugins/info.ts index b36bc1ef62..5eeb43d535 100644 --- a/lib/units/ios-device/plugins/info.ts +++ b/lib/units/ios-device/plugins/info.ts @@ -1,8 +1,8 @@ import syrup from '@devicefarmer/stf-syrup' import wireutil from '../../../wire/util.js' import logger from '../../../util/logger.js' -import _ from 'lodash' import transport from '../../base-device/support/transport.js' +import {screenWsUrl} from '../../base-device/support/screen-ws-url.js' import {getModelName} from './util/iosutil.js' import wdaClient from './wda/client.js' import { @@ -104,11 +104,7 @@ export default syrup.serial() wireutil.global, wireutil.pack(SizeIosDevice, { id: options.serial, - url: _.template(options.screenWsUrlPattern || '')({ - publicIp: options.publicIp, - publicPort: options.screenPort, - serial: options.serial - }), + url: screenWsUrl({...options, screenWsUrlPattern: options.screenWsUrlPattern || ''}), ...display }) ]) diff --git a/lib/units/provider/ADBObserver.ts b/lib/units/provider/ADBObserver.ts index 3d80c8b107..4a1af9391f 100644 --- a/lib/units/provider/ADBObserver.ts +++ b/lib/units/provider/ADBObserver.ts @@ -33,6 +33,7 @@ interface ADBObserverOptions { adbAutostart?: boolean adbPath?: string idleReconnectMinutes?: number + requestTimeoutMs?: number } interface ADBEvents { @@ -139,6 +140,7 @@ class ADBObserver extends EventEmitter { this.healthCheckIntervalMs = options?.healthCheckIntervalMs || this.healthCheckIntervalMs this.host = options?.host || this.host this.port = options?.port || this.port + this.requestTimeoutMs = options?.requestTimeoutMs || this.requestTimeoutMs this.adbAutostart = options?.adbAutostart ?? this.adbAutostart this.adbPath = options?.adbPath || this.adbPath @@ -275,7 +277,7 @@ class ADBObserver extends EventEmitter { */ async connect(host: string, port: number, timeoutMs = 15_000): Promise { const serial = deviceSerial(host, port) - const response = await this.sendADBCommand(`host:connect:${serial}`, undefined, {timeoutMs}) + const response = await this.hostRequest(`host:connect:${serial}`, timeoutMs) if (!/^(already )?connected to /i.test(response.trim())) { throw new Error(response.trim() || `Unable to connect to ${serial}`) @@ -299,6 +301,10 @@ class ADBObserver extends EventEmitter { throw err } } + + // Forget the device now rather than on the next poll: until then it would + // look connected to whoever asks. A failed poll is reported by polling itself + await this.refresh().catch(() => {}) } /** @@ -899,6 +905,57 @@ class ADBObserver extends EventEmitter { return buffer } + /** + * A host service on a connection of its own, outside the request queue. + * Meant for slow ones: `host:connect` to an unreachable host takes adb over + * a minute, which must hold up neither polling nor health checks. The ADB + * server serves connections concurrently. + */ + private hostRequest(command: string, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + let buffer = Buffer.alloc(0) + let settled = false + const settle = (err: Error | null, response = '') => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + // Also abandons a reply that is still to come + socket.destroy() + if (err) { + reject(err) + } + else { + resolve(response) + } + } + + const timer = setTimeout(() => settle(new Error(`Request timeout after ${timeoutMs}ms: ${command}`)), timeoutMs) + const socket = net.createConnection({port: this.port, host: this.host, noDelay: true}, () => { + const body = Buffer.from(command, 'utf-8') + socket.write(Buffer.concat([Buffer.from(body.length.toString(16).padStart(4, '0'), 'ascii'), body])) + }) + + // OKAY or FAIL, then a length-prefixed message + socket.on('data', data => { + buffer = Buffer.concat([buffer, data]) + if (buffer.length < 8) { + return + } + const length = parseInt(buffer.subarray(4, 8).toString('ascii'), 16) + if (buffer.length < 8 + length) { + return + } + const message = buffer.subarray(8, 8 + length).toString('utf-8') + const status = buffer.subarray(0, 4).toString('ascii') + settle(status === 'OKAY' ? null : new Error(message || 'ADB command failed'), message) + }) + socket.on('error', err => settle(err)) + socket.on('close', () => settle(new Error(`Connection closed: ${command}`))) + }) + } + /** * Send command to ADB server. * Requests are queued and processed sequentially, one connection per request. @@ -918,8 +975,9 @@ class ADBObserver extends EventEmitter { needData: false, resolve: () => { // After transport succeeds, socket is now a tunnel to device's adbd - // Device commands (shell:, logcat:, etc.) return raw streams, not length-prefixed data - this.requestQueue.push({ + // Device commands (shell:, logcat:, etc.) return raw streams, not length-prefixed data. + // It goes first: any other request queued meanwhile would be sent to the device + this.requestQueue.unshift({ command, resolve, reject, @@ -962,6 +1020,11 @@ class ADBObserver extends EventEmitter { const index = this.requestQueue.indexOf(request) if (index !== -1) { this.requestQueue.splice(index, 1) + if (request.sent) { + // ADB may still answer it on this socket: the next request + // would take that late reply for its own + this.closeConnectionAfterReply() + } reject(new Error(`Request timeout after ${timeoutMs}ms: ${command}`)) // Process next request in queue this.processNextRequest() @@ -1206,7 +1269,7 @@ class ADBObserver extends EventEmitter { } } - await this.sendADBCommand(`host:connect:${device.serial}`) + await this.hostRequest(`host:connect:${device.serial}`, 15_000) } else { // A USB device can only be reconnected while it still has diff --git a/lib/units/provider/remote-devices/api.ts b/lib/units/provider/remote-devices/api.ts index fd7f4f78e6..2e41092d3a 100644 --- a/lib/units/provider/remote-devices/api.ts +++ b/lib/units/provider/remote-devices/api.ts @@ -24,6 +24,15 @@ export const createProviderApi = ({host, port, providerName, devices}: ProviderA res.json({devices: devices.list(), webhooks: devices.webhooks.subscriptions()}) }) + app.get(`${basePath}/:serial`, (req, res) => { + const device = devices.get(req.params.serial) + if (!device) { + res.status(404).json({error: 'Device is not connected through the API'}) + return + } + res.json({device}) + }) + app.post(basePath, (req, res) => { const {serial} = devices.connect(parseConnectRequest(req.body)) res.status(202).json({provider: providerName, serial}) diff --git a/lib/units/provider/remote-devices/manager.ts b/lib/units/provider/remote-devices/manager.ts index 7a1cad42e8..5fe832dcc0 100644 --- a/lib/units/provider/remote-devices/manager.ts +++ b/lib/units/provider/remote-devices/manager.ts @@ -16,7 +16,7 @@ export interface DeviceForkOverrides { hideHeader: boolean } -export interface RemoteDevice extends ConnectRequest { +interface RemoteDeviceEntry extends ConnectRequest { serial: string /* * 'connecting' until adb reports the device usable. 'releasing' while it is @@ -24,8 +24,28 @@ export interface RemoteDevice extends ConnectRequest { * connect` leaves it "offline"), yet it must not be served or reconnected. */ state: 'connecting' | 'active' | 'releasing' + /* Whether the worker is ready: it is not before its first start and while the device recovers */ + ready: boolean + /* Email of the user who owns the device, as reported by its worker; empty if it reported none */ + owner?: string } +/* A device as the API shows it */ +export interface RemoteDevice extends ConnectRequest { + serial: string + /* 'connecting' until the device is usable by a user: adb connected and its worker ready */ + state: 'connecting' | 'active' | 'releasing' + acquired: boolean + owner: string | null +} + +const view = ({ready, owner, state, ...device}: RemoteDeviceEntry): RemoteDevice => ({ + ...device, + state: state === 'active' && !ready ? 'connecting' : state, + acquired: owner !== undefined, + owner: owner || null +}) + export type LossReason = 'connection_lost' | 'stuck' /* Why a device that failed to recover is disconnected */ @@ -62,10 +82,8 @@ const RELEASE_REASONS: Record = { */ export class RemoteDeviceManager { readonly webhooks: WebhookHub - private devices = new Map() + private devices = new Map() private releases = new Map>() - /* Email of the user who owns a device, as reported by its worker */ - private owners = new Map() /* Devices waiting for a ready worker, with the timer that gives up on them */ private recoveries = new Map() private readonly tracker: ADBObserver @@ -90,8 +108,13 @@ export class RemoteDeviceManager { return this.devices.get(serial)?.state === 'releasing' } - list() { - return Array.from(this.devices.values()) + list(): RemoteDevice[] { + return Array.from(this.devices.values(), view) + } + + get(serial: string): RemoteDevice | undefined { + const device = this.devices.get(serial) + return device && view(device) } forkOverrides(serial: string): DeviceForkOverrides | undefined { @@ -111,13 +134,13 @@ export class RemoteDeviceManager { } /* Registers the device and connects it in the background */ - connect(request: ConnectRequest): RemoteDevice { + connect(request: ConnectRequest): RemoteDeviceEntry { const serial = deviceSerial(request.host, request.port) if (this.devices.has(serial) || this.tracker.getDevice(serial)) { throw new ConflictError(`Device "${serial}" is already connected`) } - const device: RemoteDevice = {...request, serial, state: 'connecting'} + const device: RemoteDeviceEntry = {...request, serial, state: 'connecting', ready: false} this.devices.set(serial, device) this.tracker.setAutoReconnect(serial, false) if (device.webhook) { @@ -142,10 +165,13 @@ export class RemoteDeviceManager { } workerReady(serial: string) { - if (!this.isServed(serial)) { + const device = this.served(serial) + if (!device) { return } + device.ready = true + if (this.cancelRecovery(serial)) { log.info('Remote device "%s" recovered', serial) } @@ -153,15 +179,16 @@ export class RemoteDeviceManager { } workerState(serial: string, message: {state: 'busy' | 'idle', email?: string, reason?: string}) { - if (!this.isServed(serial)) { + const device = this.served(serial) + if (!device) { return } if (message.state === 'busy') { - if (message.email && this.owners.get(serial) === message.email) { + if (message.email && device.owner === message.email) { return } - this.owners.set(serial, message.email ?? '') + device.owner = message.email ?? '' this.webhooks.emit(serial, 'device.acquired', {email: message.email}) return } @@ -184,7 +211,11 @@ export class RemoteDeviceManager { * disconnected unless a worker gets ready again within `recoveryTimeoutMs`. */ private startRecovery(serial: string, cause: string) { - if (this.devices.get(serial)?.state !== 'active') { + const device = this.served(serial) + if (device) { + device.ready = false + } + if (device?.state !== 'active') { return } @@ -212,19 +243,20 @@ export class RemoteDeviceManager { /* Reports the release of an owned device once, whoever notices it first */ private released(serial: string, reason?: string) { - const email = this.owners.get(serial) + const device = this.devices.get(serial) + const email = device?.owner if (email === undefined) { return } - this.owners.delete(serial) + device!.owner = undefined this.webhooks.emit(serial, 'device.released', {email: email || undefined, reason}) } /* An API device the provider still serves: a device being released reports nothing more */ - private isServed(serial: string) { - const state = this.devices.get(serial)?.state - return state === 'connecting' || state === 'active' + private served(serial: string) { + const device = this.devices.get(serial) + return device?.state === 'connecting' || device?.state === 'active' ? device : undefined } /* @@ -244,10 +276,10 @@ export class RemoteDeviceManager { /* API devices must not outlive the provider: nobody would own them */ async shutdown() { - await Promise.all(this.list().map(device => this.release(device.serial, {reason: 'provider_shutdown'}))) + await Promise.all(Array.from(this.devices.keys(), serial => this.release(serial, {reason: 'provider_shutdown'}))) } - private async establish(device: RemoteDevice) { + private async establish(device: RemoteDeviceEntry) { // Released (and maybe registered again) while we were waiting const isCurrent = () => this.devices.get(device.serial) === device && device.state !== 'releasing' @@ -284,7 +316,7 @@ export class RemoteDeviceManager { this.webhooks.emit(device.serial, 'device.connected') } - private async failConnect(device: RemoteDevice, err: any, isCurrent: () => boolean) { + private async failConnect(device: RemoteDeviceEntry, err: any, isCurrent: () => boolean) { if (!isCurrent()) { return } @@ -336,14 +368,13 @@ export class RemoteDeviceManager { const release = this.stopAndDisconnect(device).finally(() => { this.devices.delete(serial) this.releases.delete(serial) - this.owners.delete(serial) this.tracker.setAutoReconnect(serial, true) }) this.releases.set(serial, release) return release } - private async stopAndDisconnect(device: RemoteDevice) { + private async stopAndDisconnect(device: RemoteDeviceEntry) { try { await this.removeWorker(device.serial) } diff --git a/lib/units/ratelimit/client-key.ts b/lib/units/ratelimit/client-key.ts new file mode 100644 index 0000000000..56aa88cba8 --- /dev/null +++ b/lib/units/ratelimit/client-key.ts @@ -0,0 +1,19 @@ +import type {Request} from 'express' + +const LOOPBACK = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']) + +/* + * The client a request is counted against. Behind the local reverse proxy + * (nginx, poorxy) every request comes from loopback, so the client is the + * address the proxy puts in X-Real-IP. A client connecting directly could + * forge that header: its own address is used instead. + */ +export const clientKey = (req: Request): string => { + const remote = req.socket.remoteAddress ?? '' + const realIp = req.headers['x-real-ip'] + + if (LOOPBACK.has(remote) && typeof realIp === 'string' && realIp) { + return realIp + } + return remote +} diff --git a/lib/units/ratelimit/index.js b/lib/units/ratelimit/index.js index c9067ac640..d404f09f72 100644 --- a/lib/units/ratelimit/index.js +++ b/lib/units/ratelimit/index.js @@ -1,11 +1,12 @@ import {rateLimit} from 'express-rate-limit' +import {clientKey} from './client-key.js' let rateLimitConfig = rateLimit({ windowMs: 60 * 1000, // 1 minute limit: 140 * 20, standardHeaders: 'draft-7', legacyHeaders: false, // Disable the `X-RateLimit-*` headers - keyGenerator: (req, res) => { - return req.headers['X-Real-IP'] ? req.headers['X-Real-IP'] : 'localhost' - } + keyGenerator: clientKey, + // The key does not come from X-Forwarded-For, whatever the proxy sends + validate: {xForwardedForHeader: false} }) export default rateLimitConfig diff --git a/lib/units/websocket/index.ts b/lib/units/websocket/index.ts index 68abc8f380..a71b12fad3 100644 --- a/lib/units/websocket/index.ts +++ b/lib/units/websocket/index.ts @@ -59,7 +59,6 @@ import { CapabilitiesMessage, TemporarilyUnavailableMessage, UpdateRemoteConnectUrl, - AdbKeysUpdatedMessage, SizeIosDevice, GroupMessage, UngroupMessage, @@ -581,14 +580,20 @@ export default (async (options: Options) => { }) socket.on('user.keys.accessToken.generate', async (data: any) => { - const {title} = data - const token = generateToken(user, options.secret) - await AllModel.saveUserAccessToken(user.email, { - title, - id: token.id, - jwt: token.jwt - }) - socket.emit('user.keys.accessToken.generated', {title, token: token.jwt}) + try { + const {title, lifetime = 0} = data + const token = generateToken(user, options.secret, lifetime) + await AllModel.saveUserAccessToken(user.email, { + title, + id: token.id, + jwt: token.jwt, + expiresAt: token.expiresAt + }) + socket.emit('user.keys.accessToken.generated', {title, token: token.jwt}) + } + catch (err: any) { + log.error('Failed to generate access token: %s', err?.message) + } }) socket.on('user.keys.accessToken.remove', async (data: any) => { @@ -601,16 +606,15 @@ export default (async (options: Options) => { socket.on('user.keys.adb.add', async (data: any) => { try { const key = await Adb.util.parsePublicKey(data.key) - const users = await UserModel.lookupUsersByAdbKey(key.fingerprint) - if (users.length) { - throw new AllModel.DuplicateSecondaryIndexError() - } - await UserModel.insertUserAdbKey(user.email, { + const added = await UserModel.insertUserAdbKey(user.email, { title: data.title, fingerprint: key.fingerprint }) + if (!added) { + socket.emit('user.keys.adb.error', {message: 'Key was already added'}) + return + } socket.emit('user.keys.adb.added', {title: data.title, fingerprint: key.fingerprint}) - transport.sendBroadcast(wireutil.pack(AdbKeysUpdatedMessage, {})) } catch (err: any) { socket.emit('user.keys.adb.error', {message: err.message}) @@ -618,20 +622,12 @@ export default (async (options: Options) => { }) socket.on('user.keys.adb.accept', async (data: any) => { - try { - const users = await UserModel.lookupUsersByAdbKey(data.fingerprint) - if (users.length) { - throw new AllModel.DuplicateSecondaryIndexError() - } - await UserModel.insertUserAdbKey(user.email, { - title: data.title, - fingerprint: data.fingerprint - }) + const added = await UserModel.insertUserAdbKey(user.email, { + title: data.title, + fingerprint: data.fingerprint + }) + if (added) { socket.emit('user.keys.adb.added', {title: data.title, fingerprint: data.fingerprint}) - transport.sendBroadcast(wireutil.pack(AdbKeysUpdatedMessage, {})) - } - catch (err: any) { - if (!(err instanceof AllModel.DuplicateSecondaryIndexError)) throw err } }) diff --git a/lib/units/websocket/middleware/auth.ts b/lib/units/websocket/middleware/auth.ts index 84df6f1b81..89af224404 100644 --- a/lib/units/websocket/middleware/auth.ts +++ b/lib/units/websocket/middleware/auth.ts @@ -1,8 +1,12 @@ import dbapi from '../../../db/api.js' import * as jwtutil from '../../../util/jwtutil.js' import logger from '../../../util/logger.js' +import {createAccessTokenCheck} from '../../../util/access-token-check.js' import type {Socket} from 'socket.io' +/* Shared by every namespace of the unit */ +const accessTokenExists = createAccessTokenCheck(id => dbapi.loadAccessTokenById(id).then(Boolean)) + interface AuthOptions { secret: string } @@ -21,6 +25,10 @@ export default (options: AuthOptions) => { } try { + if (token.jti && !await accessTokenExists(token.jti)) { + return next(new Error('Invalid user')) + } + const user = await dbapi.loadUser(token.email) if (user) { req.user = user diff --git a/lib/util/ProcessManager.ts b/lib/util/ProcessManager.ts index e305902d16..24d32e2763 100644 --- a/lib/util/ProcessManager.ts +++ b/lib/util/ProcessManager.ts @@ -300,8 +300,19 @@ export class ProcessManager { const previousChild = process.childProcess if (previousChild && previousChild.exitCode === null && !previousChild.killed) { this.log.warn('Process "%s" still has a live child, killing it first', id) - previousChild.removeAllListeners('message') - await procutil.gracefullyKill(previousChild, this.killTimeout) + // Its own terminate handler detaches its listeners first: an exit we + // caused must not count as a crash and schedule yet another restart + const terminatePrevious: (() => Promise) | undefined = (process as any).terminateHandler + ;(process as any).terminateHandler = undefined + if (terminatePrevious) { + await terminatePrevious() + } + else { + previousChild.removeAllListeners('exit') + previousChild.removeAllListeners('error') + previousChild.removeAllListeners('message') + await procutil.gracefullyKill(previousChild, this.killTimeout) + } } // Spawn the child process diff --git a/lib/util/access-token-check.ts b/lib/util/access-token-check.ts new file mode 100644 index 0000000000..76b235b590 --- /dev/null +++ b/lib/util/access-token-check.ts @@ -0,0 +1,39 @@ +/* + * Whether access tokens still exist: deleting a token revokes it. A token is + * looked up at most once per `ttlMs`, so a revoked token keeps working for up + * to that long. Concurrent checks of a token share one lookup; a failed lookup + * is not cached. Beyond `maxSize` tokens, the least recently looked up is dropped. + */ +export const createAccessTokenCheck = ( + exists: (id: string) => Promise, + {ttlMs = 120_000, maxSize = 10_000} = {} +) => { + const checked = new Map() + const pending = new Map>() + + return (id: string): Promise => { + const cached = checked.get(id) + if (cached && Date.now() - cached.at < ttlMs) { + return Promise.resolve(cached.valid) + } + + const inFlight = pending.get(id) + if (inFlight) { + return inFlight + } + + const lookup = exists(id) + .then(valid => { + // Map order is lookup order: re-insert, then drop the oldest + checked.delete(id) + checked.set(id, {valid, at: Date.now()}) + if (checked.size > maxSize) { + checked.delete(checked.keys().next().value!) + } + return valid + }) + .finally(() => pending.delete(id)) + pending.set(id, lookup) + return lookup + } +} diff --git a/lib/util/jwtutil.js b/lib/util/jwtutil.js index 0cd653f7cf..d7f9e10785 100644 --- a/lib/util/jwtutil.js +++ b/lib/util/jwtutil.js @@ -5,9 +5,11 @@ import {ONE_MONTH} from './apiutil.js' export const encode = function(options) { assert.ok(options.payload, 'payload required') assert.ok(options.secret, 'secret required') + // Expiration time in ms; null never expires + const exp = options.exp === undefined ? Date.now() + ONE_MONTH : options.exp let header = { alg: 'HS256', - exp: Date.now() + ONE_MONTH + ...(exp !== null && {exp}) } if (options.header) { header = _.merge(header, options.header) diff --git a/test/unit/db/device-group.integration.test.ts b/test/unit/db/device-group.integration.test.ts new file mode 100644 index 0000000000..2d61a0d0f0 --- /dev/null +++ b/test/unit/db/device-group.integration.test.ts @@ -0,0 +1,98 @@ +/* + * Device group updates that run as MongoDB aggregation pipelines: their + * semantics only show on a real server. Opt-in, against a throwaway database: + * MONGODB_INTEGRATION_TESTS=1 npx vitest run test/unit/db/device-group.integration.test.ts + * MONGODB_PORT_27017_TCP points to the server (default mongodb://127.0.0.1:27017). + */ +import {afterAll, beforeAll, beforeEach, describe, expect, it} from 'vitest' + +const enabled = process.env.MONGODB_INTEGRATION_TESTS === '1' + +describe.skipIf(!enabled)('device group updates (real MongoDB)', () => { + let db: any + let dbapi: any + const devices = () => db.connection.collection('devices') + const group = (serial: string) => devices().findOne({serial}).then((device: any) => device.group) + + beforeAll(async() => { + process.env.MONGODB_DB_NAME = `devicehub_test_${process.pid}_${Date.now()}` + db = (await import('../../../lib/db/index.js')).default + dbapi = (await import('../../../lib/db/api.js')).default + await db.connect() + }) + + afterAll(async() => { + await db?.connection.dropDatabase() + await db?.connection.client.close() + }) + + beforeEach(() => devices().deleteMany({})) + + const insert = (serial: string, groupFields: Record) => + devices().insertOne({serial, group: {id: 'g1', origin: 'g1', name: 'one', originName: 'one', ...groupFields}}) + + describe('lockDeviceByCurrent', () => { + it('locks a device of one of the user groups', async() => { + await insert('s1', {lock: false}) + await dbapi.lockDeviceByCurrent(['g1'], 's1') + expect((await group('s1')).lock).toBe(true) + }) + + // The bug let any user take any device + it('does not lock a device of another group', async() => { + await insert('s1', {id: 'other', lock: false}) + const stats = await dbapi.lockDeviceByCurrent(['g1'], 's1') + expect(stats.modifiedCount).toBe(0) + expect((await group('s1')).lock).toBe(false) + }) + + it('does not lock a device locked by someone else', async() => { + await insert('s1', {lock: true}) + const stats = await dbapi.lockDeviceByCurrent(['g1'], 's1') + expect(stats.modifiedCount).toBe(0) + expect((await group('s1')).lock).toBe(true) + }) + + // The bug removed the field of every device it was asked to lock + it('locks a device without a lock field', async() => { + await insert('s1', {}) + await dbapi.lockDeviceByCurrent(['g1'], 's1') + expect((await group('s1')).lock).toBe(true) + }) + }) + + describe('lockDeviceByOrigin', () => { + it('locks a device whose origin group is one of the user groups', async() => { + await insert('s1', {id: 'booking', lock: false}) + await dbapi.lockDeviceByOrigin(['g1'], 's1') + expect((await group('s1')).lock).toBe(true) + }) + + it('does not lock a device of another origin group', async() => { + await insert('s1', {origin: 'other', lock: false}) + const stats = await dbapi.lockDeviceByOrigin(['g1'], 's1') + expect(stats.modifiedCount).toBe(0) + expect((await group('s1')).lock).toBe(false) + }) + }) + + describe('updateDeviceGroupName', () => { + it('renames the current group of a device sitting in its origin group', async() => { + await insert('s1', {}) + await dbapi.updateDeviceGroupName('s1', {id: 'g1', name: 'renamed', class: 'bookable'}) + expect(await group('s1')).toMatchObject({name: 'renamed', originName: 'renamed'}) + }) + + it('keeps the name of a booking when its origin group is renamed', async() => { + await insert('s1', {id: 'booking', name: 'my booking'}) + await dbapi.updateDeviceGroupName('s1', {id: 'g1', name: 'renamed', class: 'bookable'}) + expect(await group('s1')).toMatchObject({name: 'my booking', originName: 'renamed'}) + }) + + it('keeps the names when an inactive booking is renamed', async() => { + await insert('s1', {id: 'booking', name: 'my booking'}) + await dbapi.updateDeviceGroupName('s1', {id: 'booking', name: 'renamed', class: 'once', isActive: false}) + expect(await group('s1')).toMatchObject({name: 'my booking', originName: 'one'}) + }) + }) +}) diff --git a/test/unit/units/api/access-tokens.test.ts b/test/unit/units/api/access-tokens.test.ts new file mode 100644 index 0000000000..514cd24182 --- /dev/null +++ b/test/unit/units/api/access-tokens.test.ts @@ -0,0 +1,73 @@ +import {afterEach, describe, expect, it, vi} from 'vitest' + +vi.mock('../../../../lib/db/api.js', () => ({ + default: { + loadUser: vi.fn(async(email: string) => ({email, privilege: 'user'})), + loadAccessTokenById: vi.fn() + } +})) + +import dbapi from '../../../../lib/db/api.js' +import * as jwtutil from '../../../../lib/util/jwtutil.js' +import {ONE_DAY} from '../../../../lib/util/apiutil.js' +import generateToken from '../../../../lib/units/api/helpers/generateToken.js' +import {accessTokenAuth} from '../../../../lib/units/api/helpers/securityHandlers.js' + +const secret = 'secret' +const user = {email: 'u@x', name: 'u'} + +describe('access token lifetime', () => { + afterEach(() => { vi.useRealTimers() }) + + it('never expires by default', () => { + vi.useFakeTimers() + const token = generateToken(user, secret) + expect(token.expiresAt).toBeNull() + + vi.advanceTimersByTime(10 * 365 * ONE_DAY) + expect(jwtutil.decode(token.jwt, secret)).toMatchObject({email: 'u@x', jti: token.id}) + }) + + it('expires after the chosen number of 30-day months', () => { + vi.useFakeTimers() + const token = generateToken(user, secret, 2) + expect(token.expiresAt).toBe(Date.now() + 60 * ONE_DAY) + + vi.advanceTimersByTime(60 * ONE_DAY - 1) + expect(jwtutil.decode(token.jwt, secret)).not.toBeNull() + vi.advanceTimersByTime(1) + expect(jwtutil.decode(token.jwt, secret)).toBeNull() + }) + + it('refuses other lifetimes', () => { + expect(() => generateToken(user, secret, 4)).toThrow(/lifetime/) + }) +}) + +describe('access token authentication', () => { + const authenticate = (jwt: string) => accessTokenAuth({headers: {authorization: `Bearer ${jwt}`}, options: {secret}}) + + it('rejects a deleted access token', async() => { + const token = generateToken(user, secret) + vi.mocked(dbapi.loadAccessTokenById).mockResolvedValueOnce(null) + + await expect(authenticate(token.jwt)).rejects.toMatchObject({status: 401, message: 'Revoked token'}) + expect(dbapi.loadAccessTokenById).toHaveBeenCalledWith(token.id) + }) + + it('accepts an existing access token', async() => { + const token = generateToken(user, secret) + vi.mocked(dbapi.loadAccessTokenById).mockResolvedValueOnce({id: token.id}) + + await expect(authenticate(token.jwt)).resolves.toBe(true) + }) + + // Session tokens and access tokens issued before `jti` cost no extra lookup + it('does not look up a token without jti', async() => { + vi.mocked(dbapi.loadAccessTokenById).mockClear() + const jwt = jwtutil.encode({payload: user, secret}) + + await expect(authenticate(jwt)).resolves.toBe(true) + expect(dbapi.loadAccessTokenById).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/units/api/user-devices.test.ts b/test/unit/units/api/user-devices.test.ts new file mode 100644 index 0000000000..bc52dc3bb2 --- /dev/null +++ b/test/unit/units/api/user-devices.test.ts @@ -0,0 +1,101 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest' + +vi.mock('../../../../lib/db/api.js', () => ({ + default: { + lockDeviceByCurrent: vi.fn(), + unlockDevice: vi.fn(), + insertUserAdbKey: vi.fn(), + loadDevice: vi.fn() + } +})) + +import dbapi from '../../../../lib/db/api.js' +import {addAdbPublicKey, addUserDevice, deleteUserDeviceBySerial} from '../../../../lib/units/api/controllers/user.js' +import {GroupMessage, UngroupMessage} from '../../../../lib/wire/wire.js' + +const response = () => { + const res = {status: vi.fn(), json: vi.fn(), setHeader: vi.fn()} + res.status.mockReturnValue(res) + return res +} + +describe('POST /user/devices', () => { + beforeEach(() => { + vi.mocked(dbapi.lockDeviceByCurrent).mockResolvedValue({ + modifiedCount: 1, + changes: [{new_val: {serial: '099643733J004550', present: true, ready: true, using: false, owner: null, provider: {name: 'p1'}}}] + }) + }) + + // A user who never added an ADB key has no `adbKeys` at all + it('takes a device for a user without ADB keys', async() => { + const runTransaction = vi.fn().mockResolvedValue({success: true}) + const req = { + body: {serial: '099643733J004550', timeout: 900000}, + user: {email: 'u@x', name: 'u', group: 'g', groups: {subscribed: []}}, + options: {txmanager: {runTransaction}} + } + const res = response() + + await addUserDevice(req, res) + + expect(res.json).toHaveBeenCalledWith({success: true, description: 'Device successfully added'}) + expect(runTransaction).toHaveBeenCalledWith('p1', '099643733J004550', GroupMessage, + expect.objectContaining({timeout: 900000, keys: []})) + }) +}) + +describe('POST /user/adbPublicKeys', () => { + it('answers 400 to a malformed key and stores nothing', async() => { + const res = response() + await addAdbPublicKey({body: {publickey: 'not a key'}, user: {email: 'u@x'}}, res) + + expect(res.status).toHaveBeenCalledWith(400) + expect(dbapi.insertUserAdbKey).not.toHaveBeenCalled() + }) +}) + +describe('DELETE /user/devices/:serial', () => { + const user = {email: 'u@x', name: 'u', group: 'g', groups: {subscribed: []}} + + beforeEach(() => { + vi.mocked(dbapi.loadDevice).mockResolvedValue({ + serial: 's1', present: true, ready: true, using: true, provider: {name: 'p1'}, + owner: {email: 'u@x', name: 'u', group: 'g'} + }) + }) + + it('answers once the device is released', async() => { + const runTransaction = vi.fn().mockResolvedValue({success: true}) + const res = response() + + await deleteUserDeviceBySerial({params: {serial: 's1'}, user, options: {txmanager: {runTransaction}}}, res) + + expect(runTransaction).toHaveBeenCalledWith('p1', 's1', UngroupMessage, expect.anything()) + expect(res.json).toHaveBeenCalledWith({success: true, description: 'Device successfully removed'}) + }) + + // Releasing the devices of a deleted group: the response belongs to the group request + it('leaves the response alone when called internally', async() => { + const res = response() + + const released = await deleteUserDeviceBySerial({ + isInternal: true, serial: 's1', user, options: {txmanager: {runTransaction: vi.fn().mockResolvedValue({success: true})}} + }, res) + + expect(released).toBe(true) + expect(res.json).not.toHaveBeenCalled() + expect(res.status).not.toHaveBeenCalled() + }) + + it('answers 504 with the reason when the device does not respond', async() => { + const res = response() + + await deleteUserDeviceBySerial({ + params: {serial: 's1'}, user, + options: {txmanager: {runTransaction: vi.fn().mockRejectedValue(new Error('Timeout when running transaction'))}} + }, res) + + expect(res.status).toHaveBeenCalledWith(504) + }) +}) diff --git a/test/unit/units/auth/oauth2.test.ts b/test/unit/units/auth/oauth2.test.ts new file mode 100644 index 0000000000..c03922bc93 --- /dev/null +++ b/test/unit/units/auth/oauth2.test.ts @@ -0,0 +1,46 @@ +import {afterEach, beforeEach, describe, expect, it} from 'vitest' +import type {AddressInfo} from 'node:net' +import type {Server} from 'node:http' +import {createApp} from '../../../../lib/units/auth/oauth2/index.js' + +describe('OAuth 2.0 auth unit', () => { + let server: Server + let base: string + + beforeEach(async() => { + const app = createApp({ + secret: 'secret', + appUrl: 'http://app.test/', + docsUrl: 'http://docs.test/', + oauth: { + authorizationURL: 'http://idp.test/authorize', + tokenURL: 'http://idp.test/token', + userinfoURL: 'http://idp.test/userinfo', + clientID: 'client', + clientSecret: 'client-secret', + callbackURL: 'http://app.test/auth/oauth/callback', + scope: ['openid'] + } + }) + server = app.listen(0) + await new Promise(resolve => server.once('listening', resolve)) + base = `http://127.0.0.1:${(server.address() as AddressInfo).port}` + }) + + afterEach(() => new Promise(resolve => server.close(() => resolve()))) + + // The UI asks for it on every page: it must not start a login + it('answers /auth/docs without going to the identity provider', async() => { + const res = await fetch(`${base}/auth/docs`, {redirect: 'manual'}) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({success: true, docsUrl: 'http://docs.test/'}) + }) + + it('still sends a login to the identity provider', async() => { + const res = await fetch(`${base}/auth/oauth/`, {redirect: 'manual'}) + + expect(res.status).toBe(302) + expect(res.headers.get('location')).toMatch(/^http:\/\/idp\.test\/authorize\?/) + }) +}) diff --git a/test/unit/units/base-device/screen-ws-url.test.ts b/test/unit/units/base-device/screen-ws-url.test.ts new file mode 100644 index 0000000000..589132d94a --- /dev/null +++ b/test/unit/units/base-device/screen-ws-url.test.ts @@ -0,0 +1,17 @@ +import {describe, expect, it} from 'vitest' +import {screenWsUrl} from '../../../../lib/units/base-device/support/screen-ws-url.ts' + +describe('screen WebSocket URL', () => { + const options = {provider: 'vk-android-1', serial: '10.0.0.5:5555', publicIp: 'devicehub.test', screenPort: 7400} + + // A reverse proxy routes the screen of a device by its provider + it('fills in the provider and the serial', () => { + expect(screenWsUrl({...options, screenWsUrlPattern: 'wss://devicehub.test/d/<%= provider %>/<%= serial %>'})) + .toBe('wss://devicehub.test/d/vk-android-1/10.0.0.5:5555') + }) + + it('fills in the public address by default', () => { + expect(screenWsUrl({...options, screenWsUrlPattern: 'ws://${publicIp}:${publicPort}'})) + .toBe('ws://devicehub.test:7400') + }) +}) diff --git a/test/unit/units/device/connect.test.ts b/test/unit/units/device/connect.test.ts index 21e719c63b..f8771607c4 100644 --- a/test/unit/units/device/connect.test.ts +++ b/test/unit/units/device/connect.test.ts @@ -9,7 +9,6 @@ import Packet from '../../../../node_modules/@u4/adbkit/dist/adb/tcpusb/packet.j import Auth from '../../../../node_modules/@u4/adbkit/dist/adb/auth.js' vi.mock('../../../../lib/units/device/support/adb.js', () => ({default: {consume() {}}})) -vi.mock('../../../../lib/units/base-device/support/router.js', () => ({default: {consume() {}}})) vi.mock('../../../../lib/units/base-device/support/transport.js', () => ({default: {consume() {}}})) vi.mock('../../../../lib/units/device/plugins/group.js', () => ({default: {consume() {}}})) vi.mock('../../../../lib/units/device/plugins/solo.js', () => ({default: {consume() {}}})) @@ -57,7 +56,7 @@ async function setup(silent: boolean, keys?: string[]) { getProperties: async () => ({'ro.product.name': 'test', 'ro.product.model': 'test', 'ro.product.device': 'test'})}), createTcpUsbBridge: (_serial: string, options: any) => { auth = options.auth; return server }} const connector = {init: (options: any) => { handlers = options.handlers }, stop: vi.fn(async () => {})} - await connect.invoke({silent, serial: 'serial', connectPort: 7401}, client, {removeAllListeners() {}}, {}, group, + await connect.invoke({silent, serial: 'serial', connectPort: 7401}, client, {}, group, {}, () => 'localhost:7401', connector, {model: 'test'}, {name: {id: 'test'}}, {bin: 'minirev', stop: async () => {}}) group.emit('join', {adbKeys: keys}) await handlers.start() @@ -105,9 +104,8 @@ describe('remote ADB authentication', () => { await vi.waitFor(() => expect(accepted.connection.authorized).toBe(true)) }) - it('applies an empty key list on owner reconnect and denies authentication after release', async () => { - const h = await setup(true, ['different']) - h.group.emit('keys', []) + it('denies authentication after release', async () => { + const h = await setup(true, [fingerprint]) const accepted = h.socket() h.authenticate(accepted.connection) await vi.waitFor(() => expect(accepted.connection.authorized).toBe(true)) diff --git a/test/unit/units/device/support/adb.test.ts b/test/unit/units/device/support/adb.test.ts new file mode 100644 index 0000000000..cb92c0d828 --- /dev/null +++ b/test/unit/units/device/support/adb.test.ts @@ -0,0 +1,29 @@ +import net from 'node:net' +import {describe, expect, it, vi} from 'vitest' + +const execFile = vi.hoisted(() => vi.fn()) +vi.mock('node:child_process', async(importOriginal) => ({ + ...await importOriginal(), + execFile +})) + +const {createWorkerAdbClient} = await import('../../../../../lib/units/device/support/adb.js') + +/* A port nothing listens on, as when the ADB server went down */ +const closedPort = async() => { + const server = net.createServer() + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)) + const {port} = server.address() as net.AddressInfo + await new Promise(resolve => server.close(resolve)) + return port +} + +describe('device worker ADB client', () => { + // The provider owns the ADB server: workers starting their own race it and each other + it('never starts the ADB server itself', async() => { + const client = createWorkerAdbClient({adbHost: '127.0.0.1', adbPort: await closedPort()}) + + await expect(client.listDevices()).rejects.toThrow(/ADB server 127\.0\.0\.1:\d+ is not running/) + expect(execFile).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/units/provider/adb-observer.test.ts b/test/unit/units/provider/adb-observer.test.ts index f8533ab745..44588a8a69 100644 --- a/test/unit/units/provider/adb-observer.test.ts +++ b/test/unit/units/provider/adb-observer.test.ts @@ -2,14 +2,26 @@ import {afterEach, beforeEach, describe, expect, it} from 'vitest' import net from 'node:net' import ADBObserver, {deviceSerial} from '../../../../lib/units/provider/ADBObserver.ts' -/** Minimal ADB server: answers one host service per connection, like the real one. */ -function fakeAdbServer(reply: (command: string) => string) { +type Reply = string | {body: string, delayMs: number} + +/** + * Minimal ADB server: answers one host service per connection, like the real + * one. After `host:transport:` the connection is a tunnel to the + * device: it answers one `shell:` command, anything else sent there is + * recorded as misrouted. + */ +function fakeAdbServer(reply: (command: string) => Reply) { const commands: string[] = [] + const misrouted: string[] = [] const server = net.createServer(socket => { let buffer = Buffer.alloc(0) + let transport = false + // Whatever comes after a host request on its connection is never read + let answering = false // The observer drops its socket on destroy() socket.on('error', () => {}) socket.on('data', data => { + if (answering) return buffer = Buffer.concat([buffer, data]) while (buffer.length >= 4) { const length = parseInt(buffer.subarray(0, 4).toString('ascii'), 16) @@ -18,15 +30,33 @@ function fakeAdbServer(reply: (command: string) => string) { buffer = buffer.subarray(4 + length) commands.push(command) - const body = Buffer.from(reply(command), 'utf-8') - socket.write(Buffer.concat([ - Buffer.from('OKAY'), - Buffer.from(body.length.toString(16).padStart(4, '0')), - body - ])) - + if (transport) { + if (command.startsWith('shell:')) { + socket.end(Buffer.from('OKAY31\n')) + } + else { + misrouted.push(command) + socket.end() + } + return + } + if (command.startsWith('host:transport:')) { + transport = true + // Slow enough for other requests to queue up meanwhile + setTimeout(() => socket.write('OKAY'), 50) + continue + } + + answering = true + const answer = reply(command) + const {body, delayMs} = typeof answer === 'string' ? {body: answer, delayMs: 0} : answer + const bytes = Buffer.from(body, 'utf-8') // Like the real server: every host request closes the connection - socket.end() + setTimeout(() => socket.end(Buffer.concat([ + Buffer.from('OKAY'), + Buffer.from(bytes.length.toString(16).padStart(4, '0')), + bytes + ])), delayMs) return } }) @@ -34,6 +64,7 @@ function fakeAdbServer(reply: (command: string) => string) { return { commands, + misrouted, listen: () => new Promise(resolve => server.listen(0, '127.0.0.1', () => resolve((server.address() as net.AddressInfo).port)) ), @@ -55,18 +86,26 @@ describe('ADBObserver connect/disconnect', () => { const connect = command.match(/^host:connect:(.+)$/) if (connect) { if (connect[1].endsWith(':1')) return `failed to connect to '${connect[1]}': Connection refused` + // An unreachable host: adb gives up long after the observer does + if (connect[1].startsWith('10.0.0.99:')) { + return {body: `failed to connect to '${connect[1]}': Operation timed out`, delayMs: 700} + } if (connected.has(connect[1])) return `already connected to ${connect[1]}` connected.add(connect[1]) return `connected to ${connect[1]}` } const disconnect = command.match(/^host:disconnect:(.+)$/) if (disconnect) { + if (disconnect[1].startsWith('10.0.0.99:')) { + // Late, yet while the request after it still waits for its reply + return {body: `disconnected ${disconnect[1]}`, delayMs: 450} + } connected.delete(disconnect[1]) return `disconnected ${disconnect[1]}` } return '' }) - observer = new ADBObserver({host: '127.0.0.1', port: await server.listen()}) + observer = new ADBObserver({host: '127.0.0.1', port: await server.listen(), requestTimeoutMs: 300}) // The provider always listens for errors; an unhandled 'error' event would throw observer.on('error', () => {}) // As in the provider: polling runs concurrently with API commands @@ -123,6 +162,70 @@ describe('ADBObserver connect/disconnect', () => { expect(connected.has('10.0.0.5:5555')).toBe(false) }) + /* A reply that comes after its request timed out must not be read as the reply to another one */ + describe('late replies', () => { + let lost: string[] + let seen: string[] + + beforeEach(async() => { + connected.add('10.0.0.5:5555') + await observer.refresh() + lost = [] + seen = [] + observer.on('disconnect', device => lost.push(device.serial)) + observer.on('connect', device => seen.push(device.serial)) + }) + + it('a slow connect neither holds up polling nor garbles the device list', async() => { + let settled = false + const connecting = observer.connect('10.0.0.99', 5555, 500) + connecting.catch(() => {}).finally(() => { settled = true }) + + // Polling goes on while adb is still trying to connect + await observer.refresh() + expect(settled).toBe(false) + + await expect(connecting).rejects.toThrow(/timeout/i) + await new Promise(resolve => setTimeout(resolve, 400)) // past adb's late reply + await observer.refresh() + + expect(lost).toEqual([]) + expect(seen).toEqual([]) + expect(observer.getDevice('10.0.0.5:5555')?.type).toBe('device') + }) + + it('a timed out request leaves no reply behind for the next one', async() => { + await expect(observer.disconnect('10.0.0.99', 5555)).rejects.toThrow(/timeout/i) + // The next request goes out before adb's late reply comes + await observer.refresh() + await new Promise(resolve => setTimeout(resolve, 600)) // past adb's late reply + await observer.refresh() + + expect(lost).toEqual([]) + expect(seen).toEqual([]) + }) + }) + + it('forgets a device as soon as it is disconnected', async() => { + connected.add('10.0.0.5:5555') + await observer.refresh() + + await observer.disconnect('10.0.0.5', 5555) + expect(observer.getDevice('10.0.0.5:5555')).toBeUndefined() + }) + + it('sends a device command through its transport before any other request', async() => { + connected.add('10.0.0.5:5555') + const queue = observer as any + const [reply] = await Promise.all([ + queue.sendADBCommand('shell:getprop ro.build.version.sdk', '10.0.0.5:5555'), + queue.sendADBCommand('host:devices') + ]) + + expect(reply).toBe('31') + expect(server.misrouted).toEqual([]) + }) + it('never reconnects devices with auto reconnect disabled', async() => { const appeared = new Promise(resolve => observer.once('connect', resolve)) await observer.connect('10.0.0.5', 5555) diff --git a/test/unit/units/provider/api.test.ts b/test/unit/units/provider/api.test.ts index f69e91cb2a..1cd82617c8 100644 --- a/test/unit/units/provider/api.test.ts +++ b/test/unit/units/provider/api.test.ts @@ -12,6 +12,7 @@ describe('provider API', () => { beforeEach(async() => { devices = { list: vi.fn(() => []), + get: vi.fn((serial: string) => serial === '10.0.0.5:5555' ? {serial, state: 'active', acquired: true, owner: 'u@x'} : undefined), webhooks: {subscriptions: () => ({})}, connect: vi.fn((request: any) => ({serial: `${request.host}:${request.port}`})), disconnect: vi.fn((host: string, port: number) => host === 'known' ? `${host}:${port}` : null) @@ -56,6 +57,15 @@ describe('provider API', () => { expect((await post(JSON.stringify({host: '10.0.0.5', port: 5555}))).status).toBe(409) }) + it('shows a device by its serial', async() => { + const res = await fetch(`${base}/10.0.0.5:5555`) + expect(res.status).toBe(200) + expect(await res.json()).toEqual({device: {serial: '10.0.0.5:5555', state: 'active', acquired: true, owner: 'u@x'}}) + + expect((await fetch(`${base}/${encodeURIComponent('[::1]:5555')}`)).status).toBe(404) + expect(devices.get).toHaveBeenLastCalledWith('[::1]:5555') + }) + it('disconnects', async() => { const res = await fetch(`${base}/known/5555`, {method: 'DELETE'}) expect(res.status).toBe(202) diff --git a/test/unit/units/provider/manager.test.ts b/test/unit/units/provider/manager.test.ts index ac88932b3b..87ee7211f0 100644 --- a/test/unit/units/provider/manager.test.ts +++ b/test/unit/units/provider/manager.test.ts @@ -206,6 +206,33 @@ describe('RemoteDeviceManager', () => { expect(events().map(e => e.event)).toEqual(['device.acquired', 'device.released']) }) + it('shows a device as connecting until its worker is ready, then who owns it', async() => { + const serial = await active() + expect(manager.get(serial)).toMatchObject({serial, state: 'connecting', acquired: false, owner: null}) + + manager.workerReady(serial) + expect(manager.get(serial)).toMatchObject({state: 'active', acquired: false, owner: null}) + + manager.workerState(serial, {state: 'busy', email: 'u@x'}) + expect(manager.list()).toEqual([expect.objectContaining({serial, state: 'active', acquired: true, owner: 'u@x'})]) + + manager.workerState(serial, {state: 'idle', email: 'u@x', reason: 'ungroup_request'}) + expect(manager.get(serial)).toMatchObject({acquired: false, owner: null}) + expect(manager.get('unknown')).toBeUndefined() + }) + + it('shows a recovering device as connecting and released', async() => { + const serial = await active() + manager.workerReady(serial) + manager.workerState(serial, {state: 'busy', email: 'u@x'}) + + manager.workerFailed(serial) + expect(manager.get(serial)).toMatchObject({state: 'connecting', acquired: false, owner: null}) + + manager.workerReady(serial) + expect(manager.get(serial)).toMatchObject({state: 'active'}) + }) + it('ignores the workers of devices it does not serve', () => { manager.workerReady('s') manager.workerState('s', {state: 'busy', email: 'u@x'}) diff --git a/test/unit/units/ratelimit/client-key.test.ts b/test/unit/units/ratelimit/client-key.test.ts new file mode 100644 index 0000000000..2c0e805639 --- /dev/null +++ b/test/unit/units/ratelimit/client-key.test.ts @@ -0,0 +1,22 @@ +import {describe, expect, it} from 'vitest' +import {clientKey} from '../../../../lib/units/ratelimit/client-key.ts' + +const request = (remoteAddress: string, realIp?: string) => + ({socket: {remoteAddress}, headers: realIp ? {'x-real-ip': realIp} : {}}) as any + +describe('rate limit client key', () => { + // Behind nginx every request comes from loopback: without the header all clients would share one limit + it('tells apart the clients of the local reverse proxy', () => { + expect(clientKey(request('127.0.0.1', '10.0.0.1'))).toBe('10.0.0.1') + expect(clientKey(request('::ffff:127.0.0.1', '10.0.0.2'))).toBe('10.0.0.2') + expect(clientKey(request('::1', '10.0.0.3'))).toBe('10.0.0.3') + }) + + it('ignores the header of a client that connects directly', () => { + expect(clientKey(request('10.0.0.9', '1.2.3.4'))).toBe('10.0.0.9') + }) + + it('falls back to the connection address without a reverse proxy header', () => { + expect(clientKey(request('127.0.0.1'))).toBe('127.0.0.1') + }) +}) diff --git a/test/unit/util/access-token-check.test.ts b/test/unit/util/access-token-check.test.ts new file mode 100644 index 0000000000..44a704b58b --- /dev/null +++ b/test/unit/util/access-token-check.test.ts @@ -0,0 +1,52 @@ +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest' +import {createAccessTokenCheck} from '../../../lib/util/access-token-check.ts' + +describe('access token check', () => { + beforeEach(() => { vi.useFakeTimers() }) + afterEach(() => { vi.useRealTimers() }) + + it('looks a token up once per TTL and sees its revocation after it', async() => { + const tokens = new Set(['t1']) + const exists = vi.fn(async(id: string) => tokens.has(id)) + const check = createAccessTokenCheck(exists, {ttlMs: 120_000}) + + expect(await check('t1')).toBe(true) + tokens.delete('t1') + vi.advanceTimersByTime(119_000) + expect(await check('t1')).toBe(true) + expect(exists).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(1_000) + expect(await check('t1')).toBe(false) + expect(exists).toHaveBeenCalledTimes(2) + }) + + it('shares one lookup between concurrent checks', async() => { + const exists = vi.fn(async() => true) + const check = createAccessTokenCheck(exists) + + expect(await Promise.all([check('t1'), check('t1'), check('t1')])).toEqual([true, true, true]) + expect(exists).toHaveBeenCalledTimes(1) + }) + + it('does not cache a failed lookup', async() => { + const exists = vi.fn().mockRejectedValueOnce(new Error('db down')).mockResolvedValue(true) + const check = createAccessTokenCheck(exists) + + await expect(check('t1')).rejects.toThrow('db down') + expect(await check('t1')).toBe(true) + }) + + it('forgets the least recently looked up token beyond its size', async() => { + const exists = vi.fn(async() => true) + const check = createAccessTokenCheck(exists, {maxSize: 2}) + + await check('t1') + await check('t2') + await check('t3') + await check('t2') + expect(exists).toHaveBeenCalledTimes(3) + await check('t1') + expect(exists).toHaveBeenCalledTimes(4) + }) +}) diff --git a/test/unit/util/process-manager.test.ts b/test/unit/util/process-manager.test.ts index 6ecc3485b7..518c7d8a36 100644 --- a/test/unit/util/process-manager.test.ts +++ b/test/unit/util/process-manager.test.ts @@ -217,6 +217,19 @@ describe('ProcessManager shutdown of workers', () => { expect(isAlive(children[1])).toBe(true) }) + it('is not when it is replaced by a forced start', async() => { + setup() + const first = await startWorker() + + expect(await manager.start('serial', true)).toBe(true) + expect(isAlive(first)).toBe(false) + + await new Promise(resolve => setTimeout(resolve, 2500)) // past the restart delay + expect(errors).toEqual([]) + expect(children).toHaveLength(2) + expect(isAlive(children[1])).toBe(true) + }) + it('is not when it is stopped on purpose', async() => { setup() await startWorker() diff --git a/ui/public/locales/en/translation.json b/ui/public/locales/en/translation.json index 73de2035fa..6bda2e2aee 100644 --- a/ui/public/locales/en/translation.json +++ b/ui/public/locales/en/translation.json @@ -469,5 +469,13 @@ "Moderator": "Moderator", "Make Moderator": "Make Moderator", "Remove Moderator": "Remove Moderator", - "Group privilege": "Group privilege" + "Group privilege": "Group privilege", + "Optional. If you add keys, only their owners can connect via adb connect. Without keys, anyone with the command can connect.": "Optional. If you add keys, only their owners can connect via adb connect. Without keys, anyone with the command can connect.", + "Lifetime": "Lifetime", + "No expiration": "No expiration", + "1 month": "1 month", + "2 months": "2 months", + "3 months": "3 months", + "Expires on {{date}}": "Expires on {{date}}", + "Never expires": "Never expires" } \ No newline at end of file diff --git a/ui/public/locales/ru-RU/translation.json b/ui/public/locales/ru-RU/translation.json index 3f14377ffa..c94badbb1b 100644 --- a/ui/public/locales/ru-RU/translation.json +++ b/ui/public/locales/ru-RU/translation.json @@ -474,5 +474,13 @@ "Moderator": "Модератор", "Make Moderator": "Сделать модератором", "Remove Moderator": "Убрать модератора", - "Group privilege": "Права в группе" + "Group privilege": "Права в группе", + "Optional. If you add keys, only their owners can connect via adb connect. Without keys, anyone with the command can connect.": "Необязательно. Если добавить ключи, подключиться через adb connect смогут только их владельцы. Без ключей — любой, у кого есть команда.", + "Lifetime": "Срок действия", + "No expiration": "Без срока", + "1 month": "1 месяц", + "2 months": "2 месяца", + "3 months": "3 месяца", + "Expires on {{date}}": "Действует до {{date}}", + "Never expires": "Бессрочный" } diff --git a/ui/src/components/lib/copyable-block/copyable-block.tsx b/ui/src/components/lib/copyable-block/copyable-block.tsx index 8cd703744c..55a9eeda16 100644 --- a/ui/src/components/lib/copyable-block/copyable-block.tsx +++ b/ui/src/components/lib/copyable-block/copyable-block.tsx @@ -9,14 +9,22 @@ import styles from './copyable-block.module.css' type CopyableBlockProps = { copyableText: string className?: string + contentClassName?: string title?: string onOkClick?: () => void isClosable?: boolean } -export const CopyableBlock = ({ title, copyableText, onOkClick, isClosable = true, className }: CopyableBlockProps) => ( +export const CopyableBlock = ({ + title, + copyableText, + onOkClick, + isClosable = true, + className, + contentClassName, +}: CopyableBlockProps) => ( - + + + ({ ...option, name: t(option.name) }))} + value={lifetime} + onChange={(value) => setLifetime(Number(value) as CreateAccessTokenLifetime)} + /> + + + + + + + + + @@ -141,6 +181,11 @@ export const AccessTokensControl = observer(({ className }: { className?: string onClose={handleCloseTokenDetail} >
+ + {tokenDetail?.expiresAt + ? t('Expires on {{date}}', { date: formatDateWithLocale(new Date(tokenDetail.expiresAt)) }) + : t('Never expires')} + form { + display: contents; + } + + /* The card already pads its content on the left */ + & > form > * { + padding-left: 0; + } + + & > form > * { + grid-column: 1; + } + + /* Under both the form and the instruction */ + & > form > .submit { + grid-column: 1 / -1; + } + + @media (max-width: 768px) { + grid-template-columns: 1fr; + } +} + +.instruction { + display: flex; + flex-direction: column; + grid-row: 1 / span 2; + grid-column: 2; + align-self: stretch; + + @media (max-width: 768px) { + grid-row: auto; + grid-column: 1; + order: -1; + /* In line with the fields under it */ + padding-left: 0; + } +} + +/* As wide as the command: the title wraps to it */ +.instructionBlock { + flex: 1; + width: min-content; + + /* The free height goes between the title and the command */ + & > * { + flex: 1; + flex-direction: column; + justify-content: space-between; + } + + @media (max-width: 768px) { + width: auto; + } +} + +/* The block keeps its own padding on every side, the command gets no extra one */ +.instructionBlock .instructionContent { + padding: 12px 0 0; + + & > * { + min-width: max-content; + } +} diff --git a/ui/src/components/ui/settings-tabs/keys-tab/adb-keys-control/adb-keys-control.tsx b/ui/src/components/ui/settings-tabs/keys-tab/adb-keys-control/adb-keys-control.tsx index 6ec8fb069d..2859ba0f97 100644 --- a/ui/src/components/ui/settings-tabs/keys-tab/adb-keys-control/adb-keys-control.tsx +++ b/ui/src/components/ui/settings-tabs/keys-tab/adb-keys-control/adb-keys-control.tsx @@ -10,13 +10,14 @@ import { Icon20AddSquareOutline, Icon20ComputerSmartphoneOutline, } from '@vkontakte/icons' -import { Button, Div, FormItem, FormStatus, Input, List, Placeholder, Textarea } from '@vkontakte/vkui' +import { Button, Div, Footnote, FormItem, FormStatus, Input, List, Placeholder, Textarea } from '@vkontakte/vkui' import { WarningModal } from '@/components/ui/modals' import { CopyableBlock } from '@/components/lib/copyable-block' import { ConditionalRender } from '@/components/lib/conditional-render' import { ContentCard } from '@/components/lib/content-card' +import { useMediaQuery } from '@/lib/hooks/use-media-query.hook' import { CONTAINER_IDS } from '@/config/inversify/container-ids' import { KeyListItem } from '../key-list-item' @@ -28,6 +29,8 @@ import type { FormEvent } from 'react' export const AdbKeysControl = observer(({ className }: { className?: string }) => { const { t } = useTranslation() const [isConfirmationOpen, setIsConfirmationOpen] = useState(false) + // Side by side with the form only on a wide screen + const isWide = !useMediaQuery('(max-width: 768px)') const adbKeyService = useInjection(CONTAINER_IDS.adbKeyService) const currentUserProfileStore = useInjection(CONTAINER_IDS.currentUserProfileStore) @@ -57,51 +60,63 @@ export const AdbKeysControl = observer(({ className }: { className?: string }) = separator onAfterButtonClick={() => adbKeyService.setIsAddAdbKeyOpen((prev) => !prev)} > + + {t( + 'Optional. If you add keys, only their owners can connect via adb connect. Without keys, anyone with the command can connect.' + )} + - -
- -