From f03daef191b494a72db276b1b17dee45501b6f21 Mon Sep 17 00:00:00 2001 From: trevj Date: Tue, 20 Nov 2018 12:03:31 -0500 Subject: [PATCH] validate manual server URLs before entry, leniently (#315) --- src/server_manager/package.json | 1 + src/server_manager/types/jsonic.d.ts | 22 +++++++++ .../outline-manual-server-entry.html | 49 +++++++++++++------ src/server_manager/web_app/app.ts | 47 +++++++++--------- .../web_app/management_urls.spec.ts | 49 +++++++++++++++++++ src/server_manager/web_app/management_urls.ts | 43 ++++++++++++++++ yarn.lock | 5 ++ 7 files changed, 176 insertions(+), 40 deletions(-) create mode 100644 src/server_manager/types/jsonic.d.ts create mode 100644 src/server_manager/web_app/management_urls.spec.ts create mode 100644 src/server_manager/web_app/management_urls.ts diff --git a/src/server_manager/package.json b/src/server_manager/package.json index 13e6f053..b3c229f2 100644 --- a/src/server_manager/package.json +++ b/src/server_manager/package.json @@ -16,6 +16,7 @@ "electron-updater": "^3.1.2", "eventemitter3": "^2.0.3", "express": "^4.16.3", + "jsonic": "^0.3.1", "node-forge": "^0.7.1", "request": "^2.87.0", "request-lite": "^2.40.1" diff --git a/src/server_manager/types/jsonic.d.ts b/src/server_manager/types/jsonic.d.ts new file mode 100644 index 00000000..aec87138 --- /dev/null +++ b/src/server_manager/types/jsonic.d.ts @@ -0,0 +1,22 @@ +// Copyright 2018 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Typings for: +// https://www.npmjs.com/package/jsonic + +declare module 'jsonic' { + function parse(s: string): Object; + namespace parse {} + export = parse; +} diff --git a/src/server_manager/ui_components/outline-manual-server-entry.html b/src/server_manager/ui_components/outline-manual-server-entry.html index efe8a9f0..54a901a4 100644 --- a/src/server_manager/ui_components/outline-manual-server-entry.html +++ b/src/server_manager/ui_components/outline-manual-server-entry.html @@ -19,6 +19,7 @@ + @@ -79,8 +80,12 @@ .drop-down:hover { cursor: pointer; } + /* This element surrounds the copy and paste elements. */ .section-content { margin: 24px 12px 0 48px; + padding: 24px; + background-color: #ECEFF1; + border-radius: 4px; } .section-content-instructions { margin: 24px 12px; @@ -98,23 +103,23 @@ margin-left: 4px; vertical-align: middle; } - .code { - background-color: #ECEFF1; - color: rgba(0,0,0,0.87); + paper-textarea { + --iron-autogrow-textarea: { + color: var(--dark-gray); + font-size: 14px; + font-family: RobotoMono-Regular, monospace; + line-height: 24px; + word-break: break-all; + } + } + /* TODO: Seems like a @mixin would help avoid duplication. */ + #command { + color: var(--dark-gray); font-size: 14px; font-family: RobotoMono-Regular, monospace; line-height: 24px; - border-radius: 4px; - border-bottom: 2px solid #263238; - padding: 24px; word-break: break-all; } - textarea { - width: 100%; - border: none; - outline: none; - box-sizing: border-box; - } iron-icon { display: inline-block; vertical-align: top; @@ -143,6 +148,9 @@ background-color: #1DE9B6; color: rgba(0,0,0,0.87); } + #doneButton[disabled] { + background-color: var(--border-color); + } #aws-logo { vertical-align: top; } @@ -234,7 +242,7 @@
Log into your server, and run this command.
-
sudo bash -c "$(wget -qO- https://raw.githubusercontent.com/Jigsaw-Code/outline-server/master/src/server_manager/install_scripts/install_server.sh)"
+
sudo bash -c "$(wget -qO- https://raw.githubusercontent.com/Jigsaw-Code/outline-server/master/src/server_manager/install_scripts/install_server.sh)"
@@ -245,12 +253,12 @@
Paste your installation output here.
- +
CANCEL - DONE + DONE
@@ -294,6 +302,10 @@ isGenericCloudProvider: { type: Boolean, computed: '_computeIsGenericCloudProvider(cloudProvider)' + }, + enableDoneButton: { + type: Boolean, + value: false, } }, showError: function(errorTitle, errorMessage) { @@ -314,7 +326,7 @@ } this.showConnection = true; this.fire('ManualServerEntered', { - userInputConfig: this.$.serverConfig.value + userInput: this.$.serverConfig.value }); }, cancelTapped: function() { @@ -357,6 +369,11 @@ _toggleDropDown: function(dropDown, icon) { dropDown.toggle(); icon.icon = dropDown.opened ? 'arrow-drop-up' : 'arrow-drop-down'; + }, + onServerConfigChanged: function () { + this.fire('ManualServerEdited', { + userInput: this.$.serverConfig.value + }); } }); diff --git a/src/server_manager/web_app/app.ts b/src/server_manager/web_app/app.ts index aceedf2e..be6625f8 100644 --- a/src/server_manager/web_app/app.ts +++ b/src/server_manager/web_app/app.ts @@ -21,6 +21,7 @@ import * as server from '../model/server'; import {TokenManager} from './digitalocean_oauth'; import * as digitalocean_server from './digitalocean_server'; +import {parseManualServerConfig} from './management_urls'; // tslint:disable-next-line:no-any type Polymer = HTMLElement&any; @@ -120,11 +121,23 @@ export class App { this.renameAccessKey(event.detail.accessKeyId, event.detail.newName, event.detail.entry); }); - appRoot.addEventListener('ManualServerEntered', (event: PolymerEvent) => { - const userInputConfig = - event.detail.userInputConfig.replace(/\s+/g, ''); // Remove whitespace + // The UI wants us to validate a server management URL. + // "Reply" by setting a field on the relevant template. + appRoot.addEventListener('ManualServerEdited', (event: PolymerEvent) => { + let isValid = true; + try { + parseManualServerConfig(event.detail.userInput); + } catch (e) { + isValid = false; + } const manualServerEntryEl = appRoot.getManualServerEntry(); - this.createManualServer(userInputConfig) + manualServerEntryEl.enableDoneButton = isValid; + }); + + appRoot.addEventListener('ManualServerEntered', (event: PolymerEvent) => { + const userInput = event.detail.userInput; + const manualServerEntryEl = appRoot.getManualServerEntry(); + this.createManualServer(userInput) .then(() => { // Clear fields on outline-manual-server-entry (e.g. dismiss the connecting popup). manualServerEntryEl.clear(); @@ -140,8 +153,8 @@ export class App { if (e.message) { errorMessage += `${e.message}\n`; } - if (userInputConfig) { - errorMessage += userInputConfig; + if (userInput) { + errorMessage += userInput; } appRoot.openManualInstallFeedback(errorMessage); } @@ -644,27 +657,13 @@ export class App { // 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(userInputConfig: string): Promise { - // Parse and validate user input. + public createManualServer(userInput: string): Promise { let serverConfig: server.ManualServerConfig; try { - // Remove anything before the first '{' and after the last '}', in case - // the user accidentally copied extra from the install script. - userInputConfig = userInputConfig.substr(userInputConfig.indexOf('{')); - userInputConfig = userInputConfig.substr(0, userInputConfig.lastIndexOf('}') + 1); - serverConfig = JSON.parse(userInputConfig); + serverConfig = parseManualServerConfig(userInput); } catch (e) { - console.error('Invalid server configuration: could not parse JSON.'); - return Promise.reject(new Error('')); - } - if (!serverConfig.apiUrl) { - const msg = 'Invalid server configuration: apiUrl is missing.'; - console.error(msg); - return Promise.reject(new Error(msg)); - } else if (!serverConfig.certSha256) { - const msg = 'Invalid server configuration: certSha256 is missing.'; - console.error(msg); - return Promise.reject(new Error(msg)); + // This shouldn't happen because the UI validates the URL before enabling the DONE button. + return Promise.reject(new Error(`could not parse server config: ${e.message}`)); } return this.manualServerRepository.addServer(serverConfig).then((manualServer) => { diff --git a/src/server_manager/web_app/management_urls.spec.ts b/src/server_manager/web_app/management_urls.spec.ts new file mode 100644 index 00000000..4948bdcd --- /dev/null +++ b/src/server_manager/web_app/management_urls.spec.ts @@ -0,0 +1,49 @@ +// Copyright 2018 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {parseManualServerConfig} from './management_urls'; + +describe('parseManualServerConfig', () => { + it('basic case', () => { + const result = + parseManualServerConfig('{"apiUrl":"http://abc.com/xyz", "certSha256":"1234567"}'); + expect(result.apiUrl).toEqual('http://abc.com/xyz'); + expect(result.certSha256).toEqual('1234567'); + }); + + it('ignores missing outer braces', () => { + const result = parseManualServerConfig('"apiUrl":"http://abc.com/xyz", "certSha256":"1234567"'); + expect(result.apiUrl).toEqual('http://abc.com/xyz'); + expect(result.certSha256).toEqual('1234567'); + }); + + it('ignores missing quotes on key names', () => { + const result = parseManualServerConfig('apiUrl:"http://abc.com/xyz", "certSha256":"1234567"'); + expect(result.apiUrl).toEqual('http://abc.com/xyz'); + expect(result.certSha256).toEqual('1234567'); + }); + + it('ignores missing quotes on values', () => { + const result = parseManualServerConfig('"apiUrl":http://abc.com/xyz, "certSha256":"1234567"'); + expect(result.apiUrl).toEqual('http://abc.com/xyz'); + expect(result.certSha256).toEqual('1234567'); + }); + + it('ignores content outside of braces', () => { + const result = parseManualServerConfig( + 'working... {"apiUrl":http://abc.com/xyz, "certSha256":"1234567"} ALL DONE'); + expect(result.apiUrl).toEqual('http://abc.com/xyz'); + expect(result.certSha256).toEqual('1234567'); + }); +}); \ No newline at end of file diff --git a/src/server_manager/web_app/management_urls.ts b/src/server_manager/web_app/management_urls.ts new file mode 100644 index 00000000..13b17986 --- /dev/null +++ b/src/server_manager/web_app/management_urls.ts @@ -0,0 +1,43 @@ +// Copyright 2018 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as jsonic from 'jsonic'; + +import {ManualServerConfig} from '../model/server'; + +// Parses a server management URL (generated by the server install script) leniently. +// TODO: ignore case of key names +// TODO: check value types +export function parseManualServerConfig(userInput: string): ManualServerConfig { + // Remove anything before and after the first and last braces, if found. + const indexOfFirstBrace = userInput.indexOf('{'); + if (indexOfFirstBrace >= 0) { + userInput = userInput.substring(indexOfFirstBrace); + } + const indexOfLastBrace = userInput.lastIndexOf('}'); + if (indexOfLastBrace >= 0) { + userInput = userInput.substring(0, indexOfLastBrace + 1); + } + + const config = jsonic(userInput) as ManualServerConfig; + + if (!config.apiUrl) { + throw new Error('no apiUrl field'); + } + if (!config.certSha256) { + throw new Error('no certSha256 field'); + } + + return config; +} \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 61f7a86e..f098c9d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3321,6 +3321,11 @@ jsonfile@^4.0.0: optionalDependencies: graceful-fs "^4.1.6" +jsonic@^0.3.1: + version "0.3.1" + resolved "https://registry.yarnpkg.com/jsonic/-/jsonic-0.3.1.tgz#da306185dc635b649ad934e3f6826188cad73bdb" + integrity sha512-5Md4EK3vPAMvP2sXY6M3/vQEPeX3LxEQBJuF979uypddXjsUlEoAI9/Nojh8tbw+YU5FjMoqSElO6oyjrAuprw== + jsonify@~0.0.0: version "0.0.0" resolved "https://registry.yarnpkg.com/jsonify/-/jsonify-0.0.0.tgz#2c74b6ee41d93ca51b7b5aaee8f503631d252a73"