From 3d323d8224cef0f253ed7f17b1e96cf2e50c6a89 Mon Sep 17 00:00:00 2001 From: Jonathan Cohen Date: Tue, 22 Oct 2019 13:04:35 -0400 Subject: [PATCH] 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 | 8 ++ .../messages/master_messages.json | 32 +++++ src/server_manager/model/server.ts | 6 + .../outline-server-settings.html | 110 +++++++++++++++++- .../ui_components/outline-server-view.html | 2 + src/server_manager/web_app/app.spec.ts | 6 + src/server_manager/web_app/app.ts | 32 ++++- .../web_app/shadowbox_server.ts | 17 +++ src/shadowbox/package.json | 2 +- src/shadowbox/server/manager_service.ts | 4 +- src/shadowbox/server/server_config.ts | 1 + src/shadowbox/tsconfig.json | 3 +- 12 files changed, 216 insertions(+), 7 deletions(-) diff --git a/src/server_manager/messages/en.json b/src/server_manager/messages/en.json index 529fdd8d..b1af3aa0 100644 --- a/src/server_manager/messages/en.json +++ b/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", diff --git a/src/server_manager/messages/master_messages.json b/src/server_manager/messages/master_messages.json index 8ecbe3ff..bbf24826 100644 --- a/src/server_manager/messages/master_messages.json +++ b/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." diff --git a/src/server_manager/model/server.ts b/src/server_manager/model/server.ts index dee1c650..5368372e 100644 --- a/src/server_manager/model/server.ts +++ b/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; @@ -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; } // Manual servers are servers which the user has independently setup to run diff --git a/src/server_manager/ui_components/outline-server-settings.html b/src/server_manager/ui_components/outline-server-settings.html index 77810835..2cf830fd 100644 --- a/src/server_manager/ui_components/outline-server-settings.html +++ b/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 @@

[[localize('settings-server-info')]]

[[localize('settings-server-rename')]]

- +
+ + [[localize('cancel')]] + [[localize('save')]] +
@@ -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; + } + } }); diff --git a/src/server_manager/ui_components/outline-server-view.html b/src/server_manager/ui_components/outline-server-view.html index 851c48d3..02f77d55 100644 --- a/src/server_manager/ui_components/outline-server-view.html +++ b/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, diff --git a/src/server_manager/web_app/app.spec.ts b/src/server_manager/web_app/app.spec.ts index a3756f40..7066e749 100644 --- a/src/server_manager/web_app/app.spec.ts +++ b/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 { + return Promise.reject(new Error('FakeServer.setPortForNewAccessKeys not implemented')); + } } class FakeManualServer extends FakeServer implements server.ManualServer { diff --git a/src/server_manager/web_app/app.ts b/src/server_manager/web_app/app.ts index f92aee23..5ee62390 100644 --- a/src/server_manager/web_app/app.ts +++ b/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((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 { diff --git a/src/server_manager/web_app/shadowbox_server.ts b/src/server_manager/web_app/shadowbox_server.ts index 667d2665..e54ec535 100644 --- a/src/server_manager/web_app/shadowbox_server.ts +++ b/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 { + console.info(`setPortForNewAcessKeys: ${newPort}`); + const requestOptions: RequestInit = { + method: 'PUT', + headers: new Headers({'Content-Type': 'application/json'}), + body: JSON.stringify({"port": newPort}) + }; + return this.apiRequest('server/port-for-new-access-keys', requestOptions).then(() => { + this.serverConfig.portForNewAccessKeys = newPort; + }); + } + private getServerConfig(): Promise { console.info('Retrieving server configuration'); return this.apiRequest('server'); diff --git a/src/shadowbox/package.json b/src/shadowbox/package.json index 5388c438..a975f166 100644 --- a/src/shadowbox/package.json +++ b/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", diff --git a/src/shadowbox/server/manager_service.ts b/src/shadowbox/server/manager_service.ts index e8ecafa7..ecd73873 100644 --- a/src/shadowbox/server/manager_service.ts +++ b/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(); } diff --git a/src/shadowbox/server/server_config.ts b/src/shadowbox/server/server_config.ts index 8b7bd983..12e8274f 100644 --- a/src/shadowbox/server/server_config.ts +++ b/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. diff --git a/src/shadowbox/tsconfig.json b/src/shadowbox/tsconfig.json index 02a1e030..e8f1b596 100644 --- a/src/shadowbox/tsconfig.json +++ b/src/shadowbox/tsconfig.json @@ -5,7 +5,8 @@ "noImplicitAny": false, "noImplicitThis": true, "module": "commonjs", - "rootDir": "." + "rootDir": ".", + "resolveJsonModule": true }, "include": [ "server/main.ts",