refactor(client): reduce config library surface

client/electron/index.ts

@@ -351,7 +351,7 @@ async function createVpnTunnel(
   // because startVpn will add a routing table entry that prefixed with this
   // host (e.g. "<host>/32"), therefore <host> must be an IP address.
   // TODO: make sure we resolve it in the native code
-  const host = config.getHostFromTransportConfig(tunnelConfig.transport);
+  const host = tunnelConfig.firstHop.host;
   if (!host) {
     throw new errors.IllegalServerConfiguration('host is missing');
   }

client/src/www/app/outline_server_repository/config.spec.ts

@@ -19,36 +19,27 @@ import * as config from './config';
 describe('getAddressFromTransport', () => {
   it('extracts address', () => {
     expect(
-      config.getAddressFromTransportConfig({host: 'example.com', port: '443'})
-    ).toEqual('example.com:443');
+      config.TEST_ONLY.getAddressFromTransportConfig({
+        host: 'example.com',
+        port: 443,
+      })
+    ).toEqual({host: 'example.com', port: 443});
     expect(
-      config.getAddressFromTransportConfig({host: '1:2::3', port: '443'})
-    ).toEqual('[1:2::3]:443');
-    expect(config.getAddressFromTransportConfig({host: 'example.com'})).toEqual(
-      'example.com'
-    );
-    expect(config.getAddressFromTransportConfig({host: '1:2::3'})).toEqual(
-      '1:2::3'
-    );
-  });
-
-  it('fails on invalid config', () => {
-    expect(config.getAddressFromTransportConfig({})).toBeUndefined();
-  });
-});
-
-describe('getHostFromTransport', () => {
-  it('extracts host', () => {
+      config.TEST_ONLY.getAddressFromTransportConfig({
+        host: '1:2::3',
+        port: 443,
+      })
+    ).toEqual({host: '1:2::3', port: 443});
     expect(
-      config.getHostFromTransportConfig({host: 'example.com', port: '443'})
-    ).toEqual('example.com');
+      config.TEST_ONLY.getAddressFromTransportConfig({host: 'example.com'})
+    ).toEqual({host: 'example.com', port: undefined});
     expect(
-      config.getHostFromTransportConfig({host: '1:2::3', port: '443'})
-    ).toEqual('1:2::3');
+      config.TEST_ONLY.getAddressFromTransportConfig({host: '1:2::3'})
+    ).toEqual({host: '1:2::3', port: undefined});
   });
 
   it('fails on invalid config', () => {
-    expect(config.getHostFromTransportConfig({})).toBeUndefined();
+    expect(config.TEST_ONLY.getAddressFromTransportConfig({})).toBeUndefined();
   });
 });
 
@@ -57,24 +48,24 @@ describe('setTransportHost', () => {
     expect(
       JSON.stringify(
         config.setTransportConfigHost(
-          {host: 'example.com', port: '443'},
+          {host: 'example.com', port: 443},
           '1.2.3.4'
         )
       )
-    ).toEqual('{"host":"1.2.3.4","port":"443"}');
+    ).toEqual('{"host":"1.2.3.4","port":443}');
     expect(
       JSON.stringify(
         config.setTransportConfigHost(
-          {host: 'example.com', port: '443'},
+          {host: 'example.com', port: 443},
           '1:2::3'
         )
       )
-    ).toEqual('{"host":"1:2::3","port":"443"}');
+    ).toEqual('{"host":"1:2::3","port":443}');
     expect(
       JSON.stringify(
-        config.setTransportConfigHost({host: '1.2.3.4', port: '443'}, '1:2::3')
+        config.setTransportConfigHost({host: '1.2.3.4', port: 443}, '1:2::3')
       )
-    ).toEqual('{"host":"1:2::3","port":"443"}');
+    ).toEqual('{"host":"1:2::3","port":443}');
   });
 
   it('fails on invalid config', () => {
@@ -89,6 +80,10 @@ describe('parseTunnelConfig', () => {
         '{"server": "example.com", "server_port": 443, "method": "METHOD", "password": "PASSWORD"}'
       )
     ).toEqual({
+      firstHop: {
+        host: 'example.com',
+        port: 443,
+      },
       transport: {
         host: 'example.com',
         port: 443,
@@ -104,6 +99,10 @@ describe('parseTunnelConfig', () => {
         '{"server": "example.com", "server_port": 443, "method": "METHOD", "password": "PASSWORD", "prefix": "POST "}'
       )
     ).toEqual({
+      firstHop: {
+        host: 'example.com',
+        port: 443,
+      },
       transport: {
         host: 'example.com',
         port: 443,
@@ -124,6 +123,10 @@ describe('parseTunnelConfig', () => {
       })
     );
     expect(config.parseTunnelConfig(ssUrl)).toEqual({
+      firstHop: {
+        host: 'example.com',
+        port: 443,
+      },
       transport: {
         host: 'example.com',
         port: 443,
@@ -133,3 +136,23 @@ describe('parseTunnelConfig', () => {
     });
   });
 });
+
+describe('serviceNameFromAccessKey', () => {
+  it('extracts name from ss:// key', () => {
+    expect(
+      config.TEST_ONLY.serviceNameFromAccessKey('ss://anything#My%20Server')
+    ).toEqual('My Server');
+  });
+  it('extracts name from ssconf:// key', () => {
+    expect(
+      config.TEST_ONLY.serviceNameFromAccessKey('ssconf://anything#My%20Server')
+    ).toEqual('My Server');
+  });
+  it('ignores parameters', () => {
+    expect(
+      config.TEST_ONLY.serviceNameFromAccessKey(
+        'ss://anything#foo=bar&My%20Server&baz=boo'
+      )
+    ).toEqual('My Server');
+  });
+});

client/src/www/app/outline_server_repository/config.ts

@@ -12,16 +12,44 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import * as net from '@outline/infrastructure/net';
 import {SHADOWSOCKS_URI} from 'ShadowsocksConfig';
 
 import * as errors from '../../model/errors';
 
+export const TEST_ONLY = {
+  getAddressFromTransportConfig: getAddressFromTransportConfig,
+  serviceNameFromAccessKey: serviceNameFromAccessKey,
+};
+
+export type ServiceConfig = {
+  readonly name: string;
+} & (StaticServiceConfig | DynamicServiceConfig);
+
+export class StaticServiceConfig {
+  constructor(
+    readonly name: string,
+    readonly tunnelconfig: TunnelConfigJson
+  ) {}
+}
+
+export class DynamicServiceConfig {
+  constructor(
+    readonly name: string,
+    readonly transportConfigLocation: URL
+  ) {}
+}
+
+class EndpointAddress {
+  readonly host: string;
+  readonly port: number | undefined;
+}
+
 // Transport configuration. Application code should treat it as opaque, as it's handled by the networking layer.
 export type TransportConfigJson = object;
 
 /** TunnelConfigJson represents the configuration to set up a tunnel. */
 export interface TunnelConfigJson {
+  firstHop: EndpointAddress | undefined;
   /** transport describes how to establish connections to the destinations.
    * See https://github.com/Jigsaw-Code/outline-apps/blob/master/client/go/outline/config.go for format. */
   transport: TransportConfigJson;
@@ -31,34 +59,23 @@ export interface TunnelConfigJson {
  * getAddressFromTransportConfig returns the address of the tunnel server, if there's a meaningful one.
  * This is used to show the server address in the UI when connected.
  */
-export function getAddressFromTransportConfig(
+function getAddressFromTransportConfig(
   transport: TransportConfigJson
-): string | undefined {
-  const hostConfig: {host?: string; port?: string} = transport;
-  if (hostConfig.host && hostConfig.port) {
-    return net.joinHostPort(hostConfig.host, hostConfig.port);
-  } else if (hostConfig.host) {
-    return hostConfig.host;
+): EndpointAddress | undefined {
+  const hostConfig: {host?: string; port?: number} = transport;
+  if (hostConfig.host) {
+    return {host: hostConfig.host, port: hostConfig?.port};
   } else {
     return undefined;
   }
 }
 
-/**
- * getHostFromTransportConfig returns the host of the tunnel server, if there's a meaningful one.
- * This is used by the proxy resolution in Electron.
- */
-export function getHostFromTransportConfig(
-  transport: TransportConfigJson
-): string | undefined {
-  return (transport as unknown as {host: string | undefined}).host;
-}
-
 /**
  * setTransportConfigHost returns a new TransportConfigJson with the given host as the tunnel server.
  * Should only be set if getHostFromTransportConfig returns one.
  * This is used by the proxy resolution in Electron.
  */
+// TODO(fortuna): Move config parsing to Go and do the DNS resolution and IP injection for Electron there.
 export function setTransportConfigHost(
   transport: TransportConfigJson,
   newHost: string
@@ -90,6 +107,9 @@ export function parseTunnelConfig(
     );
   }
 
+  // TODO(fortuna): stop converting to the Go format. Let the Go code convert.
+  // We don't validate the method because that's already done in the Go code as
+  // part of the Dynamic Key connection flow.
   const transport: TransportConfigJson = {
     host: responseJson.server,
     port: responseJson.server_port,
@@ -99,57 +119,70 @@ export function parseTunnelConfig(
   if (responseJson.prefix) {
     (transport as {prefix?: string}).prefix = responseJson.prefix;
   }
-  return {transport};
+  return {
+    transport,
+    firstHop: getAddressFromTransportConfig(transport),
+  };
 }
 
 /** Parses an access key string into a TunnelConfig object. */
-export function staticKeyToTunnelConfig(staticKey: string): TunnelConfigJson {
+function staticKeyToTunnelConfig(staticKey: string): TunnelConfigJson {
+  const config = SHADOWSOCKS_URI.parse(staticKey);
+  if (!isShadowsocksCipherSupported(config.method.data)) {
+    throw new errors.ShadowsocksUnsupportedCipher(
+      config.method.data || 'unknown'
+    );
+  }
+  const transport: TransportConfigJson = {
+    host: config.host.data,
+    port: config.port.data,
+    method: config.method.data,
+    password: config.password.data,
+  };
+  if (config.extra?.['prefix']) {
+    (transport as {prefix?: string}).prefix = config.extra?.['prefix'];
+  }
+  return {
+    transport,
+    firstHop: getAddressFromTransportConfig(transport),
+  };
+}
+
+export function parseAccessKey(accessKey: string): ServiceConfig {
   try {
-    const config = SHADOWSOCKS_URI.parse(staticKey);
-    const transport: TransportConfigJson = {
-      host: config.host.data,
-      port: config.port.data,
-      method: config.method.data,
-      password: config.password.data,
-    };
-    if (config.extra?.['prefix']) {
-      (transport as {prefix?: string}).prefix = config.extra?.['prefix'];
+    accessKey = accessKey.trim();
+
+    // The default service name is extracted from the URL fragment of the access key.
+    const name = serviceNameFromAccessKey(accessKey);
+
+    // Static ss:// keys. It encodes the full service config.
+    if (accessKey.startsWith('ss://')) {
+      return new StaticServiceConfig(name, parseTunnelConfig(accessKey));
+    }
+
+    // Dynamic ssconf:// keys. It encodes the location of the service config.
+    if (accessKey.startsWith('ssconf://') || accessKey.startsWith('https://')) {
+      try {
+        // URL does not parse the hostname (treats as opaque string) if the protocol is non-standard (e.g. non-http).
+        const configLocation = new URL(
+          accessKey.replace(/^ssconf:\/\//, 'https://')
+        );
+        return new DynamicServiceConfig(name, configLocation);
+      } catch (error) {
+        throw new errors.ServerUrlInvalid(error.message);
+      }
     }
-    return {transport};
-  } catch (cause) {
+
+    throw new TypeError('Access Key is not a ss:// or ssconf:// URL');
+  } catch (e) {
     throw new errors.ServerAccessKeyInvalid('Invalid static access key.', {
-      cause,
+      cause: e,
     });
   }
 }
 
 export function validateAccessKey(accessKey: string) {
-  if (!isDynamicAccessKey(accessKey)) {
-    return validateStaticKey(accessKey);
-  }
-
-  try {
-    // URL does not parse the hostname if the protocol is non-standard (e.g. non-http)
-    new URL(accessKey.replace(/^ssconf:\/\//, 'https://'));
-  } catch (error) {
-    throw new errors.ServerUrlInvalid(error.message);
-  }
-}
-
-function validateStaticKey(staticKey: string) {
-  let config = null;
-  try {
-    config = SHADOWSOCKS_URI.parse(staticKey);
-  } catch (error) {
-    throw new errors.ServerUrlInvalid(
-      error.message || 'failed to parse access key'
-    );
-  }
-  if (!isShadowsocksCipherSupported(config.method.data)) {
-    throw new errors.ShadowsocksUnsupportedCipher(
-      config.method.data || 'unknown'
-    );
-  }
+  parseAccessKey(accessKey);
 }
 
 // We only support AEAD ciphers for Shadowsocks.
@@ -165,21 +198,13 @@ function isShadowsocksCipherSupported(cipher?: string): boolean {
   return SUPPORTED_SHADOWSOCKS_CIPHERS.includes(cipher);
 }
 
-// TODO(daniellacosse): write unit tests for these functions
-// Determines if the key is expected to be a url pointing to an ephemeral session config.
-export function isDynamicAccessKey(accessKey: string): boolean {
-  return accessKey.startsWith('ssconf://') || accessKey.startsWith('https://');
-}
-
 /**
  * serviceNameFromAccessKey extracts the service name from the access key.
  * This is done by getting parsing the fragment hash in the URL and returning the
  * entry that is not a key=value pair.
  * This is used to name the service card in the UI when the service is added.
  */
-export function serviceNameFromAccessKey(
-  accessKey: string
-): string | undefined {
+function serviceNameFromAccessKey(accessKey: string): string | undefined {
   const {hash} = new URL(accessKey.replace(/^ss(?:conf)?:\/\//, 'https://'));
 
   if (!hash) return;