Allow the port for new access keys to be editable from the UI. (#505)

* Allow the port fo rnew access keys to be changed from the UI

* Remove old TODOs

* Use correct log levels

* CSS formatting

* Use Number.isNan

* short circuit on no change and kill dead code

* Implement fakes, fix display bug

* fix typo

* Respond to code review

* Fix underline coloring

* Limit to numerical input

* Respond to review comments

* Review comments

* Disable editing key port for old servers (#513)

* Disable editing key port for old servers

* Use -1 instead of the current port number, nits

* Be more conservative with the response code check

* Use on-tap instead of on-click for the keys port input

* Rename "toggleFoo" to "setFoo"

* Use 1.0.0 instead of 0.2.0

* Respond to review comments.  Also fixes a bug where, if the input was
invalid due to a server-side error, hitting Enter did nothing except
show a valid state

* Correct the previous bugfix

src/server_manager/messages/en.json

@@ -40,8 +40,12 @@
   "error-key-remove": "Failed to remove key",
   "error-key-rename": "Failed to rename key",
   "error-keys-get": "Could not load keys",
+  "error-keys-port-bad-input": "The port must be an integer between 1 and 65,535.",
+  "error-keys-port-in-use": "The port is already in use on the server.",
   "error-licenses": "Could not load licenses.",
   "error-metrics": "Error setting metrics enabled",
+  "error-network": "A network error occurred.",
+  "error-not-saved": "Not Saved",
   "error-server-creation": "There was an error creating your Outline server.",
   "error-server-destroy": "Failed to destroy server",
   "error-server-removed": "{serverName} no longer present in your DigitalOcean account.",
@@ -49,6 +53,7 @@
   "error-server-unreachable": "Your Outline Server was installed correctly, but we are not able to connect to it. Most likely this is because your server's firewall rules are blocking incoming connections. Please review them and make sure to allow incoming TCP connections on ports ranging from 1024 to 65535.",
   "error-server-unreachable-title": "Unable to connect to your Outline Server",
   "error-servers-removed": "{serverNames} no longer present in your DigitalOcean account.",
+  "error-unexpected": "An unexpected error occurred.",
   "feedback-cloud-provider": "Select cloud provider",
   "feedback-cloud-provider-error": "Please select a cloud provider.",
   "feedback-connection": "Can't connect to my server",
@@ -125,6 +130,9 @@
   "region-title": "Select the location of your server.",
   "remove": "Remove",
   "retry": "Retry",
+  "save": "Save",
+  "saved": "Saved",
+  "saving": "Saving...",
   "server-access": "Server access",
   "server-access-key-new": "Add new key",
   "server-access-key-rename": "Rename",

src/server_manager/messages/master_messages.json

@@ -205,6 +205,14 @@
     "message": "Could not load keys",
     "description": "This string appears in an error notification toast. It is shown when there is an error retrieving a server access keys."
   },
+  "error_keys_port_bad_input": {
+    "message": "The port must be an integer between 1 and 65,535.",
+    "description": "This string appears in an inline error message.  It signifies that the input number for the port for new access keys is invalid."
+  },
+  "error_keys_port_in_use": {
+    "message": "The port is already in use on the server.",
+    "description": "This string appears in an inline error message.  It signifies that the input port for new access keys is already being used on the server and is unavailable."
+  },
   "error_licenses": {
     "message": "Could not load licenses.",
     "description": "This string appears in a dialog that shows the application's software licenses. It is shown instead of the licenses' text when loading them fails."
@@ -213,6 +221,14 @@
     "message": "Error setting metrics enabled",
     "description": "This string appears in an error notification toast. It is shown when there is an error enabling or disabling a server's metrics reporting."
   },
+  "error_network": {
+    "messages": "A network error occurred.",
+    "description": "This string indicates that an error happened due to network errors like not being connected to the internet."
+  },
+  "error_not_saved": {
+    "messages": "Not Saved",
+    "description": "This string appears in an error notification toast.  It signifies failure to submit user input to change the port number for new access keys."
+  },
   "error_server_creation": {
     "message": "There was an error creating your Outline server.",
     "description": "This string appears in an dialog as a paragraph. The dialog is shown when there is an creating a server; the user can retry the operation or destroy the server."
@@ -253,6 +269,10 @@
       }
     }
   },
+  "error_unexpected": {
+    "message": "An unexpected error occurred.",
+    "description": "This string signifies that an error we didn't expect was encountered."
+  },
   "feedback_cloud_provider": {
     "message": "Select cloud provider",
     "description": "This string appears in the feedback dialog as an placeholder within a drop-down. Allows the user to select a cloud provider for certain feedback categories."
@@ -595,6 +615,18 @@
     "message": "Retry",
     "description": "This string appears across the application as a button. It allows the user to retry a failed operation."
   },
+  "save": {
+    "message": "Save",
+    "description": "This string appears across the application as a button.  It allows the user to submit some input"
+  },
+  "saved": {
+    "message": "Saved",
+    "description": "This string appears across the application as a transient notification toast.  It appears when user input has been accepted."
+  },
+  "saving": {
+    "message": "Saving...",
+    "description": "This string appears across the application as a transient notification toast.  It appears when user input is being processed."
+  },
   "server_access": {
     "message": "Server access",
     "description": "This string appears within the server view as the header of a card that displays the number of server access keys."

src/server_manager/model/server.ts

@@ -16,6 +16,9 @@ export interface Server {
   // Get the server's name for display.
   getName(): string;
 
+  // Gets the version of the shadowbox binary the server is running
+  getVersion(): string;
+
   // Updates the server name.
   setName(name: string): Promise<void>;
 
@@ -58,6 +61,9 @@ export interface Server {
   // Returns the port number for new access keys.
   // Returns undefined if the server doesn't have a port set.
   getPortForNewAccessKeys(): number|undefined;
+
+  // Changes the port number for new access keys.
+  setPortForNewAccessKeys(port: number): Promise<void>;
 }
 
 // Manual servers are servers which the user has independently setup to run

src/server_manager/ui_components/outline-server-settings.html

@@ -60,6 +60,11 @@
         justify-content: space-between;
         align-items: flex-start;
       }
+      #portForNewAccessKeysContainer {
+        display: flex;
+        justify-content: space-between;
+        align-items: center;
+      }
       .detail {
         margin-top: 0px;
         font-size: 12px;
@@ -67,6 +72,27 @@
       .clickable {
         cursor: pointer;
       }
+      #keysPortSaveButton {
+        background: var(--primary-green);
+        color: #fff;
+        height: 36px;
+        width: 83px;
+      }
+      #keysPortSaveButton[disabled] {
+        background-color: rgba(255, 255, 255, .12);
+        color: rgba(255, 255, 255, .3);
+      }
+      #keysPortCancelButton {
+        color: #fff;
+        width: 83px;
+      }
+      #keysPortCancelButton[disabled] {
+        color:rgba(255, 255, 255, 0.3);
+        background-color: rgba(0, 0, 0, 0);
+      }
+      #portForNewAccessKeysContainer {
+        max-width: 545px;
+      }
       paper-checkbox {
         /* We want the ink to be the color we're going to, not coming from */
         --paper-checkbox-checked-color: var(--primary-green);
@@ -74,9 +100,14 @@
         --paper-checkbox-unchecked-color: var(--light-gray);
         --paper-checkbox-unchecked-ink-color: var(--primary-green);
       }
+      /* Skip processing these with postcss-rtl as it incorrectly parses the border-color
+         in the paper-input-container-underline-focus mixin. https://github.com/vkalinichev/postcss-rtl/issues/50 */
+      /* rtl:begin:ignore */
       paper-input {
         /* Removes extra padding added by children of paper-input */
         margin-top: -8px;
+        /* Create space for error messages */
+        margin-bottom: 14px;
         --paper-input-container-label-focus: {
           color: var(--primary-green);
         };
@@ -90,9 +121,10 @@
         --paper-input-container-color: var(--medium-gray);
         --paper-input-container-input: {
           color: #fff;
-        }
-
+        };
+        --paper-input-container-invalid-color: #f28b82;
       }
+      /* rtl:end:ignore */
       paper-input[readonly] {
         --paper-input-container-underline: {
           display: none;
@@ -133,7 +165,11 @@
             <h3>[[localize('settings-server-info')]]</h3>
             <paper-input id="serverNameInput" class="server-name" value="{{name}}" label="[[localize('settings-server-name')]]" always-float-label maxlength="100" on-keydown="_handleNameInputKeyDown" on-blur="_handleNameInputBlur"></paper-input>
             <p class="detail">[[localize('settings-server-rename')]]</p>
-            <paper-input readonly value="[[serverPortForNewAccessKeys]]" label="[[localize('settings-access-key-port')]]" hidden$="[[!serverPortForNewAccessKeys]]" always-float-label maxlength="100"></paper-input>
+            <div id="portForNewAccessKeysContainer">
+              <paper-input id="keysPortInput" readonly$="[[!isAccessKeyPortEditable]]" allowed-pattern="[0-9]{1,5}" value="[[serverPortForNewAccessKeys]]" label="[[localize('settings-access-key-port')]]" maxlength="5" hidden$="[[!serverPortForNewAccessKeys]]" on-tap="setKeysPortEditing" on-keyup="_handleKeysPortInputKeyUp" always-float-label maxlength="100"></paper-input>
+              <paper-button id="keysPortCancelButton" hidden$="[[!showKeysPortButtons]]" disabled$="[[!enableKeysPortButtons]]" on-tap="cancelKeysPortEdit">[[localize('cancel')]]</paper-button>
+              <paper-button id="keysPortSaveButton" hidden$="[[!showKeysPortButtons]]" disabled$="[[!enableKeysPortButtons]]" on-tap="saveKeysPort">[[localize('save')]]</paper-button>
+            </div>
             <paper-input readonly value="[[serverHostname]]" label="[[localize('settings-server-hostname')]]" hidden$="[[!serverHostname]]" always-float-label maxlength="100"></paper-input>
             <paper-input readonly value="[[serverManagementApiUrl]]" label="[[localize('settings-server-api-url')]]" hidden$="[[!serverManagementApiUrl]]" always-float-label maxlength="100"></paper-input>
             <paper-input readonly value="[[serverCreationDate]]" label="[[localize('settings-server-creation')]]" hidden$="[[!serverCreationDate]]" always-float-label maxlength="100"></paper-input>
@@ -166,11 +202,14 @@
         serverHostname: {type: String, value: null},
         serverManagementApiUrl: {type: String, value: null},
         serverPortForNewAccessKeys: {type: Number, value: null},
+        isAccessKeyPortEditable: {type: Boolean, value: false},
         serverCreationDate: {type: String, value: null},
         serverLocation: {type: String, value: null},
         serverMonthlyCost: {type: String, value: null},
         serverMonthlyTransferLimit: {type: String, value: null},
         localize: {type: Function, readonly: true},
+        enableKeysPortButtons: {type: Boolean, value: false},
+        showKeysPortButtons: {type: Boolean, value: false}
       },
       update: function(name, metricsEnabled) {
         this.initialName = name;
@@ -208,6 +247,71 @@
           this.fire(metricsSignal);
         }
       },
+      setKeysPortEditing: function() {
+        if (!this.isAccessKeyPortEditable) { 
+          return;
+        }
+        this.showKeysPortButtons = true;
+        const invalid = this.$.keysPortInput.invalid;
+        this.$.keysPortSaveButton.disabled = invalid;
+        this.$.keysPortCancelButton.disabled = false;
+      },
+      setKeysPortSaved: function() {
+        this.serverPortForNewAccessKeys = Number(this.$.keysPortInput.value);
+        this.showKeysPortButtons = false;
+        this.$.keysPortInput.readonly = false;
+      },
+      setKeysPortErrorState: function(message) {
+        const input = this.$.keysPortInput;
+        this.enableKeysPortButtons = true; 
+        input.invalid = true;
+        input.errorMessage = message;
+        input.readonly = false;
+        input.focus();
+      },
+      saveKeysPort: function() {
+        const input = this.$.keysPortInput;
+        if (Number(input.value) === this.serverPortForNewAccessKeys) {
+          this.cancelKeysPortEdit();
+          return;
+        };
+        this.enableKeysPortButtons = false;
+        input.readonly = true;
+        input.invalid = false;
+        this.fire('ChangePortForNewAccessKeysRequested', {
+          port: Number(this.$.keysPortInput.value), 
+          serverSettings: this
+        });
+      },
+      cancelKeysPortEdit: function() {
+        this.$.keysPortInput.value = this.serverPortForNewAccessKeys;
+        this.$.keysPortInput.invalid = false;
+        this.showKeysPortButtons = false;
+      },
+      _handleKeysPortInputKeyUp: function(e) {
+        let input = this.$.keysPortInput;
+        const badInputMessage = this.localize('error-keys-port-bad-input');
+        if (event.key === 'Escape') {
+          this.cancelKeysPortEdit();
+          input.blur();
+          return; 
+        } else if (event.key === 'Enter') {
+          if (!input.invalid) {  
+            this.saveKeysPort();
+            input.blur();
+          }
+          return;
+        }
+        let port = Number(input.value);
+        if (Number.isNaN(port) || (port < 1 || port > 65535) || !Number.isInteger(port)) {
+          input.invalid = true;
+          this.$.keysPortSaveButton.disabled = true;
+          input.errorMessage = badInputMessage;
+        } else {
+          input.invalid = false;
+          this.$.keysPortSaveButton.disabled = false;
+        }
+      }
     });
   </script>
 </dom-module>

src/server_manager/ui_components/outline-server-view.html

@@ -501,6 +501,7 @@
             server-hostname="[[serverHostname]]"
             server-management-api-url="[[serverManagementApiUrl]]"
             server-port-for-new-access-keys="[[serverPortForNewAccessKeys]]"
+            is-access-key-port-editable="[[isAccessKeyPortEditable]]"
             server-creation-date="[[serverCreationDate]]"
             server-monthly-cost="[[monthlyCost]]"
             server-monthly-transfer-limit="[[_formatBytesTransferred(monthlyOutboundTransferBytes)]]"
@@ -540,6 +541,7 @@
         serverHostname: String,
         serverManagementApiUrl: String,
         serverPortForNewAccessKeys: Number,
+        isAccessKeyPortEditable: {type: Boolean, value: false},
         serverCreationDate: String,
         serverLocation: String,
         isServerManaged: Boolean,

src/server_manager/web_app/app.spec.ts

@@ -335,6 +335,9 @@ class FakeServer implements server.Server {
     this.name = name;
     return Promise.resolve();
   }
+  getVersion() {
+    return '1.2.3';
+  }
   listAccessKeys() {
     return Promise.resolve([]);
   }
@@ -375,6 +378,9 @@ class FakeServer implements server.Server {
   getPortForNewAccessKeys(): number|undefined {
     return undefined;
   }
+  setPortForNewAccessKeys(): Promise<void> {
+    return Promise.reject(new Error('FakeServer.setPortForNewAccessKeys not implemented'));
+  }
 }
 
 class FakeManualServer extends FakeServer implements server.ManualServer {

src/server_manager/web_app/app.ts

@@ -14,6 +14,7 @@
 
 import * as sentry from '@sentry/electron';
 import * as events from 'events';
+import * as semver from 'semver';
 
 import * as digitalocean_api from '../cloud/digitalocean_api';
 import * as errors from '../infrastructure/errors';
@@ -36,6 +37,8 @@ interface PolymerEvent extends Event {
 //   https://www.digitalocean.com/help/referral-program/
 const UNUSED_DIGITALOCEAN_REFERRAL_CODE = '5ddb4219b716';
 
+const CHANGE_KEYS_PORT_VERSION = "1.0.0";
+
 interface UiAccessKey {
   id: string;
   placeholderName: string;
@@ -106,6 +109,10 @@ export class App {
       this.renameAccessKey(event.detail.accessKeyId, event.detail.newName, event.detail.entry);
     });
 
+    appRoot.addEventListener('ChangePortForNewAccessKeysRequested', (event: PolymerEvent) => {
+      this.setPortForNewAccessKeys(event.detail.port, event.detail.serverSettings);
+    });
+
     // The UI wants us to validate a server management URL.
     // "Reply" by setting a field on the relevant template.
     appRoot.addEventListener('ManualServerEdited', (event: PolymerEvent) => {
@@ -534,7 +541,7 @@ export class App {
         return Promise.reject(e);
       }
 
-      return new Promise((resolve, reject) => {
+      return new Promise<T>((resolve, reject) => {
         this.appRoot.showConnectivityDialog((retry: boolean) => {
           if (retry) {
             this.digitalOceanRetry(f).then(resolve, reject);
@@ -757,6 +764,8 @@ export class App {
     view.serverHostname = selectedServer.getHostname();
     view.serverManagementApiUrl = selectedServer.getManagementApiUrl();
     view.serverPortForNewAccessKeys = selectedServer.getPortForNewAccessKeys();
+    const version = this.selectedServer.getVersion();
+    view.isAccessKeyPortEditable = version && semver.gte(version, CHANGE_KEYS_PORT_VERSION);
     view.serverCreationDate = selectedServer.getCreatedDate().toLocaleString(
         this.appRoot.language, {year: 'numeric', month: 'long', day: 'numeric'});
 
@@ -911,6 +920,27 @@ export class App {
         });
   }
 
+  private async setPortForNewAccessKeys(port: number, serverSettings: Polymer) {
+    this.appRoot.showNotification(this.appRoot.localize("saving"));
+    try {
+      await this.selectedServer.setPortForNewAccessKeys(port);
+      this.appRoot.showNotification(this.appRoot.localize("saved"));
+      serverSettings.setKeysPortSaved();
+    } catch (error) {
+      this.appRoot.showError(this.appRoot.localize("error-not-saved"));
+      if (error.isNetworkError()) {
+        serverSettings.setKeysPortErrorState(this.appRoot.localize("error-network"));
+        return;
+      }
+      const code = error.response.status;
+      if (code === 409) {
+        serverSettings.setKeysPortErrorState(this.appRoot.localize("error-keys-port-in-use"));
+        return;
+      }
+      serverSettings.setKeysPortErrorState(this.appRoot.localize("error-unexpected"));
+    }
+  }
+
   // Returns promise which fulfills when the server is created successfully,
   // or rejects with an error message that can be displayed to the user.
   public createManualServer(userInput: string): Promise<void> {

src/server_manager/web_app/shadowbox_server.ts

@@ -28,6 +28,7 @@ export interface ServerConfig {
   serverId: string;
   createdTimestampMs: number;
   portForNewAccessKeys: number;
+  version: string;
 }
 
 export class ShadowboxServer implements server.Server {
@@ -80,6 +81,10 @@ export class ShadowboxServer implements server.Server {
     });
   }
 
+  getVersion(): string {
+    return this.serverConfig.version;
+  }
+
   getMetricsEnabled(): boolean {
     return this.serverConfig.metricsEnabled;
   }
@@ -143,6 +148,18 @@ export class ShadowboxServer implements server.Server {
     }
   }
 
+  setPortForNewAccessKeys(newPort: number): Promise<void> {
+    console.info(`setPortForNewAcessKeys: ${newPort}`);
+    const requestOptions: RequestInit = {
+      method: 'PUT',
+      headers: new Headers({'Content-Type': 'application/json'}),
+      body: JSON.stringify({"port": newPort})
+    };
+    return this.apiRequest<void>('server/port-for-new-access-keys', requestOptions).then(() => {
+      this.serverConfig.portForNewAccessKeys = newPort;
+    });
+  }
+
   private getServerConfig(): Promise<ServerConfig> {
     console.info('Retrieving server configuration');
     return this.apiRequest<ServerConfig>('server');

src/shadowbox/package.json

@@ -1,7 +1,7 @@
 {
   "name": "outline-server",
   "private": true,
-  "version": "0.1.0",
+  "version": "1.0.0",
   "description": "Outline server",
   "main": "build/server/main.js",
   "author": "Outline",

src/shadowbox/server/manager_service.ts

@@ -14,6 +14,7 @@
 
 import * as restify from 'restify';
 import {makeConfig, SIP002_URI} from 'ShadowsocksConfig/shadowsocks_config';
+import {version} from '../package.json';
 
 import {JsonConfig} from '../infrastructure/json_config';
 import * as logging from '../infrastructure/logging';
@@ -140,7 +141,8 @@ export class ShadowsocksManagerService {
       metricsEnabled: this.serverConfig.data().metricsEnabled || false,
       createdTimestampMs: this.serverConfig.data().createdTimestampMs,
       portForNewAccessKeys: this.serverConfig.data().portForNewAccessKeys,
-      dataUsageTimeframe: this.serverConfig.data().dataUsageTimeframe
+      dataUsageTimeframe: this.serverConfig.data().dataUsageTimeframe,
+      version
     });
     next();
   }

src/shadowbox/server/server_config.ts

@@ -34,6 +34,7 @@ export interface ServerConfigJson {
   rollouts?: RolloutConfigJson[];
   // Sliding timeframe, in hours, used to measure data usage and enforce data limits.
   dataUsageTimeframe?: DataUsageTimeframe;
+  // We don't serialize the shadowbox version, this is obtained dynamically from node.
 }
 
 // Serialized format for rollouts.

src/shadowbox/tsconfig.json

@@ -5,7 +5,8 @@
     "noImplicitAny": false,
     "noImplicitThis": true,
     "module": "commonjs",
-    "rootDir": "."
+    "rootDir": ".",
+    "resolveJsonModule": true
   },
   "include": [
     "server/main.ts",