Medium refactoring

Summary:
Simplifies medium usage. Clients report this value as an integer. Internally, we transform this integer as type (a set of valid strings).

Instead of transform this value in different places, do it once when the client query is received.

Reviewed By: antonk52

Differential Revision: D46358024

fbshipit-source-id: ecd2b6c6ccbe7c38787a89d4e2f81930c7b91864

desktop/flipper-common/src/server-types.tsx

@@ -30,6 +30,7 @@ import {LoggerInfo} from './utils/Logger';
 // Since flipper-plugin however is currently shared among server, client and defines a lot of base types, leaving it here for now.
 
 export type FlipperServerType = 'embedded' | 'external';
+export type CertificateExchangeMedium = 'FS_ACCESS' | 'WWW' | 'NONE';
 
 export type FlipperServerState =
   | 'pending'
@@ -87,6 +88,7 @@ export type ClientQuery = {
   readonly device: string;
   readonly device_id: string;
   readonly sdk_version?: number;
+  medium: CertificateExchangeMedium;
   rsocket?: boolean;
 };
 

desktop/flipper-server-core/src/FlipperServerImpl.tsx

@@ -11,7 +11,6 @@ import './utils/macCa';
 import './utils/fetch-polyfill';
 import EventEmitter from 'events';
 import {ServerController} from './comms/ServerController';
-import {CertificateExchangeMedium} from './utils/CertificateProvider';
 import {AndroidDeviceManager} from './devices/android/androidDeviceManager';
 import {IOSDeviceManager} from './devices/ios/iOSDeviceManager';
 import metroDevice from './devices/metro/metroDeviceManager';
@@ -26,6 +25,7 @@ import {
   Logger,
   FlipperServerExecOptions,
   DeviceDebugData,
+  CertificateExchangeMedium,
 } from 'flipper-common';
 import {ServerDevice} from './devices/ServerDevice';
 import {Base64} from 'js-base64';

desktop/flipper-server-core/src/comms/BrowserServerWebSocket.tsx

@@ -144,7 +144,7 @@ class BrowserServerWebSocket extends SecureServerWebSocket {
     if (!parsedBaseQuery) {
       return;
     }
-    return {...parsedBaseQuery, medium: 3};
+    return {...parsedBaseQuery, medium: 'NONE'};
   }
 
   protected verifyClient(): ws.VerifyClientCallbackSync {

desktop/flipper-server-core/src/comms/ServerAdapter.tsx

@@ -7,9 +7,7 @@
  * @format
  */
 
-import {CertificateExchangeMedium} from '../utils/CertificateProvider';
 import {ClientConnection} from './ClientConnection';
-import {transformCertificateExchangeMediumToType} from './Utilities';
 import {
   ClientDescription,
   ClientQuery,
@@ -30,8 +28,7 @@ export type ClientCsrQuery = {
  * SecureClientQuery combines a ClientQuery with
  * ClientCsrQuery. It also adds medium information.
  */
-export type SecureClientQuery = ClientQuery &
+export type SecureClientQuery = ClientQuery & ClientCsrQuery;
-  ClientCsrQuery & {medium: 1 /*FS*/ | 2 /*WWW*/ | 3 /*NONE*/ | undefined};
 
 /**
  * Defines an interface for events triggered by a running server interacting
@@ -75,7 +72,6 @@ export interface ServerEventsListener {
     unsanitizedCSR: string,
     clientQuery: ClientQuery,
     appDirectory: string,
-    medium: CertificateExchangeMedium,
   ): Promise<{deviceId: string}>;
   /**
    * A secure connection has been established with a validated client.
@@ -161,7 +157,7 @@ abstract class ServerAdapter {
     if (message.method === 'signCertificate') {
       console.debug('CSR received from device', 'server');
 
-      const {csr, destination, medium} = message;
+      const {csr, destination} = message;
 
       console.info(
         `[conn] Starting certificate exchange: ${clientQuery.app} on ${clientQuery.device}`,
@@ -171,7 +167,6 @@ abstract class ServerAdapter {
           csr,
           clientQuery,
           destination,
-          transformCertificateExchangeMediumToType(medium),
         );
 
         console.info(

desktop/flipper-server-core/src/comms/ServerController.tsx

@@ -7,7 +7,6 @@
  * @format
  */
 
-import {CertificateExchangeMedium} from '../utils/CertificateProvider';
 import {
   ClientDescription,
   ClientQuery,
@@ -27,7 +26,6 @@ import {
   appNameWithUpdateHint,
   assertNotNull,
   cloneClientQuerySafeForLogging,
-  transformCertificateExchangeMediumToType,
 } from './Utilities';
 import ServerAdapter, {
   SecureClientQuery,
@@ -182,7 +180,7 @@ export class ServerController
       medium,
       rsocket,
     } = clientQuery;
-    const transformedMedium = transformCertificateExchangeMediumToType(medium);
+
     console.info(
       `[conn] Connection established: ${app} on ${device_id}. Medium ${medium}. CSR: ${csr_path}`,
       cloneClientQuerySafeForLogging(clientQuery),
@@ -204,7 +202,7 @@ export class ServerController
         device,
         device_id,
         sdk_version,
-        medium: transformedMedium,
+        medium,
         rsocket,
       },
       {csr, csr_path},
@@ -233,10 +231,13 @@ export class ServerController
       ...timestamp,
     });
 
-    tracker.track(
+    tracker.track('app-connection-secure-attempt', {
-      'app-connection-secure-attempt',
+      app: clientQuery.app,
-      (({csr, ...o}) => o)(clientQuery),
+      os: clientQuery.os,
-    );
+      device: clientQuery.device,
+      device_id: clientQuery.device_id,
+      medium: clientQuery.medium,
+    });
 
     const {os, app, device_id} = clientQuery;
     // without these checks, the user might see a connection timeout error instead, which would be much harder to track down
@@ -260,16 +261,13 @@ export class ServerController
       clearTimeout(timeout);
     }
 
-    const transformedMedium = transformCertificateExchangeMediumToType(
+    if (clientQuery.medium === 'WWW' || clientQuery.medium === 'NONE') {
-      clientQuery.medium,
-    );
-    if (transformedMedium === 'WWW' || transformedMedium === 'NONE') {
       this.flipperServer.registerDevice(
         new DummyDevice(
           this.flipperServer,
           clientQuery.device_id,
           clientQuery.app +
-            (transformedMedium === 'WWW' ? ' Server Exchanged' : ''),
+            (clientQuery.medium === 'WWW' ? ' Server Exchanged' : ''),
           clientQuery.os,
         ),
       );
@@ -299,7 +297,6 @@ export class ServerController
     unsanitizedCSR: string,
     clientQuery: ClientQuery,
     appDirectory: string,
-    medium: CertificateExchangeMedium,
   ): Promise<{deviceId: string}> {
     let certificateProvider: CertificateProvider;
     switch (clientQuery.os) {
@@ -318,7 +315,7 @@ export class ServerController
         );
         certificateProvider = this.flipperServer.ios.certificateProvider;
 
-        if (medium === 'WWW') {
+        if (clientQuery.medium === 'WWW') {
           certificateProvider = new WWWCertificateProvider(
             this.flipperServer.keytarManager,
           );
@@ -339,7 +336,7 @@ export class ServerController
       }
     }
 
-    certificateProvider.verifyMedium(medium);
+    certificateProvider.verifyMedium(clientQuery.medium);
 
     return new Promise((resolve, reject) => {
       reportPlatformFailures(
@@ -367,7 +364,7 @@ export class ServerController
             setTimeout(() => {
               this.emit('client-unresponsive-error', {
                 client,
-                medium,
+                medium: clientQuery.medium,
                 deviceID: response.deviceId,
               });
             }, 30 * 1000),
@@ -428,7 +425,7 @@ export class ServerController
    */
   async addConnection(
     connection: ClientConnection,
-    query: ClientQuery & {medium: CertificateExchangeMedium},
+    query: ClientQuery,
     csrQuery: ClientCsrQuery,
     silentReplace?: boolean,
   ): Promise<ClientDescription> {

desktop/flipper-server-core/src/comms/ServerWebSocket.tsx

@@ -99,7 +99,7 @@ class ServerWebSocket extends ServerAdapter {
         });
 
         try {
-          this.onConnection(ws, request);
+          this.onConnection(ws, request); // insecure connection, with medium.
         } catch (error) {
           // TODO: Investigate if we need to close the socket in the `error` listener
           // DRI: @aigoncharov

desktop/flipper-server-core/src/comms/Utilities.tsx

@@ -7,9 +7,13 @@
  * @format
  */
 
-import {ClientQuery, DeviceOS, ResponseMessage} from 'flipper-common';
+import {
+  CertificateExchangeMedium,
+  ClientQuery,
+  DeviceOS,
+  ResponseMessage,
+} from 'flipper-common';
 import {ParsedUrlQuery} from 'querystring';
-import {CertificateExchangeMedium} from '../utils/CertificateProvider';
 import {SecureClientQuery} from './ServerAdapter';
 
 /**
@@ -126,11 +130,23 @@ export function parseClientQuery(
     return;
   }
 
+  let medium: number | undefined;
+  if (typeof query.medium === 'string') {
+    medium = parseInt(query.medium, 10);
+  } else if (typeof query.medium === 'number') {
+    medium = query.medium;
+  }
+
+  if (medium !== undefined && (medium < 1 || medium > 3)) {
+    throw new Error('Unsupported exchange medium: ' + medium);
+  }
+
   const clientQuery: ClientQuery = {
     device_id,
     device,
     app,
     os,
+    medium: transformCertificateExchangeMediumToType(medium),
   };
 
   if (typeof query.sdk_version === 'string') {
@@ -178,11 +194,19 @@ export function parseSecureClientQuery(
   let medium: number | undefined;
   if (typeof query.medium === 'string') {
     medium = parseInt(query.medium, 10);
+  } else if (typeof query.medium === 'number') {
+    medium = query.medium;
   }
+
   if (medium !== undefined && (medium < 1 || medium > 3)) {
     throw new Error('Unsupported exchange medium: ' + medium);
   }
-  return {...clientQuery, csr, csr_path, medium: medium as any};
+  return {
+    ...clientQuery,
+    csr,
+    csr_path,
+    medium: transformCertificateExchangeMediumToType(medium),
+  };
 }
 
 export function cloneClientQuerySafeForLogging(clientQuery: SecureClientQuery) {

desktop/flipper-server-core/src/comms/__tests__/BrowserServerWebSocket.node.tsx

@@ -81,7 +81,7 @@ describe('BrowserServerWebSocket', () => {
       os,
       app,
       sdk_version: sdkVersion,
-      medium: 3,
+      medium: 'NONE',
     };
     expect(mockSEListener.onConnectionAttempt).toBeCalledWith(
       expectedClientQuery,
@@ -183,7 +183,7 @@ describe('BrowserServerWebSocket', () => {
       os: 'MacOS',
       app: device,
       sdk_version: 4,
-      medium: 3,
+      medium: 'NONE',
     };
     expect(mockSEListener.onConnectionAttempt).toBeCalledWith(
       expectedClientQuery,

desktop/flipper-server-core/src/comms/__tests__/SecureServerWebSocket.node.tsx

@@ -18,6 +18,7 @@ import WebSocket from 'ws';
 
 import SecureServerWebSocket from '../SecureServerWebSocket';
 import {SecureClientQuery} from '../ServerAdapter';
+import {transformCertificateExchangeMediumToType} from '../Utilities';
 import WebSocketClientConnection from '../WebSocketClientConnection';
 import {createMockSEListener, WSMessageAccumulator} from './utils';
 
@@ -80,7 +81,7 @@ describe('SecureServerWebSocket', () => {
       sdk_version: sdkVersion,
       csr,
       csr_path: csrPath,
-      medium,
+      medium: transformCertificateExchangeMediumToType(medium),
     };
     expect(mockSEListener.onSecureConnectionAttempt).toBeCalledWith(
       expectedClientQuery,

desktop/flipper-server-core/src/comms/__tests__/ServerWebSocket.node.tsx

@@ -45,7 +45,7 @@ describe('ServerWebSocket', () => {
 
     expect(mockSEListener.onConnectionAttempt).toBeCalledTimes(0);
     wsClient = new WebSocket(
-      `ws://localhost:${port}?device_id=${deviceId}&device=${device}&app=${app}&os=${os}&sdk_version=${sdkVersion}`,
+      `ws://localhost:${port}?device_id=${deviceId}&device=${device}&app=${app}&os=${os}&sdk_version=${sdkVersion}&medium=2`,
     );
     const receivedMessages = new WSMessageAccumulator();
     await new Promise<void>((resolve, reject) => {
@@ -60,6 +60,7 @@ describe('ServerWebSocket', () => {
       os,
       app,
       sdk_version: sdkVersion,
+      medium: 'WWW',
     };
     expect(mockSEListener.onConnectionAttempt).toBeCalledWith(
       expectedClientQuery,

desktop/flipper-server-core/src/utils/CertificateProvider.tsx

@@ -7,6 +7,7 @@
  * @format
  */
 
+import {CertificateExchangeMedium} from 'flipper-common';
 import {
   deviceCAcertFile,
   deviceClientCertFile,
@@ -16,8 +17,6 @@ import {
   getCACertificate,
 } from './certificateUtils';
 
-export type CertificateExchangeMedium = 'FS_ACCESS' | 'WWW' | 'NONE';
-
 export default abstract class CertificateProvider {
   abstract medium: CertificateExchangeMedium;
   abstract name: string;

desktop/flipper-server-core/src/utils/tracker.tsx

@@ -7,14 +7,14 @@
  * @format
  */
 
-import {getLogger} from 'flipper-common';
+import {getLogger, CertificateExchangeMedium} from 'flipper-common';
 
 type AppConnectionPayload = {
   app: string;
   os: string;
   device: string;
   device_id: string;
-  medium?: number | undefined;
+  medium?: CertificateExchangeMedium;
 };
 
 type AppConnectionCertificateExchangePayload = AppConnectionPayload & {

desktop/flipper-ui-core/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap

@@ -9,6 +9,7 @@ exports[`can create a Fake flipper with legacy wrapper 1`] = `
         "app": "TestApp",
         "device": "MockAndroidDevice",
         "device_id": "serial",
+        "medium": "NONE",
         "os": "Android",
         "sdk_version": 4,
       },

desktop/flipper-ui-core/src/__tests__/test-utils/MockFlipper.tsx

@@ -180,6 +180,7 @@ export default class MockFlipper {
       device: device.title,
       device_id: device.serial,
       sdk_version: 4,
+      medium: 'NONE',
     };
     const id = buildClientId({
       app: query.app,

desktop/flipper-ui-core/src/utils/__tests__/exportData.node.tsx

@@ -105,7 +105,13 @@ function generateClientFromClientWithSalt(
   const identifier = generateClientIdentifierWithSalt(client.id, salt);
   return {
     id: identifier,
-    query: {app, os, device, device_id: salt + '-' + device_id},
+    query: {
+      app,
+      os,
+      device,
+      device_id: salt + '-' + device_id,
+      medium: client.query.medium,
+    },
   };
 }
 
@@ -114,7 +120,7 @@ function generateClientFromDevice(device: Device, app: string): ClientExport {
   const identifier = generateClientIdentifier(device, app);
   return {
     id: identifier,
-    query: {app, os, device: deviceType, device_id: serial},
+    query: {app, os, device: deviceType, device_id: serial, medium: 'NONE'},
   };
 }
 
@@ -149,6 +155,7 @@ test('test generateClientFromClientWithSalt helper function', () => {
       os: 'iOS',
       device: 'emulator',
       device_id: 'salt-serial',
+      medium: 'NONE',
     },
   });
   expect(client).toEqual({
@@ -158,6 +165,7 @@ test('test generateClientFromClientWithSalt helper function', () => {
       os: 'iOS',
       device: 'emulator',
       device_id: 'serial',
+      medium: 'NONE',
     },
   });
 });
@@ -178,6 +186,7 @@ test('test generateClientFromDevice helper function', () => {
       os: 'iOS',
       device: 'emulator',
       device_id: 'serial',
+      medium: 'NONE',
     },
   });
 });
@@ -727,6 +736,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -742,6 +752,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -757,6 +768,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -816,6 +828,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -831,6 +844,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -873,6 +887,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -888,6 +903,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -935,6 +951,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -950,6 +967,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial1',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -965,6 +983,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial2',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -980,6 +999,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial2',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -1051,6 +1071,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial',
+      medium: 'NONE',
     },
     null,
     logger,
@@ -1066,6 +1087,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
       os: 'iOS',
       device: 'TestiPhone',
       device_id: 'serial-archived',
+      medium: 'NONE',
     },
     null,
     logger,