From 99125dbb77f0a066721c77c091109b1d90a86f8f Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 7 May 2021 14:37:23 -0400 Subject: [PATCH 01/39] feat: Cleanup spawned session after process termination --- ui/desktop/electron-app/src/spawn-promise.js | 20 ++++++++- ui/desktop/electron-app/src/spawn-session.js | 45 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 ui/desktop/electron-app/src/spawn-session.js diff --git a/ui/desktop/electron-app/src/spawn-promise.js b/ui/desktop/electron-app/src/spawn-promise.js index dc8069791..d96b08953 100644 --- a/ui/desktop/electron-app/src/spawn-promise.js +++ b/ui/desktop/electron-app/src/spawn-promise.js @@ -1,5 +1,6 @@ const path = require('path'); const { spawn, spawnSync } = require('child_process'); +const spawnSession = require('./spawn-session'); const boundaryPath = path.resolve(__dirname, '..', 'cli', 'boundary'); @@ -45,14 +46,26 @@ module.exports = { childProcess.stdout.on('data', (data) => { outputStream += data.toString(); const jsonData = jsonify(outputStream); - if (jsonData) resolve(jsonData); + if (jsonData) { + if (jsonData.termination_reason) { + spawnSession.cancelProcess(childProcess.pid); + } else { + // Track only successfully launched child processes + spawnSession.add({ childProcess, data: jsonData }); + } + outputStream = ''; + resolve(jsonData); + } }); + // @todo Cancel spawned session childprocess on error? Does it get automatically cancelled? + // Test on connection limit errors especially childProcess.stderr.on('data', (data) => { errorStream += data.toString(); const jsonData = jsonify(errorStream); if (jsonData) { const error = jsonData.api_error || jsonData.error; + errorStream = ''; reject(new Error(error.message)); } }); @@ -60,7 +73,10 @@ module.exports = { }, /** - * + * Spawn child process and return output immediately. + * This function is intended for non-connection related tasks. + * @param {string} command + * @return {string} */ spawnSync(command) { const childProcess = spawnSync(boundaryPath, command); diff --git a/ui/desktop/electron-app/src/spawn-session.js b/ui/desktop/electron-app/src/spawn-session.js new file mode 100644 index 000000000..72292deeb --- /dev/null +++ b/ui/desktop/electron-app/src/spawn-session.js @@ -0,0 +1,45 @@ +const process = require('process'); + +const spawnedSessions = []; + +const purge = (process_id, index) => { + process.kill(process_id); + // Remove tracking after cancellation + delete spawnedSessions[index]; +}; + +module.exports = { + /** + * Track session spawned process and it's details. + * This function is intended to tracked launched local proxy. + * @param {object} params + */ + add: ({ childProcess, data }) => { + debugger; + spawnedSessions.push({ + data, + process_id: childProcess.pid, + }); + }, + /** + * Cancel spawned child process using session, if available. + * This function is intended to cancel launched local proxy. + * @param {string} session_id + */ + cancelSession: (session_id) => { + spawnedSessions.find((value, index) => { + const { process_id, data } = value; + if (data.session_id === session_id) purge(process_id, index); + }); + }, + /** + * Cancel spawned child process, if available. + * This function is intended to cancel launched local proxy. + * @param {string} process_id + */ + cancelProcess: (process_id) => { + spawnedSessions.find((value, index) => { + if (value.process_id === process_id) purge(process_id, index); + }); + }, +}; From 2f12cc91f2b41e8320e4f34fe29c3c4421dd16e8 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 7 May 2021 15:31:19 -0400 Subject: [PATCH 02/39] feat: Cancelling session should close associated spawned proc --- .../app/routes/scopes/scope/projects/sessions.js | 6 ++++++ .../scopes/scope/projects/targets/target/sessions.js | 10 ++++++++++ ui/desktop/electron-app/src/boundary-cli.js | 1 - ui/desktop/electron-app/src/handlers.js | 10 +++++++++- ui/desktop/electron-app/src/spawn-session.js | 3 +-- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/ui/desktop/app/routes/scopes/scope/projects/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/sessions.js index 272cf0672..2715a7aa3 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/sessions.js @@ -11,6 +11,7 @@ export default class ScopesScopeProjectsSessionsRoute extends Route { // =services + @service ipc; @service session; // =attributes @@ -83,5 +84,10 @@ export default class ScopesScopeProjectsSessionsRoute extends Route { @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); + try { + this.ipc.invoke('cancel', { session_id: session.id }); + } catch (e) { + //Ignore errors + } } } diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js index 4f0281e2f..2a849f3b9 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js @@ -1,8 +1,13 @@ import Route from '@ember/routing/route'; import { action } from '@ember/object'; +import { inject as service } from '@ember/service'; import { notifySuccess, notifyError } from 'core/decorators/notify'; export default class ScopesScopeProjectsTargetsTargetSessionsRoute extends Route { + // =services + + @service ipc; + // =actions /** @@ -14,5 +19,10 @@ export default class ScopesScopeProjectsTargetsTargetSessionsRoute extends Route @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); + try { + this.ipc.invoke('cancel', { session_id: session.id }); + } catch (e) { + //Ignore errors + } } } diff --git a/ui/desktop/electron-app/src/boundary-cli.js b/ui/desktop/electron-app/src/boundary-cli.js index 453a959df..eeb5050f8 100644 --- a/ui/desktop/electron-app/src/boundary-cli.js +++ b/ui/desktop/electron-app/src/boundary-cli.js @@ -53,6 +53,5 @@ module.exports = { ? `CLI Version: ${versionNumber}\nCLI Commit: ${gitRevision}` : `CLI Commit: ${gitRevision}`; return { gitRevision, versionNumber, formatted }; - return rawOutput; }, }; diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 4e102212a..04343c5ae 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -2,7 +2,8 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); const handle = require('./ipc-handler'); const boundaryCli = require('./boundary-cli'); -const origin = require('./origin.js'); +const origin = require('./origin'); +const spawnSession = require('./spawn-session'); /** * Returns the current runtime origin, which is used by the main thread to @@ -58,3 +59,10 @@ handle('cliExists', () => boundaryCli.exists()); handle('connect', ({ target_id, token, host_id }) => boundaryCli.connect(target_id, token, host_id) ); + +/** + * Cancel an established boundary session spawned process. + */ +handle('cancel', ({ session_id }) => { + spawnSession.cancelSession(session_id); +}); diff --git a/ui/desktop/electron-app/src/spawn-session.js b/ui/desktop/electron-app/src/spawn-session.js index 72292deeb..f632e4cea 100644 --- a/ui/desktop/electron-app/src/spawn-session.js +++ b/ui/desktop/electron-app/src/spawn-session.js @@ -5,7 +5,7 @@ const spawnedSessions = []; const purge = (process_id, index) => { process.kill(process_id); // Remove tracking after cancellation - delete spawnedSessions[index]; + spawnedSessions.splice(index, 1); }; module.exports = { @@ -15,7 +15,6 @@ module.exports = { * @param {object} params */ add: ({ childProcess, data }) => { - debugger; spawnedSessions.push({ data, process_id: childProcess.pid, From dfbb54a1eec9edc22205e8e6018eb5d7ba824f45 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 11 May 2021 16:38:26 -0400 Subject: [PATCH 03/39] refactor: Add SessionManager and Session classes to electron --- ui/desktop/electron-app/src/boundary-cli.js | 36 +------ ui/desktop/electron-app/src/handlers.js | 11 ++- ui/desktop/electron-app/src/session/index.js | 99 +++++++++++++++++++ .../electron-app/src/session/manager.js | 39 ++++++++ ui/desktop/electron-app/src/spawn-promise.js | 21 ++-- 5 files changed, 157 insertions(+), 49 deletions(-) create mode 100644 ui/desktop/electron-app/src/session/index.js create mode 100644 ui/desktop/electron-app/src/session/manager.js diff --git a/ui/desktop/electron-app/src/boundary-cli.js b/ui/desktop/electron-app/src/boundary-cli.js index eeb5050f8..9eeaa4f8d 100644 --- a/ui/desktop/electron-app/src/boundary-cli.js +++ b/ui/desktop/electron-app/src/boundary-cli.js @@ -1,46 +1,12 @@ const path = require('path'); -const { spawnAsyncJSONPromise, spawnSync } = require('./spawn-promise'); -const origin = require('./origin.js'); +const { spawnSync } = require('./spawn-promise'); const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); -/** - * Super paranoid shell quote/escape and validation. Input must be base62. - * @param {string} str - */ -const escapeAndValidateBase62 = (str) => { - const candidate = str.toString(); - if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; - throw new Error(` - Could not invoke command: - input contained unsafe characters. - `); -}; - module.exports = { // Check boundary cli existence exists: () => Boolean(cliPath()), - // Initiate connection and return output - connect: (target_id, token, host_id) => { - const addr = origin.origin; - const sanitized = { - target_id: escapeAndValidateBase62(target_id), - token: escapeAndValidateBase62(token), - }; - if (host_id) sanitized.host_id = escapeAndValidateBase62(host_id); - const command = [ - 'connect', - `-target-id=${sanitized.target_id}`, - `-token=${sanitized.token}`, - `-addr=${addr}`, - '-format=json', - ]; - - if (host_id) command.push(`-host-id=${sanitized.host_id}`); - return spawnAsyncJSONPromise(command); - }, - // Returns JSON-formatted version information from the CLI version: () => { const command = ['-v']; diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 04343c5ae..7127c1f12 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -3,7 +3,7 @@ const isDev = require('electron-is-dev'); const handle = require('./ipc-handler'); const boundaryCli = require('./boundary-cli'); const origin = require('./origin'); -const spawnSession = require('./spawn-session'); +const sessionManager = require('./session/manager'); /** * Returns the current runtime origin, which is used by the main thread to @@ -57,12 +57,13 @@ handle('cliExists', () => boundaryCli.exists()); * Establishes a boundary session and returns session details. */ handle('connect', ({ target_id, token, host_id }) => - boundaryCli.connect(target_id, token, host_id) + sessionManager.start(origin.origin, target_id, token, host_id) ); /** * Cancel an established boundary session spawned process. */ -handle('cancel', ({ session_id }) => { - spawnSession.cancelSession(session_id); -}); +handle('cancel', ({ session_id }) => + // spawnSession.cancelSession(session_id); + sessionManager.stop(session_id) +); diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js new file mode 100644 index 000000000..3d5563f1d --- /dev/null +++ b/ui/desktop/electron-app/src/session/index.js @@ -0,0 +1,99 @@ +const { spawnAsyncJSONPromise } = require('../spawn-promise'); + +// FIXME create structure 'util' folder for spawn-promise + +/** + * Super paranoid shell quote/escape and validation. Input must be base62. + * @param {string} str + */ +const escapeAndValidateBase62 = (str) => { + const candidate = str.toString(); + if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; + throw new Error(` + Could not invoke command: + input contained unsafe characters. + `); +}; + +class Session { + #id; + #pid; + #addr; + #token; + #hostId; + #targetId; + #proxyDetails; + + /** + * Initialize a session to a controller address + * using target details. + * @param {string} addr + * @param {string} targetId + * @param {string} token + * @param {string} hostId + */ + constructor(addr, targetId, token, hostId) { + this.#addr = addr; + this.#targetId = targetId; + this.#token = token; + this.#hostId = hostId; + } + + /** + * @return {boolean} + */ + get isActive() { + return Boolean(this.#pid); + } + + /** + * Returns session proxy details. + * Only available after successful start of a session. + * @return {object} + */ + get proxyDetails() { + return this.#proxyDetails; + } + + // Spawn process + // Track spawned process this.pid = pid; + /** + * Using cli, initialize a session to a target. + * Tracks local proxy details if successful. + */ + start() { + const command = this.cliCommand(); + return spawnAsyncJSONPromise(command).then((spawnedSession) => { + this.#id = this.#proxyDetails.session_id; + this.#pid = spawnedSession.childProcess.pid; + this.#proxyDetails = spawnedSession.response; + }); + } + + /** + * Generate cli command for session. + * @returns {[]} + */ + cliCommand() { + const sanitized = { + target_id: escapeAndValidateBase62(this.#targetId), + token: escapeAndValidateBase62(this.#token), + }; + + const command = [ + 'connect', + `-target-id=${sanitized.target_id}`, + `-token=${sanitized.token}`, + `-addr=${this.#addr}`, + '-format=json', + ]; + + if (this.#hostId) { + sanitized.host_id = escapeAndValidateBase62(this.#hostId); + command.push(`-host-id=${sanitized.host_id}`); + } + return command; + } +} + +module.exports = Session; diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js new file mode 100644 index 000000000..60be77204 --- /dev/null +++ b/ui/desktop/electron-app/src/session/manager.js @@ -0,0 +1,39 @@ +const Session = require('./index.js'); + +class SessionManager { + #sessions; + + constructor() { + this.#sessions = []; + } + + /** + * Start a session and track it. + * Returns session proxy details if successfully started. + * Only successfully started sessions will be tracked. + * @param {string} addr + * @param {string} target_id + * @param {string} token + * @param {string} host_id + */ + start(addr, target_id, token, host_id) { + const session = new Session(addr, target_id, token, host_id); + return session.start().then(() => { + this.#sessions.push(session); + return session.proxyDetails; + }); + } + + stopById() { + } + + stopAll() { + } + + get sessions() { + return this.#sessions; + } +} + +const sessionManager = new SessionManager(); +module.exports = sessionManager; diff --git a/ui/desktop/electron-app/src/spawn-promise.js b/ui/desktop/electron-app/src/spawn-promise.js index d96b08953..10467ffc6 100644 --- a/ui/desktop/electron-app/src/spawn-promise.js +++ b/ui/desktop/electron-app/src/spawn-promise.js @@ -47,25 +47,28 @@ module.exports = { outputStream += data.toString(); const jsonData = jsonify(outputStream); if (jsonData) { - if (jsonData.termination_reason) { - spawnSession.cancelProcess(childProcess.pid); - } else { - // Track only successfully launched child processes - spawnSession.add({ childProcess, data: jsonData }); - } - outputStream = ''; - resolve(jsonData); + // if (jsonData.termination_reason) { + // spawnSession.cancelProcess(childProcess.pid); + // } else { + // // Track only successfully launched child processes + // spawnSession.add({ childProcess, data: jsonData }); + // } + // FIXME + // outputStream = ''; + resolve({ childProcess, response: jsonData }); } }); // @todo Cancel spawned session childprocess on error? Does it get automatically cancelled? + // @todo on error - should childprocess be closed? // Test on connection limit errors especially childProcess.stderr.on('data', (data) => { errorStream += data.toString(); const jsonData = jsonify(errorStream); if (jsonData) { const error = jsonData.api_error || jsonData.error; - errorStream = ''; + // FIXME + // errorStream = ''; reject(new Error(error.message)); } }); From 4ace04de7ac099f1c8804ce2fe26d8db5a3b844a Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 5 May 2021 15:42:12 -0400 Subject: [PATCH 04/39] feat: Add condensed style to Rose::Table --- .../addon/components/rose/table/index.hbs | 2 +- .../addon/components/rose/table/row/index.hbs | 2 +- .../styles/rose/components/table/_table.scss | 15 ++ .../rose/tests/dummy/app/templates/index.hbs | 177 +++++++++--------- 4 files changed, 107 insertions(+), 89 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.hbs b/addons/rose/addon/components/rose/table/index.hbs index 655e7dbd4..643a14cfa 100644 --- a/addons/rose/addon/components/rose/table/index.hbs +++ b/addons/rose/addon/components/rose/table/index.hbs @@ -1,4 +1,4 @@ - +
{{#if @caption}} {{/if}} diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index e5bad550e..5267351c8 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -1,4 +1,4 @@ - + {{yield (hash cell=(component 'rose/anonymous' tagName='td' diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 0c581393e..7460d2f4d 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -38,4 +38,19 @@ [class*='rose-button'] { margin-bottom: 0; } + + &.rose-table-condensed { + .rose-table-cell { + @include type.type(s); + $lineHeight: 36 / 14; + line-height: $lineHeight; + padding: (sizing.rems(s) / 2) sizing.rems(s); + } + } + + .rose-table-row-visually-hidden { + position: absolute; + top: -9999rem; + left: -9999rem; + } } diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index 8bb8b116d..3c7b27e5c 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -1,93 +1,96 @@ - - +

Condensed Table

- - - Menu item - Menu item - danger - - Button menu item - + + + + Address + Host Set + Name + Description + + + + + 1.1.1.1 + host-set-1, host-set-2 + optional name + description + + + + + Footer + + + - Section - Section - +

Key/Value

+ + + Key + Value + + + Key + Value + + + Key + Value + - - - Menu item - Menu item - danger - Button - default - - Button - danger - - -
+ {{!-- In actual use, form fields should be yielded from an outer form component --}} + + + + Key + + + + + + Add + + + - - Menu item - Menu item - danger - + + + + Key + + + + + + Remove + + + - Button menu item - Button - danger - + - - Menu item - - - - - Menu item - Button menu item - - - - - -
    -
  • - - - -
  • -
  • - - - -
  • -
  • - - - -
  • -
-
-
-
- - - - <:header> -

Authentication Pending

- - <:body> -

Complete authentication in the newly-opened window.

-

Don't see the new window? Try again.

-

Cancel

- -
+

Table

+ + + + Address + Host Set + Name + Description + + + + + 1.1.1.1 + host-set-1, host-set-2 + optional name + description + + + + + Footer + + + \ No newline at end of file From 8e1e2b6b3628833e60a7c4678a8df7cef2272d08 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 5 May 2021 17:21:49 -0400 Subject: [PATCH 05/39] feat: Add Rose::Table::Row::Cell for cells to expand/shrink --- .../components/rose/table/row/cell/index.hbs | 1 + .../components/rose/table/row/cell/index.js | 17 +++ .../addon/components/rose/table/row/index.hbs | 13 ++- .../styles/rose/components/table/_table.scss | 7 ++ .../app/components/rose/table/row/cell.js | 1 + .../rose/tests/dummy/app/templates/index.hbs | 102 +++++++++++++++++- .../components/rose/table/row/cell-test.js | 26 +++++ 7 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 addons/rose/addon/components/rose/table/row/cell/index.hbs create mode 100644 addons/rose/addon/components/rose/table/row/cell/index.js create mode 100644 addons/rose/app/components/rose/table/row/cell.js create mode 100644 addons/rose/tests/integration/components/rose/table/row/cell-test.js diff --git a/addons/rose/addon/components/rose/table/row/cell/index.hbs b/addons/rose/addon/components/rose/table/row/cell/index.hbs new file mode 100644 index 000000000..fb5c4b157 --- /dev/null +++ b/addons/rose/addon/components/rose/table/row/cell/index.hbs @@ -0,0 +1 @@ +{{yield}} \ No newline at end of file diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js new file mode 100644 index 000000000..4a7572db7 --- /dev/null +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -0,0 +1,17 @@ +import Component from '@ember/component'; +import layout from './index'; +import { computed } from '@ember/object'; + +export default Component.extend({ + layout, + classNameBindings: [ + 'cellStyle', + 'cellSizeStyle', + ], + cellStyle: computed(function () { + return this.tagName.match('th') ? `rose-table-header-cell` : 'rose-table-cell'; + }), + cellSizeStyle: computed(function () { + return this.style ? `rose-table-cell-${this.style}` : ''; + }), +}); diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 5267351c8..52d1f3ba9 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -1,12 +1,21 @@ + {{yield (hash + cell=(component 'rose/table/row/cell' tagName='td') + headerCell=(component 'rose/table/row/cell' tagName='th') + )}} + + + +{{!-- {{yield (hash cell=(component 'rose/anonymous' tagName='td' class='rose-table-cell' + classNames='{{style}} {{@style}}' ) headerCell=(component 'rose/anonymous' tagName='th' - class='rose-table-header-cell' + class='rose-table-header-cell @style {{if @style (concat "rose-table-cell-" @style)}}' ) )}} - + --}} diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 7460d2f4d..0e3305a9a 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -13,6 +13,7 @@ .rose-table-cell, .rose-table-header-cell { vertical-align: middle; + white-space: nowrap; } .rose-table-header-cell { @@ -35,6 +36,12 @@ } } + // Grow to full available width + .rose-table-cell-expand { + width: 100%; + border: 1px solid red; + } + [class*='rose-button'] { margin-bottom: 0; } diff --git a/addons/rose/app/components/rose/table/row/cell.js b/addons/rose/app/components/rose/table/row/cell.js new file mode 100644 index 000000000..a6ca12293 --- /dev/null +++ b/addons/rose/app/components/rose/table/row/cell.js @@ -0,0 +1 @@ +export { default } from 'rose/components/rose/table/row/cell'; \ No newline at end of file diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index 3c7b27e5c..27c3bbeff 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -1,9 +1,8 @@

Condensed Table

- - - Address + + Address Host Set Name Description @@ -24,6 +23,103 @@ +

Condensed Table With Expanded Cells

+ + + + Address + Host Set + Name + Description + + + + + 1.1.1.1 + host-set-1, host-set-2 + + Remove + + description + + + + + Footer + + + + +

Condensed Table Key/Value Lookalike

+ + + + Key + Value + + + + + Key + Value + + + Key + Value + + + Key + Value + + + Key + + + {{!-- --}} + + Remove + {{!-- --}} + + + + + {{!-- Remove --}} + + + + + + + Footer + + + + +

Condensed Table With Visually Hidden Header & Footer

+ + + + Address + Host Set + Name + Description + + + + + 1.1.1.1 + host-set-1, host-set-2 + optional name + description + + + + + Footer + + + +

Key/Value

diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js new file mode 100644 index 000000000..9279a81df --- /dev/null +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -0,0 +1,26 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +module('Integration | Component | rose/table/row/cell', function(hooks) { + setupRenderingTest(hooks); + + test('it renders', async function(assert) { + // Set any properties with this.set('myProperty', 'value'); + // Handle any actions with this.set('myAction', function(val) { ... }); + + await render(hbs``); + + assert.equal(this.element.textContent.trim(), ''); + + // Template block usage: + await render(hbs` + + template block text + + `); + + assert.equal(this.element.textContent.trim(), 'template block text'); + }); +}); From 3425dc89be53bc667161d3417bb8515c7b652d34 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 10:00:16 -0400 Subject: [PATCH 06/39] chore: Setup Rose::Table cell alignment config --- .../styles/rose/components/table/_table.scss | 6 +++- .../rose/tests/dummy/app/templates/index.hbs | 34 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 0e3305a9a..6fc53e78e 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -39,7 +39,11 @@ // Grow to full available width .rose-table-cell-expand { width: 100%; - border: 1px solid red; + + & > * { + vertical-align: inherit; + margin: 0; + } } [class*='rose-button'] { diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index 27c3bbeff..22d4f9e81 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -50,6 +50,31 @@ +

Condensed Table With Right and Center aligned cells

+ + + + Address + Host Set + Name + Description + + + + + 1.1.1.1 + host-set-1, host-set-2 + optional name + description + + + + + Footer + + + +

Condensed Table Key/Value Lookalike

@@ -75,15 +100,8 @@ Key - {{!-- --}} Remove - {{!-- --}} - - - - - {{!-- Remove --}} @@ -135,7 +153,6 @@ Value
- {{!-- In actual use, form fields should be yielded from an outer form component --}} @@ -163,7 +180,6 @@ -

Table

From cf40b53fd05042a456ac623fded54d1d175df792 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 10:14:11 -0400 Subject: [PATCH 07/39] chore: Format newly added files in rose --- .../rose/addon/components/rose/table/row/cell/index.js | 9 ++++----- .../rose/addon/styles/rose/components/table/_table.scss | 2 +- addons/rose/app/components/rose/table/row/cell.js | 2 +- .../integration/components/rose/table/row/cell-test.js | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js index 4a7572db7..0fc901f10 100644 --- a/addons/rose/addon/components/rose/table/row/cell/index.js +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -4,12 +4,11 @@ import { computed } from '@ember/object'; export default Component.extend({ layout, - classNameBindings: [ - 'cellStyle', - 'cellSizeStyle', - ], + classNameBindings: ['cellStyle', 'cellSizeStyle'], cellStyle: computed(function () { - return this.tagName.match('th') ? `rose-table-header-cell` : 'rose-table-cell'; + return this.tagName.match('th') + ? `rose-table-header-cell` + : 'rose-table-cell'; }), cellSizeStyle: computed(function () { return this.style ? `rose-table-cell-${this.style}` : ''; diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 6fc53e78e..997ef944a 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -43,7 +43,7 @@ & > * { vertical-align: inherit; margin: 0; - } + } } [class*='rose-button'] { diff --git a/addons/rose/app/components/rose/table/row/cell.js b/addons/rose/app/components/rose/table/row/cell.js index a6ca12293..50e7502b4 100644 --- a/addons/rose/app/components/rose/table/row/cell.js +++ b/addons/rose/app/components/rose/table/row/cell.js @@ -1 +1 @@ -export { default } from 'rose/components/rose/table/row/cell'; \ No newline at end of file +export { default } from 'rose/components/rose/table/row/cell'; diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js index 9279a81df..c58c9096c 100644 --- a/addons/rose/tests/integration/components/rose/table/row/cell-test.js +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -3,10 +3,10 @@ import { setupRenderingTest } from 'ember-qunit'; import { render } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; -module('Integration | Component | rose/table/row/cell', function(hooks) { +module('Integration | Component | rose/table/row/cell', function (hooks) { setupRenderingTest(hooks); - test('it renders', async function(assert) { + test('it renders', async function (assert) { // Set any properties with this.set('myProperty', 'value'); // Handle any actions with this.set('myAction', function(val) { ... }); From a909ba67df6f576c4a44ab48b25c47aace2f31d3 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 14:27:46 -0400 Subject: [PATCH 08/39] chore: Minor cleanup in Rose::Table::Row template --- addons/rose/addon/components/rose/table/row/index.hbs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 52d1f3ba9..5afc830e2 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -11,11 +11,10 @@ cell=(component 'rose/anonymous' tagName='td' class='rose-table-cell' - classNames='{{style}} {{@style}}' ) headerCell=(component 'rose/anonymous' tagName='th' - class='rose-table-header-cell @style {{if @style (concat "rose-table-cell-" @style)}}' + class='rose-table-header-cell' ) )}} --}} From 6b93032a78a3c6ee74e28901a0f0169e7455a5d0 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 16:57:49 -0400 Subject: [PATCH 09/39] style: Update Rose::Table storybook --- .../components/rose/table/index.stories.mdx | 107 +++++++++++++++++- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index a720bd877..a6a840401 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -1,11 +1,14 @@ import { Meta, Story, Preview } from '@storybook/addon-docs/blocks'; import { hbs } from 'ember-cli-htmlbars'; +import { boolean, select } from '@storybook/addon-knobs'; # Table -A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. +A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. Rows can be visually hidden, cells can be expanded to fit full width, and center/right cell alignment is supported. + +Cell expansion style should be applied to only one cell in a row. ```htmlbars @@ -24,6 +27,18 @@ A table component to display data in structure of rows and columns. It supports optional name description + + 1.1.1.1 + host-set-2 + optional name + description + + + 1.1.1.1 + host-set-1 + optional name + description + @@ -36,11 +51,11 @@ A table component to display data in structure of rows and columns. It supports {{ template: hbs` - + - + Address - Host Set + Host Set Name Description @@ -48,7 +63,19 @@ A table component to display data in structure of rows and columns. It supports 1.1.1.1 - host-set-1, host-set-2 + host-set-1, host-set-2 + optional name + description + + + 1.1.1.1 + host-set-2 + optional name + description + + + 1.1.1.1 + host-set-1 optional name description @@ -60,6 +87,74 @@ A table component to display data in structure of rows and columns. It supports `, - context: {} + context: { + visuallyHideHeaderRow: boolean('visuallyHideHeaderRow', false), + cellStyle: select( + 'cellStyle', + { + default: '', + 'expand': 'expand' + } + ), + style: select( + 'style', + { + 'default': '', + 'condensed': 'condensed' + } + ) + }, + }} + + + + {{ + template: hbs` + + + + Key + Value + + + + + Key + Value + + + Key + Value + + + Key + Value + + + Key + + + + Add + + + + + Key + + + + Remove + + + + + + + Footer + + + + `, }} From 84051e4b7242a62e8defeddec4178b5d19993b4a Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 17:06:01 -0400 Subject: [PATCH 10/39] chore: Refactor Rose::Table::Row visually-hidden to hidden --- .../components/rose/table/index.stories.mdx | 22 +++++++++---------- .../addon/components/rose/table/row/index.hbs | 2 +- .../rose/tests/dummy/app/templates/index.hbs | 8 +++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index a6a840401..d2f56a5b8 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -10,12 +10,12 @@ A table component to display data in structure of rows and columns. It supports Cell expansion style should be applied to only one cell in a row. -```htmlbars - +```html + - + Address - Host Set + Host Set Name Description @@ -23,19 +23,19 @@ Cell expansion style should be applied to only one cell in a row. 1.1.1.1 - host-set-1, host-set-2 + host-set-1, host-set-2 optional name description 1.1.1.1 - host-set-2 + host-set-2 optional name description 1.1.1.1 - host-set-1 + host-set-1 optional name description @@ -53,7 +53,7 @@ Cell expansion style should be applied to only one cell in a row. template: hbs` - + Address Host Set Name @@ -88,7 +88,7 @@ Cell expansion style should be applied to only one cell in a row. `, context: { - visuallyHideHeaderRow: boolean('visuallyHideHeaderRow', false), + hideHeaderRow: boolean('hideHeaderRow', false), cellStyle: select( 'cellStyle', { @@ -112,7 +112,7 @@ Cell expansion style should be applied to only one cell in a row. template: hbs` - + Key Value @@ -150,7 +150,7 @@ Cell expansion style should be applied to only one cell in a row. - + Footer diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 5afc830e2..7431132ea 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -1,4 +1,4 @@ - + {{yield (hash cell=(component 'rose/table/row/cell' tagName='td') headerCell=(component 'rose/table/row/cell' tagName='th') diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index 22d4f9e81..eabd793cb 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -43,7 +43,7 @@ description - + Footer @@ -106,7 +106,7 @@ - + Footer @@ -116,7 +116,7 @@

Condensed Table With Visually Hidden Header & Footer

- + Address Host Set Name @@ -132,7 +132,7 @@ - + Footer From ec9d953bad2a65bd21828724b3573719d5e5f73a Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 6 May 2021 17:36:30 -0400 Subject: [PATCH 11/39] test: Add Rose::Table::Row::Cell tests --- .../components/rose/table/row/cell/index.js | 7 ++-- .../addon/components/rose/table/row/index.hbs | 2 +- .../integration/components/rose/table-test.js | 5 +++ .../components/rose/table/row-test.js | 5 +++ .../components/rose/table/row/cell-test.js | 40 ++++++++++++------- 5 files changed, 41 insertions(+), 18 deletions(-) diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js index 0fc901f10..01bcb5fd7 100644 --- a/addons/rose/addon/components/rose/table/row/cell/index.js +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -4,13 +4,14 @@ import { computed } from '@ember/object'; export default Component.extend({ layout, - classNameBindings: ['cellStyle', 'cellSizeStyle'], - cellStyle: computed(function () { + tagName: 'td', + classNameBindings: ['cellTypeStyle', 'cellStyle'], + cellTypeStyle: computed(function () { return this.tagName.match('th') ? `rose-table-header-cell` : 'rose-table-cell'; }), - cellSizeStyle: computed(function () { + cellStyle: computed(function () { return this.style ? `rose-table-cell-${this.style}` : ''; }), }); diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 7431132ea..5183c0746 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -1,6 +1,6 @@
{{yield (hash - cell=(component 'rose/table/row/cell' tagName='td') + cell=(component 'rose/table/row/cell') headerCell=(component 'rose/table/row/cell' tagName='th') )}} diff --git a/addons/rose/tests/integration/components/rose/table-test.js b/addons/rose/tests/integration/components/rose/table-test.js index 06f2a5b8c..aa78b58eb 100644 --- a/addons/rose/tests/integration/components/rose/table-test.js +++ b/addons/rose/tests/integration/components/rose/table-test.js @@ -43,4 +43,9 @@ module('Integration | Component | rose/table', function (hooks) { `); assert.ok(find('tfoot')); }); + + test('it supports a style class', async function (assert) { + await render(hbs``); + assert.ok(find('.rose-table-condensed')); + }); }); diff --git a/addons/rose/tests/integration/components/rose/table/row-test.js b/addons/rose/tests/integration/components/rose/table/row-test.js index 94c57cd08..7e0774504 100644 --- a/addons/rose/tests/integration/components/rose/table/row-test.js +++ b/addons/rose/tests/integration/components/rose/table/row-test.js @@ -32,4 +32,9 @@ module('Integration | Component | rose/table/row', function (hooks) { assert.ok(find('th')); assert.ok(find('.rose-table-header-cell')); }); + + test('it can be hidden', async function (assert) { + await render(hbs``); + assert.ok(find('.rose-table-row-visually-hidden')); + }); }); diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js index c58c9096c..48928d3d6 100644 --- a/addons/rose/tests/integration/components/rose/table/row/cell-test.js +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -1,26 +1,38 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; -import { hbs } from 'ember-cli-htmlbars'; +import { render, find } from '@ember/test-helpers'; +import hbs from 'htmlbars-inline-precompile'; module('Integration | Component | rose/table/row/cell', function (hooks) { setupRenderingTest(hooks); test('it renders', async function (assert) { - // Set any properties with this.set('myProperty', 'value'); - // Handle any actions with this.set('myAction', function(val) { ... }); + assert.expect(2); + await render(hbs` + content + `); + assert.ok(find('td')); + assert.equal(find('.rose-table-cell').textContent.trim(), 'content'); + }); - await render(hbs``); + test('it renders with attributes', async function (assert) { + assert.expect(1); + await render(hbs``); + assert.ok(find('#cell')); + }); - assert.equal(this.element.textContent.trim(), ''); + test('it renders as header cell', async function (assert) { + assert.expect(4); + await render(hbs``); + assert.ok(find('th')); + assert.ok(find('.rose-table-header-cell')); + assert.notOk(find('td')); + assert.notOk(find('.rose-table-cell')); + }); - // Template block usage: - await render(hbs` - - template block text - - `); - - assert.equal(this.element.textContent.trim(), 'template block text'); + test('it adds a style class', async function (assert) { + assert.expect(1); + await render(hbs``); + assert.ok(find('.rose-table-cell-expand')); }); }); From 8feeaa47dc5874d7c6753d8fc919856d1c97328f Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Tue, 11 May 2021 15:06:04 -0700 Subject: [PATCH 12/39] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Refactor=20rose=20?= =?UTF-8?q?table=20component?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We refactor the component adding a condensed variant of the table, refactor the hidden feature. We also provide different alignments for a cell. Updated tests and storybook page --- .../components/rose/table/index.stories.mdx | 32 ++++++++++--------- .../components/rose/table/row/cell/index.js | 11 ++++++- .../addon/components/rose/table/row/index.hbs | 14 -------- .../styles/rose/components/table/_table.scss | 4 +++ .../rose/tests/dummy/app/templates/index.hbs | 14 ++++---- .../components/rose/table/row/cell-test.js | 32 +++++++++++++++++++ 6 files changed, 70 insertions(+), 37 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index d2f56a5b8..f11560e41 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -30,8 +30,8 @@ Cell expansion style should be applied to only one cell in a row. 1.1.1.1 host-set-2 - optional name - description + optional name + description 1.1.1.1 @@ -54,10 +54,10 @@ Cell expansion style should be applied to only one cell in a row. - Address - Host Set - Name - Description + Address + Host Set + Name + Description @@ -68,16 +68,16 @@ Cell expansion style should be applied to only one cell in a row. description - 1.1.1.1 - host-set-2 - optional name - description + 1.1.1.1 + host-set-2 + optional name + description - 1.1.1.1 - host-set-1 - optional name - description + 1.1.1.1 + host-set-1 + optional name + description @@ -102,7 +102,9 @@ Cell expansion style should be applied to only one cell in a row. 'default': '', 'condensed': 'condensed' } - ) + ), + cellAlignmentCenter: boolean('cellAlignmentCenter', false), + cellAlignmentRight: boolean('cellAlignmentRight', false), }, }} diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js index 01bcb5fd7..09f5f53dc 100644 --- a/addons/rose/addon/components/rose/table/row/cell/index.js +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -5,7 +5,7 @@ import { computed } from '@ember/object'; export default Component.extend({ layout, tagName: 'td', - classNameBindings: ['cellTypeStyle', 'cellStyle'], + classNameBindings: ['cellTypeStyle', 'cellStyle', 'cellAlignmentStyle'], cellTypeStyle: computed(function () { return this.tagName.match('th') ? `rose-table-header-cell` @@ -14,4 +14,13 @@ export default Component.extend({ cellStyle: computed(function () { return this.style ? `rose-table-cell-${this.style}` : ''; }), + cellAlignmentStyle: computed(function () { + if (this.alignRight) { + return 'align-right'; + } + + if (this.alignCenter) { + return 'align-center'; + } + }) }); diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 5183c0746..4b511d869 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -4,17 +4,3 @@ headerCell=(component 'rose/table/row/cell' tagName='th') )}} - - -{{!-- - {{yield (hash - cell=(component 'rose/anonymous' - tagName='td' - class='rose-table-cell' - ) - headerCell=(component 'rose/anonymous' - tagName='th' - class='rose-table-header-cell' - ) - )}} - --}} diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 997ef944a..4f6ea31d8 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -14,6 +14,9 @@ .rose-table-header-cell { vertical-align: middle; white-space: nowrap; + + &.align-right { text-align: right; } + &.align-center { text-align: center; } } .rose-table-header-cell { @@ -57,6 +60,7 @@ line-height: $lineHeight; padding: (sizing.rems(s) / 2) sizing.rems(s); } + } .rose-table-row-visually-hidden { diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index eabd793cb..17cd6861b 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -43,7 +43,7 @@ description - + Footer @@ -63,9 +63,9 @@ 1.1.1.1 - host-set-1, host-set-2 - optional name - description + host-set-1, host-set-2 + optional name + description @@ -106,7 +106,7 @@ - + Footer @@ -116,7 +116,7 @@

Condensed Table With Visually Hidden Header & Footer

- + Address Host Set Name @@ -132,7 +132,7 @@ - + Footer diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js index 48928d3d6..dcc93e33c 100644 --- a/addons/rose/tests/integration/components/rose/table/row/cell-test.js +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -35,4 +35,36 @@ module('Integration | Component | rose/table/row/cell', function (hooks) { await render(hbs``); assert.ok(find('.rose-table-cell-expand')); }); + + test('it renders with align center in a header cell', async function (assert) { + assert.expect(2); + await render( + hbs`` + ); + assert.ok(find('.rose-table-header-cell')); + assert.ok(find('.align-center')); + }); + + test('it renders with align center in a body', async function (assert) { + assert.expect(2); + await render(hbs``); + assert.ok(find('.rose-table-cell')); + assert.ok(find('.align-center')); + }); + + test('it renders with align right in a header cell', async function (assert) { + assert.expect(2); + await render( + hbs`` + ); + assert.ok(find('.rose-table-header-cell')); + assert.ok(find('.align-right')); + }); + + test('it renders with align right', async function (assert) { + assert.expect(2); + await render(hbs``); + assert.ok(find('.rose-table-cell')); + assert.ok(find('.align-right')); + }); }); From 2be18bcf6f9a5f335d88ee684d1680c093f62b22 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Tue, 11 May 2021 18:09:38 -0400 Subject: [PATCH 13/39] chore: Cleanup --- .../routes/scopes/scope/projects/sessions.js | 6 +--- .../scope/projects/targets/target/sessions.js | 6 +--- ui/desktop/electron-app/src/handlers.js | 5 +-- ui/desktop/electron-app/src/session/index.js | 31 +++++++++++++++---- .../electron-app/src/session/manager.js | 16 ++++++++-- .../acceptance/projects/sessions-test.js | 17 ++++++++++ .../projects/targets/sessions-test.js | 12 +++++++ 7 files changed, 71 insertions(+), 22 deletions(-) diff --git a/ui/desktop/app/routes/scopes/scope/projects/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/sessions.js index 2715a7aa3..908636f1a 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/sessions.js @@ -84,10 +84,6 @@ export default class ScopesScopeProjectsSessionsRoute extends Route { @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - try { - this.ipc.invoke('cancel', { session_id: session.id }); - } catch (e) { - //Ignore errors - } + await this.ipc.invoke('cancel', { session_id: session.id }); } } diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js index 2a849f3b9..2263aa448 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js @@ -19,10 +19,6 @@ export default class ScopesScopeProjectsTargetsTargetSessionsRoute extends Route @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - try { - this.ipc.invoke('cancel', { session_id: session.id }); - } catch (e) { - //Ignore errors - } + await this.ipc.invoke('cancel', { session_id: session.id }); } } diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/handlers.js index 7127c1f12..3aee556f9 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/handlers.js @@ -63,7 +63,4 @@ handle('connect', ({ target_id, token, host_id }) => /** * Cancel an established boundary session spawned process. */ -handle('cancel', ({ session_id }) => - // spawnSession.cancelSession(session_id); - sessionManager.stop(session_id) -); +handle('cancel', ({ session_id }) => sessionManager.stopById(session_id)); diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index 3d5563f1d..cfc760ba7 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -1,4 +1,4 @@ -const { spawnAsyncJSONPromise } = require('../spawn-promise'); +const { spawnAsyncJSONPromise } = require('../spawn-promise.js'); // FIXME create structure 'util' folder for spawn-promise @@ -21,6 +21,7 @@ class Session { #addr; #token; #hostId; + #process; #targetId; #proxyDetails; @@ -39,11 +40,19 @@ class Session { this.#hostId = hostId; } + /** + * Session id + * @return {string} + */ + get id() { + return this.#id; + } + /** * @return {boolean} */ get isActive() { - return Boolean(this.#pid); + return this.#process.connected; } /** @@ -55,8 +64,6 @@ class Session { return this.#proxyDetails; } - // Spawn process - // Track spawned process this.pid = pid; /** * Using cli, initialize a session to a target. * Tracks local proxy details if successful. @@ -64,9 +71,21 @@ class Session { start() { const command = this.cliCommand(); return spawnAsyncJSONPromise(command).then((spawnedSession) => { - this.#id = this.#proxyDetails.session_id; - this.#pid = spawnedSession.childProcess.pid; + this.#process = spawnedSession.childProcess; this.#proxyDetails = spawnedSession.response; + this.#process = spawnedSession.childProcess; + this.#id = this.#proxyDetails.session_id; + }); + } + + /** + * Stop proxy used by session. + */ + stop() { + return new Promise((resolve, reject) => { + this.#process.on('close', () => resolve()); + this.#process.on('error', (e) => reject(e)); + this.#process.kill(); }); } diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js index 60be77204..840ee76f3 100644 --- a/ui/desktop/electron-app/src/session/manager.js +++ b/ui/desktop/electron-app/src/session/manager.js @@ -16,7 +16,7 @@ class SessionManager { * @param {string} token * @param {string} host_id */ - start(addr, target_id, token, host_id) { + async start(addr, target_id, token, host_id) { const session = new Session(addr, target_id, token, host_id); return session.start().then(() => { this.#sessions.push(session); @@ -24,7 +24,19 @@ class SessionManager { }); } - stopById() { + /** + * Stop a session and remove it from tracked sessions. + * @param {string} session_id + */ + async stopById(session_id) { + const index = this.#sessions.findIndex( + (session) => session.id === session_id + ); + if (index === -1) return; + const session = this.#sessions[index]; + return session.stop().then(() => { + this.#sessions.splice(index, 1); + }); } stopAll() { diff --git a/ui/desktop/tests/acceptance/projects/sessions-test.js b/ui/desktop/tests/acceptance/projects/sessions-test.js index eb3e0bab1..012db14cb 100644 --- a/ui/desktop/tests/acceptance/projects/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/sessions-test.js @@ -19,6 +19,7 @@ import { authenticateSession, invalidateSession, } from 'ember-simple-auth/test-support'; +import sinon from 'sinon'; module('Acceptance | projects | sessions', function (hooks) { setupApplicationTest(hooks); @@ -43,6 +44,7 @@ module('Acceptance | projects | sessions', function (hooks) { const stubs = { global: null, org: null, + ipcService: null, }; const urls = { @@ -144,6 +146,9 @@ module('Acceptance | projects | sessions', function (hooks) { window.addEventListener('message', messageHandler); setDefaultOrigin(this); + + const ipcService = this.owner.lookup('service:ipc'); + stubs.ipcService = sinon.stub(ipcService, 'invoke'); }); hooks.afterEach(function () { @@ -240,6 +245,7 @@ module('Acceptance | projects | sessions', function (hooks) { test('cancelling a session', async function (assert) { assert.expect(1); + stubs.ipcService.withArgs('cancel'); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); @@ -258,4 +264,15 @@ module('Acceptance | projects | sessions', function (hooks) { }, 750); await visit(urls.sessions); }); + + test('cancelling a session with ipc error shows notification', async function (assert) { + assert.expect(1); + stubs.ipcService.withArgs('cancel').throws(); + later(async () => { + run.cancelTimers(); + await click('tbody tr:first-child td:last-child button'); + assert.ok(find('[role="alert"].is-error')); + }, 750); + await visit(urls.sessions); + }); }); diff --git a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js index a28682950..9fde4b210 100644 --- a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js @@ -248,6 +248,7 @@ module('Acceptance | projects | targets | sessions', function (hooks) { test('cancelling a session', async function (assert) { assert.expect(2); const sessionsCount = this.server.schema.sessions.all().models.length; + stubs.ipcService.withArgs('cancel'); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); @@ -268,6 +269,17 @@ module('Acceptance | projects | targets | sessions', function (hooks) { await visit(urls.sessions); }); + test('cancelling a session with ipc error shows notification', async function (assert) { + assert.expect(1); + stubs.ipcService.withArgs('cancel').throws(); + later(async () => { + run.cancelTimers(); + await click('tbody tr:first-child td:last-child button'); + assert.ok(find('[role="alert"].is-error')); + }, 750); + await visit(urls.sessions); + }); + test('connecting to a target', async function (assert) { assert.expect(3); stubs.ipcService.withArgs('cliExists').returns(true); From 828faf2be4733dd2a046b9d2c5dcf720f2c489e9 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 09:53:10 -0400 Subject: [PATCH 14/39] refactor: Create helpers folder in desktop/electron --- ui/desktop/electron-app/src/boundary-cli.js | 2 +- .../src/{ => helpers}/spawn-promise.js | 0 ui/desktop/electron-app/src/session/index.js | 4 +- ui/desktop/electron-app/src/spawn-session.js | 44 ------------------- 4 files changed, 2 insertions(+), 48 deletions(-) rename ui/desktop/electron-app/src/{ => helpers}/spawn-promise.js (100%) delete mode 100644 ui/desktop/electron-app/src/spawn-session.js diff --git a/ui/desktop/electron-app/src/boundary-cli.js b/ui/desktop/electron-app/src/boundary-cli.js index 9eeaa4f8d..216886971 100644 --- a/ui/desktop/electron-app/src/boundary-cli.js +++ b/ui/desktop/electron-app/src/boundary-cli.js @@ -1,5 +1,5 @@ const path = require('path'); -const { spawnSync } = require('./spawn-promise'); +const { spawnSync } = require('./helpers/spawn-promise.js'); const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); diff --git a/ui/desktop/electron-app/src/spawn-promise.js b/ui/desktop/electron-app/src/helpers/spawn-promise.js similarity index 100% rename from ui/desktop/electron-app/src/spawn-promise.js rename to ui/desktop/electron-app/src/helpers/spawn-promise.js diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index cfc760ba7..d7a4af930 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -1,6 +1,4 @@ -const { spawnAsyncJSONPromise } = require('../spawn-promise.js'); - -// FIXME create structure 'util' folder for spawn-promise +const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); /** * Super paranoid shell quote/escape and validation. Input must be base62. diff --git a/ui/desktop/electron-app/src/spawn-session.js b/ui/desktop/electron-app/src/spawn-session.js deleted file mode 100644 index f632e4cea..000000000 --- a/ui/desktop/electron-app/src/spawn-session.js +++ /dev/null @@ -1,44 +0,0 @@ -const process = require('process'); - -const spawnedSessions = []; - -const purge = (process_id, index) => { - process.kill(process_id); - // Remove tracking after cancellation - spawnedSessions.splice(index, 1); -}; - -module.exports = { - /** - * Track session spawned process and it's details. - * This function is intended to tracked launched local proxy. - * @param {object} params - */ - add: ({ childProcess, data }) => { - spawnedSessions.push({ - data, - process_id: childProcess.pid, - }); - }, - /** - * Cancel spawned child process using session, if available. - * This function is intended to cancel launched local proxy. - * @param {string} session_id - */ - cancelSession: (session_id) => { - spawnedSessions.find((value, index) => { - const { process_id, data } = value; - if (data.session_id === session_id) purge(process_id, index); - }); - }, - /** - * Cancel spawned child process, if available. - * This function is intended to cancel launched local proxy. - * @param {string} process_id - */ - cancelProcess: (process_id) => { - spawnedSessions.find((value, index) => { - if (value.process_id === process_id) purge(process_id, index); - }); - }, -}; From 74501a440ff27988c13ecad8c1d38b77542ef823 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 10:27:38 -0400 Subject: [PATCH 15/39] refactor: Create structure in desktop/electron-app --- ui/desktop/electron-app/src/boundary-cli.js | 23 ---------- .../{ => config}/content-security-policy.js | 2 +- .../electron-app/src/{ => config}/menu.js | 2 +- .../src/{ => helpers}/app-updater.js | 0 .../electron-app/src/helpers/spawn-promise.js | 42 ++++++------------- ui/desktop/electron-app/src/index.js | 10 ++--- .../electron-app/src/{ => ipc}/handlers.js | 12 +++--- .../electron-app/src/{ => ipc}/ipc-handler.js | 0 .../src/{origin.js => origin/index.js} | 0 ui/desktop/electron-app/src/session/index.js | 17 ++++++-- ui/desktop/electron-app/src/utils/jsonify.js | 11 +++++ 11 files changed, 50 insertions(+), 69 deletions(-) delete mode 100644 ui/desktop/electron-app/src/boundary-cli.js rename ui/desktop/electron-app/src/{ => config}/content-security-policy.js (96%) rename ui/desktop/electron-app/src/{ => config}/menu.js (97%) rename ui/desktop/electron-app/src/{ => helpers}/app-updater.js (100%) rename ui/desktop/electron-app/src/{ => ipc}/handlers.js (87%) rename ui/desktop/electron-app/src/{ => ipc}/ipc-handler.js (100%) rename ui/desktop/electron-app/src/{origin.js => origin/index.js} (100%) create mode 100644 ui/desktop/electron-app/src/utils/jsonify.js diff --git a/ui/desktop/electron-app/src/boundary-cli.js b/ui/desktop/electron-app/src/boundary-cli.js deleted file mode 100644 index 216886971..000000000 --- a/ui/desktop/electron-app/src/boundary-cli.js +++ /dev/null @@ -1,23 +0,0 @@ -const path = require('path'); -const { spawnSync } = require('./helpers/spawn-promise.js'); - -const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); - -module.exports = { - // Check boundary cli existence - exists: () => Boolean(cliPath()), - - // Returns JSON-formatted version information from the CLI - version: () => { - const command = ['-v']; - const rawOutput = spawnSync(command); - let gitRevision = /Git Revision:\s*(?.*)\n/.exec(rawOutput); - let versionNumber = /Version Number:\s*(?.*)\n/.exec(rawOutput); - if (gitRevision) gitRevision = gitRevision.groups.rev; - if (versionNumber) versionNumber = versionNumber.groups.ver; - const formatted = versionNumber - ? `CLI Version: ${versionNumber}\nCLI Commit: ${gitRevision}` - : `CLI Commit: ${gitRevision}`; - return { gitRevision, versionNumber, formatted }; - }, -}; diff --git a/ui/desktop/electron-app/src/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js similarity index 96% rename from ui/desktop/electron-app/src/content-security-policy.js rename to ui/desktop/electron-app/src/config/content-security-policy.js index 6f5156bb6..55f9a67de 100644 --- a/ui/desktop/electron-app/src/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('./origin.js'); +const origin = require('../origin.js'); const isDev = require('electron-is-dev'); const csp = { diff --git a/ui/desktop/electron-app/src/menu.js b/ui/desktop/electron-app/src/config/menu.js similarity index 97% rename from ui/desktop/electron-app/src/menu.js rename to ui/desktop/electron-app/src/config/menu.js index 2ba8eb6e7..5f669e4bb 100644 --- a/ui/desktop/electron-app/src/menu.js +++ b/ui/desktop/electron-app/src/config/menu.js @@ -1,5 +1,5 @@ const { shell } = require('electron'); -const appUpdater = require('./app-updater.js'); +const appUpdater = require('../helpers/app-updater.js'); const generateMenuTemplate = () => { return [ diff --git a/ui/desktop/electron-app/src/app-updater.js b/ui/desktop/electron-app/src/helpers/app-updater.js similarity index 100% rename from ui/desktop/electron-app/src/app-updater.js rename to ui/desktop/electron-app/src/helpers/app-updater.js diff --git a/ui/desktop/electron-app/src/helpers/spawn-promise.js b/ui/desktop/electron-app/src/helpers/spawn-promise.js index 10467ffc6..4df9db988 100644 --- a/ui/desktop/electron-app/src/helpers/spawn-promise.js +++ b/ui/desktop/electron-app/src/helpers/spawn-promise.js @@ -1,19 +1,9 @@ const path = require('path'); const { spawn, spawnSync } = require('child_process'); -const spawnSession = require('./spawn-session'); +const jsonify = require('../utils/jsonify.js'); const boundaryPath = path.resolve(__dirname, '..', 'cli', 'boundary'); -// Convert to json -const jsonify = (data) => { - if (typeof data !== 'string') data = JSON.stringify(data); - try { - return JSON.parse(data); - } catch (e) { - // Ignore parse errors - } -}; - // You can throw exceptions, or allow them to occur, and this is supported. // Exceptions thrown in this way will be returned to the UI as an // IPC rejection. However in most cases, you probably want to return a @@ -43,35 +33,29 @@ module.exports = { let outputStream = ''; let errorStream = ''; - childProcess.stdout.on('data', (data) => { + const processData = (data) => { outputStream += data.toString(); const jsonData = jsonify(outputStream); if (jsonData) { - // if (jsonData.termination_reason) { - // spawnSession.cancelProcess(childProcess.pid); - // } else { - // // Track only successfully launched child processes - // spawnSession.add({ childProcess, data: jsonData }); - // } - // FIXME - // outputStream = ''; + childProcess.removeListener('data', processData); resolve({ childProcess, response: jsonData }); } - }); + }; - // @todo Cancel spawned session childprocess on error? Does it get automatically cancelled? - // @todo on error - should childprocess be closed? - // Test on connection limit errors especially - childProcess.stderr.on('data', (data) => { - errorStream += data.toString(); + childProcess.stdout.on('data', processData); + + // Errors are cli returned json objects + const processError = (error) => { + errorStream += error.toString(); const jsonData = jsonify(errorStream); if (jsonData) { const error = jsonData.api_error || jsonData.error; - // FIXME - // errorStream = ''; + childProcess.removeListener('error', processError); reject(new Error(error.message)); } - }); + }; + + childProcess.stderr.on('data', processError); }); }, diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 0fbcb3524..3ff94bf64 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -13,13 +13,13 @@ const { Menu, MenuItem, } = require('electron'); -require('./handlers.js'); +require('./ipc/handlers.js'); -const origin = require('./origin.js'); -const { generateCSPHeader } = require('./content-security-policy.js'); +const origin = require('./origin/index.js'); +const { generateCSPHeader } = require('./config/content-security-policy.js'); -const menu = require('./menu.js'); -const appUpdater = require('./app-updater.js'); +const menu = require('./config/menu.js'); +const appUpdater = require('./helpers/app-updater.js'); const isDev = require('electron-is-dev'); // Register the custom file protocol diff --git a/ui/desktop/electron-app/src/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js similarity index 87% rename from ui/desktop/electron-app/src/handlers.js rename to ui/desktop/electron-app/src/ipc/handlers.js index 1a03eea6a..d3a8d4efd 100644 --- a/ui/desktop/electron-app/src/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -1,9 +1,9 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); -const handle = require('./ipc-handler'); -const boundaryCli = require('./boundary-cli'); -const origin = require('./origin'); -const sessionManager = require('./session/manager'); +const handle = require('./ipc-handler.js'); +const boundaryCli = require('../cli/index.js'); +const origin = require('../origin/index.js'); +const sessionManager = require('../session/manager.js'); /** * Returns the current runtime origin, which is used by the main thread to @@ -23,9 +23,7 @@ handle('setOrigin', async (requestOrigin) => { /** * Resets the origin. */ -handle('resetOrigin', async (requestOrigin) => { - origin.resetOrigin(); -}); +handle('resetOrigin', async () => origin.resetOrigin()); /** * Opens the specified URL in an external browser. Only secure HTTPs URLs are diff --git a/ui/desktop/electron-app/src/ipc-handler.js b/ui/desktop/electron-app/src/ipc/ipc-handler.js similarity index 100% rename from ui/desktop/electron-app/src/ipc-handler.js rename to ui/desktop/electron-app/src/ipc/ipc-handler.js diff --git a/ui/desktop/electron-app/src/origin.js b/ui/desktop/electron-app/src/origin/index.js similarity index 100% rename from ui/desktop/electron-app/src/origin.js rename to ui/desktop/electron-app/src/origin/index.js diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index d7a4af930..57a41dfa1 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -1,5 +1,5 @@ const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); - +const jsonify = require('../utils/jsonify.js'); /** * Super paranoid shell quote/escape and validation. Input must be base62. * @param {string} str @@ -15,7 +15,6 @@ const escapeAndValidateBase62 = (str) => { class Session { #id; - #pid; #addr; #token; #hostId; @@ -73,11 +72,12 @@ class Session { this.#proxyDetails = spawnedSession.response; this.#process = spawnedSession.childProcess; this.#id = this.#proxyDetails.session_id; + this.monitor(); }); } /** - * Stop proxy used by session. + * Stop proxy process used by session. */ stop() { return new Promise((resolve, reject) => { @@ -87,6 +87,17 @@ class Session { }); } + monitor() { + let outputStream = ''; + this.#process.on('data', (data) => { + outputStream += data; + const jsonData = jsonify(outputStream); + if (jsonData && jsonData.termination_reason) { + this.stop(); + } + }); + } + /** * Generate cli command for session. * @returns {[]} diff --git a/ui/desktop/electron-app/src/utils/jsonify.js b/ui/desktop/electron-app/src/utils/jsonify.js new file mode 100644 index 000000000..e75bab6fb --- /dev/null +++ b/ui/desktop/electron-app/src/utils/jsonify.js @@ -0,0 +1,11 @@ +// Convert to json +const jsonify = (data) => { + if (typeof data !== 'string') data = JSON.stringify(data); + try { + return JSON.parse(data); + } catch (e) { + // Ignore parse errors + } +}; + +module.exports = jsonify; From ee748de5c9a70ac41ca7d3b47cf28b4b43da26b3 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 11:10:11 -0400 Subject: [PATCH 16/39] chore: Run cancellation tests in electron --- .../electron-app/config/forge.config.js | 2 +- .../src/config/content-security-policy.js | 2 +- .../electron-app/src/helpers/spawn-promise.js | 2 +- ui/desktop/electron-app/src/session/index.js | 13 --------- yarn.lock | 27 ++++--------------- 5 files changed, 8 insertions(+), 38 deletions(-) diff --git a/ui/desktop/electron-app/config/forge.config.js b/ui/desktop/electron-app/config/forge.config.js index 293ae0952..c426f9aff 100644 --- a/ui/desktop/electron-app/config/forge.config.js +++ b/ui/desktop/electron-app/config/forge.config.js @@ -1,5 +1,5 @@ const process = require('process'); -const { version } = require('../src/boundary-cli.js'); +const { version } = require('../src/cli/index.js'); const formattedCLIVersion = version().formatted; diff --git a/ui/desktop/electron-app/src/config/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js index 55f9a67de..8895178ef 100644 --- a/ui/desktop/electron-app/src/config/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('../origin.js'); +const origin = require('../origin/index.js'); const isDev = require('electron-is-dev'); const csp = { diff --git a/ui/desktop/electron-app/src/helpers/spawn-promise.js b/ui/desktop/electron-app/src/helpers/spawn-promise.js index 4df9db988..86ac21f68 100644 --- a/ui/desktop/electron-app/src/helpers/spawn-promise.js +++ b/ui/desktop/electron-app/src/helpers/spawn-promise.js @@ -2,7 +2,7 @@ const path = require('path'); const { spawn, spawnSync } = require('child_process'); const jsonify = require('../utils/jsonify.js'); -const boundaryPath = path.resolve(__dirname, '..', 'cli', 'boundary'); +const boundaryPath = path.resolve(__dirname, '..', '..', 'cli', 'boundary'); // You can throw exceptions, or allow them to occur, and this is supported. // Exceptions thrown in this way will be returned to the UI as an diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index 57a41dfa1..be20bff12 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -1,5 +1,4 @@ const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); -const jsonify = require('../utils/jsonify.js'); /** * Super paranoid shell quote/escape and validation. Input must be base62. * @param {string} str @@ -72,7 +71,6 @@ class Session { this.#proxyDetails = spawnedSession.response; this.#process = spawnedSession.childProcess; this.#id = this.#proxyDetails.session_id; - this.monitor(); }); } @@ -87,17 +85,6 @@ class Session { }); } - monitor() { - let outputStream = ''; - this.#process.on('data', (data) => { - outputStream += data; - const jsonData = jsonify(outputStream); - if (jsonData && jsonData.termination_reason) { - this.stop(); - } - }); - } - /** * Generate cli command for session. * @returns {[]} diff --git a/yarn.lock b/yarn.lock index 8e43f086e..497898c23 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7578,7 +7578,7 @@ debug@~4.1.0: dependencies: ms "^2.1.1" -debuglog@*, debuglog@^1.0.1: +debuglog@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492" integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI= @@ -12380,7 +12380,7 @@ import-lazy@^4.0.0: resolved "https://registry.yarnpkg.com/import-lazy/-/import-lazy-4.0.0.tgz#e8eb627483a0a43da3c03f3e35548be5cb0cc153" integrity sha512-rKtvo6a868b5Hu3heneU+L4yEQ4jYKLtjpnPeUdK7h0yzXGmyBTypknlkCvHFBqfX9YlorEiMM6Dnq/5atfHkw== -imurmurhash@*, imurmurhash@^0.1.4: +imurmurhash@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea" integrity sha1-khi5srkoojixPcT7a21XbyMUU+o= @@ -14078,11 +14078,6 @@ lodash._baseflatten@^3.0.0: lodash.isarguments "^3.0.0" lodash.isarray "^3.0.0" -lodash._baseindexof@*: - version "3.1.0" - resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c" - integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw= - lodash._baseuniq@~4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8" @@ -14091,16 +14086,11 @@ lodash._baseuniq@~4.6.0: lodash._createset "~4.0.0" lodash._root "~3.0.0" -lodash._bindcallback@*, lodash._bindcallback@^3.0.0: +lodash._bindcallback@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e" integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4= -lodash._cacheindexof@*: - version "3.0.2" - resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92" - integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI= - lodash._createassigner@^3.0.0: version "3.1.1" resolved "https://registry.yarnpkg.com/lodash._createassigner/-/lodash._createassigner-3.1.1.tgz#838a5bae2fdaca63ac22dee8e19fa4e6d6970b11" @@ -14110,19 +14100,12 @@ lodash._createassigner@^3.0.0: lodash._isiterateecall "^3.0.0" lodash.restparam "^3.0.0" -lodash._createcache@*: - version "3.1.2" - resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093" - integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM= - dependencies: - lodash._getnative "^3.0.0" - lodash._createset@~4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26" integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY= -lodash._getnative@*, lodash._getnative@^3.0.0: +lodash._getnative@^3.0.0: version "3.9.1" resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5" integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U= @@ -14340,7 +14323,7 @@ lodash.pick@^4.4.0: resolved "https://registry.yarnpkg.com/lodash.pick/-/lodash.pick-4.4.0.tgz#52f05610fff9ded422611441ed1fc123a03001b3" integrity sha1-UvBWEP/53tQiYRRB7R/BI6AwAbM= -lodash.restparam@*, lodash.restparam@^3.0.0: +lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU= From 3ef7436f3f24d8fd819c6cc4fd1d3402347a0536 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 11:40:50 -0400 Subject: [PATCH 17/39] chore: Cleanup SessionManager interface --- ui/desktop/electron-app/src/session/index.js | 23 ++++++++----------- .../electron-app/src/session/manager.js | 23 ++++--------------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/session/index.js index be20bff12..34f4440b7 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/session/index.js @@ -48,16 +48,7 @@ class Session { * @return {boolean} */ get isActive() { - return this.#process.connected; - } - - /** - * Returns session proxy details. - * Only available after successful start of a session. - * @return {object} - */ - get proxyDetails() { - return this.#proxyDetails; + return !this.#process.killed; } /** @@ -71,6 +62,7 @@ class Session { this.#proxyDetails = spawnedSession.response; this.#process = spawnedSession.childProcess; this.#id = this.#proxyDetails.session_id; + return this.#proxyDetails; }); } @@ -79,9 +71,14 @@ class Session { */ stop() { return new Promise((resolve, reject) => { - this.#process.on('close', () => resolve()); - this.#process.on('error', (e) => reject(e)); - this.#process.kill(); + if (this.isActive) { + this.#process.on('close', () => resolve()); + this.#process.on('error', (e) => reject(e)); + this.#process.kill(); + } else { + // Do nothing when process isn't active + resolve(); + } }); } diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js index 840ee76f3..cb25bef93 100644 --- a/ui/desktop/electron-app/src/session/manager.js +++ b/ui/desktop/electron-app/src/session/manager.js @@ -10,7 +10,6 @@ class SessionManager { /** * Start a session and track it. * Returns session proxy details if successfully started. - * Only successfully started sessions will be tracked. * @param {string} addr * @param {string} target_id * @param {string} token @@ -18,33 +17,21 @@ class SessionManager { */ async start(addr, target_id, token, host_id) { const session = new Session(addr, target_id, token, host_id); - return session.start().then(() => { - this.#sessions.push(session); - return session.proxyDetails; - }); + this.#sessions.push(session); + return session.start(); } /** - * Stop a session and remove it from tracked sessions. + * Stop a session using identifier. * @param {string} session_id */ async stopById(session_id) { - const index = this.#sessions.findIndex( - (session) => session.id === session_id - ); - if (index === -1) return; - const session = this.#sessions[index]; - return session.stop().then(() => { - this.#sessions.splice(index, 1); - }); + const session = this.#sessions.find((session) => session.id === session_id); + return session.stop(); } stopAll() { } - - get sessions() { - return this.#sessions; - } } const sessionManager = new SessionManager(); From 4e13ea56c3a4b89c20b451ae8a78677f81eab0e5 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 12:35:17 -0400 Subject: [PATCH 18/39] Add bug fix to changelog --- ui/desktop/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/desktop/CHANGELOG.md b/ui/desktop/CHANGELOG.md index e72d90c5e..614ad5f8a 100644 --- a/ui/desktop/CHANGELOG.md +++ b/ui/desktop/CHANGELOG.md @@ -2,6 +2,12 @@ Canonical reference for changes, improvements, and bugfixes for Boundary Desktop. +## Next + +### Bug Fixes + +- Close spawned boundary process on session cancel. ([PR](https://github.com/hashicorp/boundary-ui/pull/549)) + ## v1.0.0 [CLI 0.2.0] (2021.04.14) ### New From ce7cdba0bc2a25133e982b1af2d98dde1c3c17cb Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 12:27:16 -0400 Subject: [PATCH 19/39] feat: Prompt for quitting session on desktop quit --- ui/desktop/electron-app/src/index.js | 27 ++++++++++++++++++- .../electron-app/src/session/manager.js | 11 +++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 3ff94bf64..e4db2c671 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -7,9 +7,9 @@ const { const { session, app, + dialog, protocol, BrowserWindow, - ipcMain, Menu, MenuItem, } = require('electron'); @@ -17,6 +17,7 @@ require('./ipc/handlers.js'); const origin = require('./origin/index.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); +const sessionManager = require('./session/manager.js'); const menu = require('./config/menu.js'); const appUpdater = require('./helpers/app-updater.js'); @@ -183,6 +184,30 @@ app.on('ready', async () => { }); }); +/** + * Prompt for closing spawned processes + */ +app.on('before-quit', (event) => { + if (sessionManager.hasActiveSessions) { + const dialogOpts = { + type: 'question', + buttons: ['Close & Quit', 'Ignore & Quit', 'Cancel'], + detail: 'Close sessions before quitting?', + }; + + dialog.showMessageBox(dialogOpts).then((returnValue) => { + switch (returnValue.response) { + case 0: + sessionManager.stopAll(); + break; + case 2: + event.preventDefault(); + break; + } + }); + } +}); + // Handle an unhandled error in the main thread // // Note that 'uncaughtException' is a crude mechanism for exception handling intended to diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js index cb25bef93..63b3a8943 100644 --- a/ui/desktop/electron-app/src/session/manager.js +++ b/ui/desktop/electron-app/src/session/manager.js @@ -7,6 +7,14 @@ class SessionManager { this.#sessions = []; } + /** + * Checks for active sessions + * @returns {boolean} + */ + get hasActiveSessions() { + return Boolean(this.#sessions.find((session) => session.isActive)); + } + /** * Start a session and track it. * Returns session proxy details if successfully started. @@ -27,10 +35,11 @@ class SessionManager { */ async stopById(session_id) { const session = this.#sessions.find((session) => session.id === session_id); - return session.stop(); + return session && session.stop(); } stopAll() { + this.#sessions.forEach((session) => session.stop()); } } From 4b587d35fa831ece55d480d3bffd1c8bd289248d Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 12:48:03 -0400 Subject: [PATCH 20/39] docs: Add feature note to changelog --- ui/desktop/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/desktop/CHANGELOG.md b/ui/desktop/CHANGELOG.md index 614ad5f8a..93e8fb472 100644 --- a/ui/desktop/CHANGELOG.md +++ b/ui/desktop/CHANGELOG.md @@ -4,6 +4,10 @@ Canonical reference for changes, improvements, and bugfixes for Boundary Desktop ## Next +### New + +- Add prompt to close sessions when quitting client. ([PR](https://github.com/hashicorp/boundary-ui/pull/555)) + ### Bug Fixes - Close spawned boundary process on session cancel. ([PR](https://github.com/hashicorp/boundary-ui/pull/549)) From dcc77153bfc699077f6b608bc38ef38a669fa760 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 14:27:01 -0400 Subject: [PATCH 21/39] feat: Do not allow desktop to quit without closing sessions --- ui/desktop/electron-app/src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index e4db2c671..07a9a32d2 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -191,7 +191,7 @@ app.on('before-quit', (event) => { if (sessionManager.hasActiveSessions) { const dialogOpts = { type: 'question', - buttons: ['Close & Quit', 'Ignore & Quit', 'Cancel'], + buttons: ['Close', 'Cancel'], detail: 'Close sessions before quitting?', }; @@ -200,7 +200,7 @@ app.on('before-quit', (event) => { case 0: sessionManager.stopAll(); break; - case 2: + case 1: event.preventDefault(); break; } From 56936f78b89c94386206d55aa986079bfa115105 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 15:31:28 -0400 Subject: [PATCH 22/39] chore: Update cancel to stop for ipc sessions call Additional cleanup: remove unused async calls, separate session and associated manager models, & attempt encoding origin url as part of sanitization for cli command --- .../routes/scopes/scope/projects/sessions.js | 2 +- .../scope/projects/targets/target/sessions.js | 2 +- .../electron-app/src/helpers/spawn-promise.js | 6 +-- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../src/models/session-manager.js | 33 ++++++++++++++++ .../{session/index.js => models/session.js} | 22 +++-------- .../electron-app/src/session/manager.js | 39 ++----------------- ui/desktop/electron-app/src/utils/base62.js | 14 +++++++ .../acceptance/projects/sessions-test.js | 4 +- .../projects/targets/sessions-test.js | 2 +- 10 files changed, 66 insertions(+), 60 deletions(-) create mode 100644 ui/desktop/electron-app/src/models/session-manager.js rename ui/desktop/electron-app/src/{session/index.js => models/session.js} (78%) create mode 100644 ui/desktop/electron-app/src/utils/base62.js diff --git a/ui/desktop/app/routes/scopes/scope/projects/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/sessions.js index 908636f1a..cd11b0569 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/sessions.js @@ -84,6 +84,6 @@ export default class ScopesScopeProjectsSessionsRoute extends Route { @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - await this.ipc.invoke('cancel', { session_id: session.id }); + await this.ipc.invoke('stop', { session_id: session.id }); } } diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js index 2263aa448..f84af4eac 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/target/sessions.js @@ -19,6 +19,6 @@ export default class ScopesScopeProjectsTargetsTargetSessionsRoute extends Route @notifySuccess('notifications.canceled-success') async cancelSession(session) { await session.cancelSession(); - await this.ipc.invoke('cancel', { session_id: session.id }); + await this.ipc.invoke('stop', { session_id: session.id }); } } diff --git a/ui/desktop/electron-app/src/helpers/spawn-promise.js b/ui/desktop/electron-app/src/helpers/spawn-promise.js index 86ac21f68..b8ff5d980 100644 --- a/ui/desktop/electron-app/src/helpers/spawn-promise.js +++ b/ui/desktop/electron-app/src/helpers/spawn-promise.js @@ -37,7 +37,7 @@ module.exports = { outputStream += data.toString(); const jsonData = jsonify(outputStream); if (jsonData) { - childProcess.removeListener('data', processData); + childProcess.removeAllListeners(); resolve({ childProcess, response: jsonData }); } }; @@ -50,12 +50,12 @@ module.exports = { const jsonData = jsonify(errorStream); if (jsonData) { const error = jsonData.api_error || jsonData.error; - childProcess.removeListener('error', processError); + childProcess.removeAllListeners(); reject(new Error(error.message)); } }; - childProcess.stderr.on('data', processError); + childProcess.stderr.on('error', processError); }); }, diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index d3a8d4efd..e727a4b48 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -61,4 +61,4 @@ handle('connect', ({ target_id, token, host_id }) => /** * Cancel an established boundary session spawned process. */ -handle('cancel', ({ session_id }) => sessionManager.stopById(session_id)); +handle('stop', ({ session_id }) => sessionManager.stopById(session_id)); diff --git a/ui/desktop/electron-app/src/models/session-manager.js b/ui/desktop/electron-app/src/models/session-manager.js new file mode 100644 index 000000000..dfcfd923a --- /dev/null +++ b/ui/desktop/electron-app/src/models/session-manager.js @@ -0,0 +1,33 @@ +const Session = require('./session.js'); + +class SessionManager { + #sessions = []; + + /** + * Start a session and track it. + * Returns session proxy details if successfully started. + * @param {string} addr + * @param {string} target_id + * @param {string} token + * @param {string} host_id + */ + start(addr, target_id, token, host_id) { + const session = new Session(addr, target_id, token, host_id); + this.#sessions.push(session); + return session.start(); + } + + /** + * Stop a session using identifier. + * @param {string} session_id + */ + stopById(session_id) { + const session = this.#sessions.find((session) => session.id === session_id); + return session.stop(); + } + + stopAll() { + } +} + +module.exports = SessionManager; diff --git a/ui/desktop/electron-app/src/session/index.js b/ui/desktop/electron-app/src/models/session.js similarity index 78% rename from ui/desktop/electron-app/src/session/index.js rename to ui/desktop/electron-app/src/models/session.js index 34f4440b7..105b7d723 100644 --- a/ui/desktop/electron-app/src/session/index.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -1,16 +1,5 @@ const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); -/** - * Super paranoid shell quote/escape and validation. Input must be base62. - * @param {string} str - */ -const escapeAndValidateBase62 = (str) => { - const candidate = str.toString(); - if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; - throw new Error(` - Could not invoke command: - input contained unsafe characters. - `); -}; +const base62 = require('../utils/base62.js'); class Session { #id; @@ -88,20 +77,21 @@ class Session { */ cliCommand() { const sanitized = { - target_id: escapeAndValidateBase62(this.#targetId), - token: escapeAndValidateBase62(this.#token), + target_id: base62.escapeAndValidate(this.#targetId), + token: base62.escapeAndValidate(this.#token), + addr: encodeURI(this.#addr), }; const command = [ 'connect', `-target-id=${sanitized.target_id}`, `-token=${sanitized.token}`, - `-addr=${this.#addr}`, + `-addr=${sanitized.addr}`, '-format=json', ]; if (this.#hostId) { - sanitized.host_id = escapeAndValidateBase62(this.#hostId); + sanitized.host_id = base62.escapeAndValidate(this.#hostId); command.push(`-host-id=${sanitized.host_id}`); } return command; diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/session/manager.js index cb25bef93..dc96e096f 100644 --- a/ui/desktop/electron-app/src/session/manager.js +++ b/ui/desktop/electron-app/src/session/manager.js @@ -1,38 +1,7 @@ -const Session = require('./index.js'); - -class SessionManager { - #sessions; - - constructor() { - this.#sessions = []; - } - - /** - * Start a session and track it. - * Returns session proxy details if successfully started. - * @param {string} addr - * @param {string} target_id - * @param {string} token - * @param {string} host_id - */ - async start(addr, target_id, token, host_id) { - const session = new Session(addr, target_id, token, host_id); - this.#sessions.push(session); - return session.start(); - } - - /** - * Stop a session using identifier. - * @param {string} session_id - */ - async stopById(session_id) { - const session = this.#sessions.find((session) => session.id === session_id); - return session.stop(); - } - - stopAll() { - } -} +const SessionManager = require('../models/session-manager.js'); +/** + * Singleton for SessionManager + */ const sessionManager = new SessionManager(); module.exports = sessionManager; diff --git a/ui/desktop/electron-app/src/utils/base62.js b/ui/desktop/electron-app/src/utils/base62.js new file mode 100644 index 000000000..0c844b790 --- /dev/null +++ b/ui/desktop/electron-app/src/utils/base62.js @@ -0,0 +1,14 @@ +module.exports = { + /** + * Super paranoid shell quote/escape and validation. Input must be base62. + * @param {string} str + */ + escapeAndValidate: (str) => { + const candidate = str.toString(); + if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; + throw new Error(` + Could not invoke command: + input contained unsafe characters. + `); + }, +}; diff --git a/ui/desktop/tests/acceptance/projects/sessions-test.js b/ui/desktop/tests/acceptance/projects/sessions-test.js index 012db14cb..2cdbafc16 100644 --- a/ui/desktop/tests/acceptance/projects/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/sessions-test.js @@ -245,7 +245,7 @@ module('Acceptance | projects | sessions', function (hooks) { test('cancelling a session', async function (assert) { assert.expect(1); - stubs.ipcService.withArgs('cancel'); + stubs.ipcService.withArgs('stop'); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); @@ -267,7 +267,7 @@ module('Acceptance | projects | sessions', function (hooks) { test('cancelling a session with ipc error shows notification', async function (assert) { assert.expect(1); - stubs.ipcService.withArgs('cancel').throws(); + stubs.ipcService.withArgs('stop').throws(); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); diff --git a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js index 9fde4b210..e6031ab88 100644 --- a/ui/desktop/tests/acceptance/projects/targets/sessions-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/sessions-test.js @@ -271,7 +271,7 @@ module('Acceptance | projects | targets | sessions', function (hooks) { test('cancelling a session with ipc error shows notification', async function (assert) { assert.expect(1); - stubs.ipcService.withArgs('cancel').throws(); + stubs.ipcService.withArgs('stop').throws(); later(async () => { run.cancelTimers(); await click('tbody tr:first-child td:last-child button'); From 542b469ad8deb769a16090f7270d79292e91c6f8 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 15:46:40 -0400 Subject: [PATCH 23/39] chore: Reapply stopping all session in desktop after merge --- ui/desktop/electron-app/src/models/session-manager.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ui/desktop/electron-app/src/models/session-manager.js b/ui/desktop/electron-app/src/models/session-manager.js index dfcfd923a..3bcd7938f 100644 --- a/ui/desktop/electron-app/src/models/session-manager.js +++ b/ui/desktop/electron-app/src/models/session-manager.js @@ -3,6 +3,14 @@ const Session = require('./session.js'); class SessionManager { #sessions = []; + /** + * Checks for active sessions + * @returns {boolean} + */ + get hasActiveSessions() { + return Boolean(this.#sessions.find((session) => session.isActive)); + } + /** * Start a session and track it. * Returns session proxy details if successfully started. @@ -23,10 +31,11 @@ class SessionManager { */ stopById(session_id) { const session = this.#sessions.find((session) => session.id === session_id); - return session.stop(); + return session && session.stop(); } stopAll() { + this.#sessions.forEach((session) => session.stop()); } } From 13146975e2c7e5a2e10b59ef4baf5951fb0b58f8 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 16:57:27 -0400 Subject: [PATCH 24/39] fix: Add missing cli folder in electron-app --- ui/desktop/electron-app/.gitignore | 2 +- ui/desktop/electron-app/src/cli/index.js | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 ui/desktop/electron-app/src/cli/index.js diff --git a/ui/desktop/electron-app/.gitignore b/ui/desktop/electron-app/.gitignore index 0928c4d31..75a95bf75 100644 --- a/ui/desktop/electron-app/.gitignore +++ b/ui/desktop/electron-app/.gitignore @@ -93,7 +93,7 @@ ember-dist/ ember-test/ # CLI -cli/ +/cli/ # Autoupdate nextVersion/ diff --git a/ui/desktop/electron-app/src/cli/index.js b/ui/desktop/electron-app/src/cli/index.js new file mode 100644 index 000000000..cfb5153a5 --- /dev/null +++ b/ui/desktop/electron-app/src/cli/index.js @@ -0,0 +1,23 @@ +const path = require('path'); +const { spawnSync } = require('../helpers/spawn-promise.js'); + +const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); + +module.exports = { + // Check boundary cli existence + exists: () => Boolean(cliPath()), + + // Returns JSON-formatted version information from the CLI + version: () => { + const command = ['-v']; + const rawOutput = spawnSync(command); + let gitRevision = /Git Revision:\s*(?.*)\n/.exec(rawOutput); + let versionNumber = /Version Number:\s*(?.*)\n/.exec(rawOutput); + if (gitRevision) gitRevision = gitRevision.groups.rev; + if (versionNumber) versionNumber = versionNumber.groups.ver; + const formatted = versionNumber + ? `CLI Version: ${versionNumber}\nCLI Commit: ${gitRevision}` + : `CLI Commit: ${gitRevision}`; + return { gitRevision, versionNumber, formatted }; + }, +}; From ae8e20b8e1e8a6f9fae118dfae6e090a40e88896 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 10:23:22 -0400 Subject: [PATCH 25/39] refactor: Create services folder in desktop/electron-app --- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../src/{session/manager.js => services/session-manager.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename ui/desktop/electron-app/src/{session/manager.js => services/session-manager.js} (100%) diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index e727a4b48..befb10dda 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -3,7 +3,7 @@ const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); const origin = require('../origin/index.js'); -const sessionManager = require('../session/manager.js'); +const sessionManager = require('../services/session-manager.js'); /** * Returns the current runtime origin, which is used by the main thread to diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/services/session-manager.js similarity index 100% rename from ui/desktop/electron-app/src/session/manager.js rename to ui/desktop/electron-app/src/services/session-manager.js From 9f32b8cdc3654387ab7f09decf219d701c9380ee Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 10:36:28 -0400 Subject: [PATCH 26/39] refactor: Origin runtime settings into origin service desktop --- .../src/config/content-security-policy.js | 2 +- .../src/helpers/net-request-promise.js | 24 ++++++++++++++ ui/desktop/electron-app/src/index.js | 2 +- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../index.js => models/runtime-settings.js} | 33 ++----------------- .../electron-app/src/services/origin.js | 8 +++++ 6 files changed, 37 insertions(+), 34 deletions(-) create mode 100644 ui/desktop/electron-app/src/helpers/net-request-promise.js rename ui/desktop/electron-app/src/{origin/index.js => models/runtime-settings.js} (59%) create mode 100644 ui/desktop/electron-app/src/services/origin.js diff --git a/ui/desktop/electron-app/src/config/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js index 8895178ef..1de13924e 100644 --- a/ui/desktop/electron-app/src/config/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('../origin/index.js'); +const origin = require('../services/origin.js'); const isDev = require('electron-is-dev'); const csp = { diff --git a/ui/desktop/electron-app/src/helpers/net-request-promise.js b/ui/desktop/electron-app/src/helpers/net-request-promise.js new file mode 100644 index 000000000..eb48426c7 --- /dev/null +++ b/ui/desktop/electron-app/src/helpers/net-request-promise.js @@ -0,0 +1,24 @@ +const { net } = require('electron'); + +const requestTimeoutSeconds = 10; +// Simple promise wrapper around Electron's net.request feature. +// https://www.electronjs.org/docs/api/client-request +const netRequestPromise = (url) => + new Promise((resolve, reject) => { + try { + const request = net.request(url); + // We don't worry about canceling the request timeout timer once a + // request ends because: request.abort() has no effect after a request + // ends and the promise will already have been completed, so a rejection + // has no effect either. + const requestTimeout = setTimeout(() => { + request.abort(); + reject(new Error('Request timeout')); + }, requestTimeoutSeconds * 1000); + request.on('error', reject); + request.on('response', resolve); + request.end(); + } catch (e) { + reject(e); + } + }); diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 3ff94bf64..8e61bea48 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -15,7 +15,7 @@ const { } = require('electron'); require('./ipc/handlers.js'); -const origin = require('./origin/index.js'); +const origin = require('./services/origin.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); const menu = require('./config/menu.js'); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index befb10dda..f32a00e28 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -2,7 +2,7 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); -const origin = require('../origin/index.js'); +const origin = require('../services/origin.js'); const sessionManager = require('../services/session-manager.js'); /** diff --git a/ui/desktop/electron-app/src/origin/index.js b/ui/desktop/electron-app/src/models/runtime-settings.js similarity index 59% rename from ui/desktop/electron-app/src/origin/index.js rename to ui/desktop/electron-app/src/models/runtime-settings.js index 5cc150c32..fdd91608c 100644 --- a/ui/desktop/electron-app/src/origin/index.js +++ b/ui/desktop/electron-app/src/models/runtime-settings.js @@ -1,34 +1,7 @@ -const { net } = require('electron'); - -const requestTimeoutSeconds = 10; -// Simple promise wrapper around Electron's net.request feature. -// https://www.electronjs.org/docs/api/client-request -const netRequestPromise = (url) => - new Promise((resolve, reject) => { - try { - const request = net.request(url); - // We don't worry about canceling the request timeout timer once a - // request ends because: request.abort() has no effect after a request - // ends and the promise will already have been completed, so a rejection - // has no effect either. - const requestTimeout = setTimeout(() => { - request.abort(); - reject(new Error('Request timeout')); - }, requestTimeoutSeconds * 1000); - request.on('error', reject); - request.on('response', resolve); - request.end(); - } catch (e) { - reject(e); - } - }); - -// Provides a singleton class instance to enable a consistent view of -// runtime settings across the application. +const netRequestPromise = require('../helpers/net-request-promise.js'); // Runtime settings are any configuration, especially user-specified, // set up at runtime. For example, the Boundary origin is a runtime setting. - class RuntimeSettings { // Internal private origin is exposed via getter/setter below. #origin = undefined; @@ -84,6 +57,4 @@ class RuntimeSettings { } } -const runtimeSettings = new RuntimeSettings(); - -module.exports = runtimeSettings; +module.exports = RuntimeSettings; diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/origin.js new file mode 100644 index 000000000..8d33f04fc --- /dev/null +++ b/ui/desktop/electron-app/src/services/origin.js @@ -0,0 +1,8 @@ +const RuntimeSesttings = require('../models/runtime-settings.js'); + +// Provides a singleton class instance to enable a consistent view of +// runtime settings across the application. + +const runtimeSettings = new RuntimeSettings(); + +module.exports = runtimeSettings; From 385ea34c98e86200bcd42be77f6e38c19533b944 Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Thu, 13 May 2021 09:41:13 -0700 Subject: [PATCH 27/39] Swap @style='expand' to boolean @expand='true' --- .../components/rose/table/index.stories.mdx | 24 +++++++------------ .../components/rose/table/row/cell/index.js | 8 ++++--- .../rose/tests/dummy/app/templates/index.hbs | 10 ++++---- .../components/rose/table/row/cell-test.js | 10 ++++++-- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index f11560e41..0a70f24e8 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -15,7 +15,7 @@ Cell expansion style should be applied to only one cell in a row. Address - Host Set + Host Set Name Description @@ -23,19 +23,19 @@ Cell expansion style should be applied to only one cell in a row. 1.1.1.1 - host-set-1, host-set-2 + host-set-1, host-set-2 optional name description 1.1.1.1 - host-set-2 + host-set-2 optional name description 1.1.1.1 - host-set-1 + host-set-1 optional name description @@ -55,7 +55,7 @@ Cell expansion style should be applied to only one cell in a row. Address - Host Set + Host Set Name Description @@ -63,19 +63,19 @@ Cell expansion style should be applied to only one cell in a row. 1.1.1.1 - host-set-1, host-set-2 + host-set-1, host-set-2 optional name description 1.1.1.1 - host-set-2 + host-set-2 optional name description 1.1.1.1 - host-set-1 + host-set-1 optional name description @@ -89,13 +89,7 @@ Cell expansion style should be applied to only one cell in a row. `, context: { hideHeaderRow: boolean('hideHeaderRow', false), - cellStyle: select( - 'cellStyle', - { - default: '', - 'expand': 'expand' - } - ), + cellExpand: boolean('cellExpand', false), style: select( 'style', { diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js index 09f5f53dc..4d0a04897 100644 --- a/addons/rose/addon/components/rose/table/row/cell/index.js +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -5,14 +5,16 @@ import { computed } from '@ember/object'; export default Component.extend({ layout, tagName: 'td', - classNameBindings: ['cellTypeStyle', 'cellStyle', 'cellAlignmentStyle'], + classNameBindings: ['cellTypeStyle', 'cellExpand', 'cellAlignmentStyle'], cellTypeStyle: computed(function () { return this.tagName.match('th') ? `rose-table-header-cell` : 'rose-table-cell'; }), - cellStyle: computed(function () { - return this.style ? `rose-table-cell-${this.style}` : ''; + cellExpand: computed(function () { + if (this.expand) { + return 'rose-table-cell-expand' + } }), cellAlignmentStyle: computed(function () { if (this.alignRight) { diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index 17cd6861b..c7c91cea6 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -30,7 +30,7 @@ Address Host Set Name - Description + Description @@ -40,7 +40,7 @@ Remove - description + description @@ -80,7 +80,7 @@ Key - Value + Value @@ -98,7 +98,7 @@ Key - + Remove @@ -120,7 +120,7 @@ Address Host Set Name - Description + Description diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js index dcc93e33c..65fa7083f 100644 --- a/addons/rose/tests/integration/components/rose/table/row/cell-test.js +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -30,12 +30,18 @@ module('Integration | Component | rose/table/row/cell', function (hooks) { assert.notOk(find('.rose-table-cell')); }); - test('it adds a style class', async function (assert) { + test('it adds expand style class', async function (assert) { assert.expect(1); - await render(hbs``); + await render(hbs``); assert.ok(find('.rose-table-cell-expand')); }); + test('it does not add expand style class', async function (assert) { + assert.expect(1); + await render(hbs``); + assert.notOk(find('.rose-table-cell-expand')); + }); + test('it renders with align center in a header cell', async function (assert) { assert.expect(2); await render( From cc01bddea32c27788ad00eb858cd8421e346b2f3 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 12:50:01 -0400 Subject: [PATCH 28/39] refactor: Create net-request-promise helper for runtimesettin --- ui/desktop/electron-app/src/cli/index.js | 2 +- .../electron-app/src/helpers/net-request-promise.js | 2 ++ ui/desktop/electron-app/src/models/session.js | 11 +++++------ ui/desktop/electron-app/src/services/origin.js | 2 +- .../src/utils/{base62.js => sanitizer.js} | 10 +++++++++- 5 files changed, 18 insertions(+), 9 deletions(-) rename ui/desktop/electron-app/src/utils/{base62.js => sanitizer.js} (58%) diff --git a/ui/desktop/electron-app/src/cli/index.js b/ui/desktop/electron-app/src/cli/index.js index cfb5153a5..0aaad4e31 100644 --- a/ui/desktop/electron-app/src/cli/index.js +++ b/ui/desktop/electron-app/src/cli/index.js @@ -1,7 +1,7 @@ const path = require('path'); const { spawnSync } = require('../helpers/spawn-promise.js'); -const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); +const cliPath = async () => path.resolve(__dirname, '..', '..', 'cli', 'boundary'); module.exports = { // Check boundary cli existence diff --git a/ui/desktop/electron-app/src/helpers/net-request-promise.js b/ui/desktop/electron-app/src/helpers/net-request-promise.js index eb48426c7..c86030c79 100644 --- a/ui/desktop/electron-app/src/helpers/net-request-promise.js +++ b/ui/desktop/electron-app/src/helpers/net-request-promise.js @@ -22,3 +22,5 @@ const netRequestPromise = (url) => reject(e); } }); + +module.exports = netRequestPromise; diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index 105b7d723..20d5b37a4 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -1,5 +1,5 @@ const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); -const base62 = require('../utils/base62.js'); +const sanitizer = require('../utils/sanitizer.js'); class Session { #id; @@ -77,21 +77,20 @@ class Session { */ cliCommand() { const sanitized = { - target_id: base62.escapeAndValidate(this.#targetId), - token: base62.escapeAndValidate(this.#token), - addr: encodeURI(this.#addr), + target_id: sanitizer.base62EscapeAndValidate(this.#targetId), + token: sanitizer.base62EscapeAndValidate(this.#token), }; const command = [ 'connect', `-target-id=${sanitized.target_id}`, `-token=${sanitized.token}`, - `-addr=${sanitized.addr}`, + `-addr=${this.#addr}`, '-format=json', ]; if (this.#hostId) { - sanitized.host_id = base62.escapeAndValidate(this.#hostId); + sanitized.host_id = sanitizer.base62EscapeAndValidate(this.#hostId); command.push(`-host-id=${sanitized.host_id}`); } return command; diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/origin.js index 8d33f04fc..8c5634079 100644 --- a/ui/desktop/electron-app/src/services/origin.js +++ b/ui/desktop/electron-app/src/services/origin.js @@ -1,4 +1,4 @@ -const RuntimeSesttings = require('../models/runtime-settings.js'); +const RuntimeSettings = require('../models/runtime-settings.js'); // Provides a singleton class instance to enable a consistent view of // runtime settings across the application. diff --git a/ui/desktop/electron-app/src/utils/base62.js b/ui/desktop/electron-app/src/utils/sanitizer.js similarity index 58% rename from ui/desktop/electron-app/src/utils/base62.js rename to ui/desktop/electron-app/src/utils/sanitizer.js index 0c844b790..bc48fe0cf 100644 --- a/ui/desktop/electron-app/src/utils/base62.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -1,9 +1,11 @@ +const { quote } = require('shell-quote'); + module.exports = { /** * Super paranoid shell quote/escape and validation. Input must be base62. * @param {string} str */ - escapeAndValidate: (str) => { + base62EscapeAndValidate: (str) => { const candidate = str.toString(); if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; throw new Error(` @@ -11,4 +13,10 @@ module.exports = { input contained unsafe characters. `); }, + /** + * Return a quoted string suitable for using in shell commands. + * @param {string} str + * @returns {string} + */ + shellEscape: (str) => quote([str]), }; From 1397df807f83d144ee563822f74fbde525b94073 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 10:40:50 -0400 Subject: [PATCH 29/39] fix: Add url validation to origin addr in desktop --- ui/desktop/electron-app/src/models/session.js | 3 ++- ui/desktop/electron-app/src/utils/sanitizer.js | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index 20d5b37a4..6ec9d3ce8 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -79,13 +79,14 @@ class Session { const sanitized = { target_id: sanitizer.base62EscapeAndValidate(this.#targetId), token: sanitizer.base62EscapeAndValidate(this.#token), + addr: sanitizer.urlValidate(this.#addr), }; const command = [ 'connect', `-target-id=${sanitized.target_id}`, `-token=${sanitized.token}`, - `-addr=${this.#addr}`, + `-addr=${sanitized.addr}`, '-format=json', ]; diff --git a/ui/desktop/electron-app/src/utils/sanitizer.js b/ui/desktop/electron-app/src/utils/sanitizer.js index bc48fe0cf..582f2532c 100644 --- a/ui/desktop/electron-app/src/utils/sanitizer.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -1,4 +1,4 @@ -const { quote } = require('shell-quote'); +const { URL } = require('url'); module.exports = { /** @@ -18,5 +18,16 @@ module.exports = { * @param {string} str * @returns {string} */ - shellEscape: (str) => quote([str]), + urlValidate: (str) => { + try { + const url = new URL(str); + return url.href; + } catch (e) { + // Parse error + throw new Error(` + Could not invoke command: + input contained unsafe url. + `); + } + }, }; From 82a0c7c17cb912bc19ef63fc11ce80eaa01def86 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 14:29:26 -0400 Subject: [PATCH 30/39] refactor: Rename origin service to runtime-settings --- .../src/config/content-security-policy.js | 6 +++--- ui/desktop/electron-app/src/index.js | 4 ++-- ui/desktop/electron-app/src/ipc/handlers.js | 12 ++++++------ .../src/services/{origin.js => runtime-settings.js} | 0 4 files changed, 11 insertions(+), 11 deletions(-) rename ui/desktop/electron-app/src/services/{origin.js => runtime-settings.js} (100%) diff --git a/ui/desktop/electron-app/src/config/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js index 1de13924e..19ccafc93 100644 --- a/ui/desktop/electron-app/src/config/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('../services/origin.js'); +const runtimeSettings = require('../services/runtime-settings.js'); const isDev = require('electron-is-dev'); const csp = { @@ -28,9 +28,9 @@ const generateCSPHeader = () => { const policy = Object.assign({}, csp); // If an origin is specified, add it to the connect-src directive - if (origin.origin) { + if (runtimeSettings.origin) { policy['connect-src'] = policy['connect-src'].slice(); - policy['connect-src'].push(origin.origin); + policy['connect-src'].push(runtimeSettings.origin); } return Object.keys(policy) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 8e61bea48..93c9c3024 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -15,7 +15,7 @@ const { } = require('electron'); require('./ipc/handlers.js'); -const origin = require('./services/origin.js'); +const runtimeSettings = require('./services/runtime-settings.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); const menu = require('./config/menu.js'); @@ -131,7 +131,7 @@ app.on('ready', async () => { // If the user-specified origin changes, reload the page so that // the CSP can be refreshed with the this source allowed - origin.onOriginChange(() => mainWindow.loadURL(emberAppURL)); + runtimeSettings.onOriginChange(() => mainWindow.loadURL(emberAppURL)); // If you want to open up dev tools programmatically, call // mainWindow.openDevTools(); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index f32a00e28..e3bdcbabd 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -2,28 +2,28 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); -const origin = require('../services/origin.js'); const sessionManager = require('../services/session-manager.js'); +const runtimeSettings = require('../services/runtime-settings.js'); /** * Returns the current runtime origin, which is used by the main thread to * rewrite the CSP to allow requests. */ -handle('getOrigin', () => origin.origin); +handle('getOrigin', () => runtimeSettings.origin); /** * Sets the origin to be used in the content security policy and triggers * a main window reload. */ handle('setOrigin', async (requestOrigin) => { - await origin.validateOrigin(requestOrigin); - origin.origin = requestOrigin; + await runtimeSettings.validateOrigin(requestOrigin); + runtimeSettings.origin = requestOrigin; }); /** * Resets the origin. */ -handle('resetOrigin', async () => origin.resetOrigin()); +handle('resetOrigin', async () => runtimeSettings.resetOrigin()); /** * Opens the specified URL in an external browser. Only secure HTTPs URLs are @@ -55,7 +55,7 @@ handle('cliExists', () => boundaryCli.exists()); * Establishes a boundary session and returns session details. */ handle('connect', ({ target_id, token, host_id }) => - sessionManager.start(origin.origin, target_id, token, host_id) + sessionManager.start(runtimeSettings.origin, target_id, token, host_id) ); /** diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/runtime-settings.js similarity index 100% rename from ui/desktop/electron-app/src/services/origin.js rename to ui/desktop/electron-app/src/services/runtime-settings.js From 1a0650ff7cee6bc9db6c6a8a381a0de946d43130 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 15:00:24 -0400 Subject: [PATCH 31/39] fix: Add url validation to origin validation in desktop --- ui/desktop/electron-app/src/ipc/handlers.js | 6 ++++-- ui/desktop/electron-app/src/utils/sanitizer.js | 7 ++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index e3bdcbabd..529fc34a5 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -4,6 +4,7 @@ const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); const sessionManager = require('../services/session-manager.js'); const runtimeSettings = require('../services/runtime-settings.js'); +const sanitizer = require('../utils/sanitizer.js'); /** * Returns the current runtime origin, which is used by the main thread to @@ -16,8 +17,9 @@ handle('getOrigin', () => runtimeSettings.origin); * a main window reload. */ handle('setOrigin', async (requestOrigin) => { - await runtimeSettings.validateOrigin(requestOrigin); - runtimeSettings.origin = requestOrigin; + const origin = sanitizer.urlValidate(requestOrigin); + await runtimeSettings.validateOrigin(origin); + runtimeSettings.origin = origin; }); /** diff --git a/ui/desktop/electron-app/src/utils/sanitizer.js b/ui/desktop/electron-app/src/utils/sanitizer.js index 582f2532c..9e635b0d5 100644 --- a/ui/desktop/electron-app/src/utils/sanitizer.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -21,13 +21,10 @@ module.exports = { urlValidate: (str) => { try { const url = new URL(str); - return url.href; + return `${url.protocol}://${url.host}`; } catch (e) { // Parse error - throw new Error(` - Could not invoke command: - input contained unsafe url. - `); + throw new Error(`URL ${str} could not be validated.`); } }, }; From f2f5a46cdfc6d5eddcae098d092bb5cfa8ab3538 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 15:08:06 -0400 Subject: [PATCH 32/39] refactor: Add conditional chaining to session-manager call --- ui/desktop/electron-app/src/index.js | 2 +- ui/desktop/electron-app/src/models/session-manager.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 07a9a32d2..94508b0dd 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -200,7 +200,7 @@ app.on('before-quit', (event) => { case 0: sessionManager.stopAll(); break; - case 1: + default: event.preventDefault(); break; } diff --git a/ui/desktop/electron-app/src/models/session-manager.js b/ui/desktop/electron-app/src/models/session-manager.js index 3bcd7938f..b6e0d0939 100644 --- a/ui/desktop/electron-app/src/models/session-manager.js +++ b/ui/desktop/electron-app/src/models/session-manager.js @@ -31,7 +31,7 @@ class SessionManager { */ stopById(session_id) { const session = this.#sessions.find((session) => session.id === session_id); - return session && session.stop(); + return session?.stop?.(); } stopAll() { From ce9503fc5b2e652716206a58b30caec19aab747a Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Fri, 14 May 2021 15:40:18 -0700 Subject: [PATCH 33/39] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Refactor=20table?= =?UTF-8?q?=20component=20with=20PR=20suggestions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add shrink style to cells and deprecate expand. Add Key/value style for table rows. Refactor storybook table page with all the changes. Refactor styles for new feature and simplification. --- .../components/rose/table/index.stories.mdx | 75 +++++++++++-------- .../components/rose/table/row/cell/index.js | 8 +- .../addon/components/rose/table/row/index.hbs | 2 +- .../styles/rose/components/table/_table.scss | 31 +++++--- .../rose/tests/dummy/app/templates/index.hbs | 51 +++++++------ .../components/rose/table/row-test.js | 11 +++ .../components/rose/table/row/cell-test.js | 10 +-- 7 files changed, 116 insertions(+), 72 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index 0a70f24e8..7c91f0de4 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -6,37 +6,39 @@ import { boolean, select } from '@storybook/addon-knobs'; # Table -A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. Rows can be visually hidden, cells can be expanded to fit full width, and center/right cell alignment is supported. +A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. Rows can be visually hidden, styled for a key/value visualization, cells can be shrinked to its content with, and center/right cell alignment is supported. -Cell expansion style should be applied to only one cell in a row. +Row `key/value` style should be applied per row. + +Cell shrink should be applied to all the cells in a row. Otherwise the bigger cell with shrink applied will determinate the width of the row. ```html Address - Host Set + Host Set Name - Description + Description 1.1.1.1 - host-set-1, host-set-2 - optional name - description + host-set-1, host-set-2 + optional name + description 1.1.1.1 - host-set-2 - optional name + host-set-2 + optional name description 1.1.1.1 - host-set-1 - optional name + host-set-1 + optional name description @@ -54,29 +56,29 @@ Cell expansion style should be applied to only one cell in a row. - Address - Host Set - Name + Address + Host Set + Name Description - 1.1.1.1 - host-set-1, host-set-2 - optional name + 1.1.1.1 + host-set-1, host-set-2 + optional name description - 1.1.1.1 - host-set-2 - optional name + 1.1.1.1 + host-set-2 + optional name description - 1.1.1.1 - host-set-1 - optional name + 1.1.1.1 + host-set-1 + optional name description @@ -89,7 +91,7 @@ Cell expansion style should be applied to only one cell in a row. `, context: { hideHeaderRow: boolean('hideHeaderRow', false), - cellExpand: boolean('cellExpand', false), + cellShrink: boolean('cellShrink', false), style: select( 'style', { @@ -110,34 +112,34 @@ Cell expansion style should be applied to only one cell in a row. Key - Value + Value - + Key Value - + Key Value - + Key Value - + Key - + Add - + Key - + Remove @@ -152,5 +154,14 @@ Cell expansion style should be applied to only one cell in a row. `, + context: { + style: select( + 'style', + { + 'default': '', + 'key-value': 'key-value' + } + ) + } }} diff --git a/addons/rose/addon/components/rose/table/row/cell/index.js b/addons/rose/addon/components/rose/table/row/cell/index.js index 4d0a04897..588c072cf 100644 --- a/addons/rose/addon/components/rose/table/row/cell/index.js +++ b/addons/rose/addon/components/rose/table/row/cell/index.js @@ -5,15 +5,15 @@ import { computed } from '@ember/object'; export default Component.extend({ layout, tagName: 'td', - classNameBindings: ['cellTypeStyle', 'cellExpand', 'cellAlignmentStyle'], + classNameBindings: ['cellTypeStyle', 'cellShrink', 'cellAlignmentStyle'], cellTypeStyle: computed(function () { return this.tagName.match('th') ? `rose-table-header-cell` : 'rose-table-cell'; }), - cellExpand: computed(function () { - if (this.expand) { - return 'rose-table-cell-expand' + cellShrink: computed(function () { + if (this.shrink) { + return 'rose-table-cell-shrink' } }), cellAlignmentStyle: computed(function () { diff --git a/addons/rose/addon/components/rose/table/row/index.hbs b/addons/rose/addon/components/rose/table/row/index.hbs index 4b511d869..65fb90e81 100644 --- a/addons/rose/addon/components/rose/table/row/index.hbs +++ b/addons/rose/addon/components/rose/table/row/index.hbs @@ -1,4 +1,4 @@ -
+ {{yield (hash cell=(component 'rose/table/row/cell') headerCell=(component 'rose/table/row/cell' tagName='th') diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index 4f6ea31d8..a3c3bfcdc 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -10,10 +10,20 @@ text-align: left; } + .rose-table-row-key-value { + td:nth-child(2) { + display: flex; + input { + width: 100%; + margin-right: sizing.rems(s); + } + button { margin-right: 0; } + } + } + .rose-table-cell, .rose-table-header-cell { vertical-align: middle; - white-space: nowrap; &.align-right { text-align: right; } &.align-center { text-align: center; } @@ -39,20 +49,23 @@ } } - // Grow to full available width - .rose-table-cell-expand { - width: 100%; - - & > * { - vertical-align: inherit; - margin: 0; - } + .rose-table-cell-shrink { + width: 1%; + white-space: nowrap; } [class*='rose-button'] { margin-bottom: 0; } + .rose-table-cell { + .rose-form-input-field { + margin: 0; + width: inherit; + vertical-align: inherit; + } + } + &.rose-table-condensed { .rose-table-cell { @include type.type(s); diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index c7c91cea6..f162f9948 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -23,24 +23,24 @@ -

Condensed Table With Expanded Cells

+

Condensed Table with Shrink Cells

- Address - Host Set - Name - Description + Address + Host Set + Name + Description 1.1.1.1 - host-set-1, host-set-2 - + host-set-1, host-set-2 + Remove - description + description @@ -50,12 +50,12 @@ -

Condensed Table With Right and Center aligned cells

+

Condensed Table with Right and Center aligned cells

- Address - Host Set + Address + Host Set Name Description @@ -79,29 +79,38 @@ - Key - Value + Key + Value - + Key Value - + Key Value - + Key Value - + Key - + - - Remove + + Remove + + + + + Key + + + + Add @@ -120,7 +129,7 @@ Address Host Set Name - Description + Description diff --git a/addons/rose/tests/integration/components/rose/table/row-test.js b/addons/rose/tests/integration/components/rose/table/row-test.js index 7e0774504..c61552778 100644 --- a/addons/rose/tests/integration/components/rose/table/row-test.js +++ b/addons/rose/tests/integration/components/rose/table/row-test.js @@ -7,17 +7,20 @@ module('Integration | Component | rose/table/row', function (hooks) { setupRenderingTest(hooks); test('it renders', async function (assert) { + assert.expect(2); await render(hbs``); assert.ok(find('tr')); assert.ok(find('.rose-table-row')); }); test('it renders with attributes', async function (assert) { + assert.expect(1); await render(hbs``); assert.ok(find('#row')); }); test('it renders a cell', async function (assert) { + assert.expect(2); await render(hbs` `); @@ -26,6 +29,7 @@ module('Integration | Component | rose/table/row', function (hooks) { }); test('it renders a header cell', async function (assert) { + assert.expect(2); await render(hbs` `); @@ -34,7 +38,14 @@ module('Integration | Component | rose/table/row', function (hooks) { }); test('it can be hidden', async function (assert) { + assert.expect(1); await render(hbs``); assert.ok(find('.rose-table-row-visually-hidden')); }); + + test('it can be key/value style', async function (assert) { + assert.expect(1); + await render(hbs``); + assert.ok(find('.rose-table-row-key-value')); + }); }); diff --git a/addons/rose/tests/integration/components/rose/table/row/cell-test.js b/addons/rose/tests/integration/components/rose/table/row/cell-test.js index 65fa7083f..4b8c3332d 100644 --- a/addons/rose/tests/integration/components/rose/table/row/cell-test.js +++ b/addons/rose/tests/integration/components/rose/table/row/cell-test.js @@ -30,16 +30,16 @@ module('Integration | Component | rose/table/row/cell', function (hooks) { assert.notOk(find('.rose-table-cell')); }); - test('it adds expand style class', async function (assert) { + test('it adds shrink style class', async function (assert) { assert.expect(1); - await render(hbs``); - assert.ok(find('.rose-table-cell-expand')); + await render(hbs``); + assert.ok(find('.rose-table-cell-shrink')); }); - test('it does not add expand style class', async function (assert) { + test('it does not add expand style class in default cells', async function (assert) { assert.expect(1); await render(hbs``); - assert.notOk(find('.rose-table-cell-expand')); + assert.notOk(find('.rose-table-cell-shrink')); }); test('it renders with align center in a header cell', async function (assert) { From ee29f3c5991dc57cc46ccabb1f8de531f5cc362b Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Mon, 17 May 2021 16:33:34 -0400 Subject: [PATCH 34/39] refactor: change isActive to isRunning and related language to reduce confusion with the API concept of session status "active" --- ui/desktop/electron-app/src/models/session-manager.js | 6 +++--- ui/desktop/electron-app/src/models/session.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ui/desktop/electron-app/src/models/session-manager.js b/ui/desktop/electron-app/src/models/session-manager.js index b6e0d0939..311d86899 100644 --- a/ui/desktop/electron-app/src/models/session-manager.js +++ b/ui/desktop/electron-app/src/models/session-manager.js @@ -4,11 +4,11 @@ class SessionManager { #sessions = []; /** - * Checks for active sessions + * Checks for running sessions * @returns {boolean} */ - get hasActiveSessions() { - return Boolean(this.#sessions.find((session) => session.isActive)); + get hasRunningSessions() { + return Boolean(this.#sessions.find((session) => session.isRunning)); } /** diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index 6ec9d3ce8..a59316f7f 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -36,7 +36,7 @@ class Session { /** * @return {boolean} */ - get isActive() { + get isRunning() { return !this.#process.killed; } @@ -60,12 +60,12 @@ class Session { */ stop() { return new Promise((resolve, reject) => { - if (this.isActive) { + if (this.isRunning) { this.#process.on('close', () => resolve()); this.#process.on('error', (e) => reject(e)); this.#process.kill(); } else { - // Do nothing when process isn't active + // Do nothing when process isn't running resolve(); } }); From 4fed7f3d94d75f28101d26d8ba7bd02e0217427f Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Mon, 17 May 2021 17:23:18 -0400 Subject: [PATCH 35/39] refactor: rename to "running" --- ui/desktop/electron-app/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 3b0b79f0d..0de2bfb54 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -188,7 +188,7 @@ app.on('ready', async () => { * Prompt for closing spawned processes */ app.on('before-quit', (event) => { - if (sessionManager.hasActiveSessions) { + if (sessionManager.hasRunningSessions) { const dialogOpts = { type: 'question', buttons: ['Close', 'Cancel'], From 545e7d5a7f2d8c032d954fad84625189526c37cb Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 19 May 2021 09:45:07 -0400 Subject: [PATCH 36/39] fix: import session manager correctly and reconstruct origin URL without extra colons --- ui/desktop/electron-app/src/index.js | 2 +- ui/desktop/electron-app/src/utils/sanitizer.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 0de2bfb54..1f220eb85 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -17,7 +17,7 @@ require('./ipc/handlers.js'); const runtimeSettings = require('./services/runtime-settings.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); -const sessionManager = require('./session/manager.js'); +const sessionManager = require('./services/session-manager.js'); const menu = require('./config/menu.js'); const appUpdater = require('./helpers/app-updater.js'); diff --git a/ui/desktop/electron-app/src/utils/sanitizer.js b/ui/desktop/electron-app/src/utils/sanitizer.js index 9e635b0d5..5dbc5efa9 100644 --- a/ui/desktop/electron-app/src/utils/sanitizer.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -21,7 +21,7 @@ module.exports = { urlValidate: (str) => { try { const url = new URL(str); - return `${url.protocol}://${url.host}`; + return `${url.protocol}//${url.host}`; } catch (e) { // Parse error throw new Error(`URL ${str} could not be validated.`); From e86a4fc27dc264aad56e19b977fdb897d470a703 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 19 May 2021 13:04:40 -0400 Subject: [PATCH 37/39] chore: bump CLI version and update changelog --- ui/desktop/CHANGELOG.md | 2 +- ui/desktop/electron-app/config/cli.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/desktop/CHANGELOG.md b/ui/desktop/CHANGELOG.md index 93e8fb472..42bd5a07d 100644 --- a/ui/desktop/CHANGELOG.md +++ b/ui/desktop/CHANGELOG.md @@ -2,7 +2,7 @@ Canonical reference for changes, improvements, and bugfixes for Boundary Desktop. -## Next +## v1.0.1 [CLI 0.2.2] (2021.05.19) ### New diff --git a/ui/desktop/electron-app/config/cli.js b/ui/desktop/electron-app/config/cli.js index 9ebc26ea7..6c7ecb6a4 100644 --- a/ui/desktop/electron-app/config/cli.js +++ b/ui/desktop/electron-app/config/cli.js @@ -4,7 +4,7 @@ const https = require('https'); const decompress = require('decompress'); const os = require('os'); -const artifactVersion = '0.2.0'; +const artifactVersion = '0.2.2'; const artifactDestination = path.resolve(__dirname, '..', 'cli'); const downloadArtifact = version => { From ab668d74eb717ca9d3c3a5d234d6690598e14457 Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Wed, 19 May 2021 11:16:30 -0700 Subject: [PATCH 38/39] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Delete=20key-value?= =?UTF-8?q?=20row=20and=20other=20tweaks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the recently introduced key-value class for rows, for they key-value lookalike, when editing we want to use 3 colums on the table rather than 2. Some style's tweaks to simplify --- .../components/rose/table/index.stories.mdx | 77 ++++++++----- .../styles/rose/components/table/_table.scss | 23 +--- .../rose/tests/dummy/app/templates/index.hbs | 109 +++++++----------- 3 files changed, 99 insertions(+), 110 deletions(-) diff --git a/addons/rose/addon/components/rose/table/index.stories.mdx b/addons/rose/addon/components/rose/table/index.stories.mdx index 7c91f0de4..444419be4 100644 --- a/addons/rose/addon/components/rose/table/index.stories.mdx +++ b/addons/rose/addon/components/rose/table/index.stories.mdx @@ -6,9 +6,7 @@ import { boolean, select } from '@storybook/addon-knobs'; # Table -A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. Rows can be visually hidden, styled for a key/value visualization, cells can be shrinked to its content with, and center/right cell alignment is supported. - -Row `key/value` style should be applied per row. +A table component to display data in structure of rows and columns. It supports `header`, `body` and `footer` optional definitions. Each definition allows for a table `row` to be defined with either a `cell` or `headerCell` table element. Rows can be visually hidden, and cells can be shrinked to its content with, and center/right cell alignment is supported. Cell shrink should be applied to all the cells in a row. Otherwise the bigger cell with shrink applied will determinate the width of the row. @@ -108,48 +106,71 @@ Cell shrink should be applied to all the cells in a row. Otherwise the bigger ce {{ template: hbs` - + - - Key - Value + + Key + Value - - Key - Value - - - Key - Value - - - Key - Value - - + Key - Add + + Remove + - + + Key + + + + Choose an option + Value 1 + Value 2 + Value 3 + + + + + Add + + + Key - Remove + + Add + + + + Key + + + + + Add + - - - Footer + + + Footer @@ -158,8 +179,8 @@ Cell shrink should be applied to all the cells in a row. Otherwise the bigger ce style: select( 'style', { - 'default': '', - 'key-value': 'key-value' + 'condensed': 'condensed', + 'default': '' } ) } diff --git a/addons/rose/addon/styles/rose/components/table/_table.scss b/addons/rose/addon/styles/rose/components/table/_table.scss index a3c3bfcdc..f9b7b9daf 100644 --- a/addons/rose/addon/styles/rose/components/table/_table.scss +++ b/addons/rose/addon/styles/rose/components/table/_table.scss @@ -10,15 +10,12 @@ text-align: left; } - .rose-table-row-key-value { - td:nth-child(2) { - display: flex; - input { - width: 100%; - margin-right: sizing.rems(s); - } - button { margin-right: 0; } - } + // This styles are add to stop the input to apply extra margin (default on forms) + .rose-form-input-field, + .rose-form-select, + .rose-form-input { + margin: 0; + width: 100%; } .rose-table-cell, @@ -58,14 +55,6 @@ margin-bottom: 0; } - .rose-table-cell { - .rose-form-input-field { - margin: 0; - width: inherit; - vertical-align: inherit; - } - } - &.rose-table-condensed { .rose-table-cell { @include type.type(s); diff --git a/addons/rose/tests/dummy/app/templates/index.hbs b/addons/rose/tests/dummy/app/templates/index.hbs index f162f9948..e82d02286 100644 --- a/addons/rose/tests/dummy/app/templates/index.hbs +++ b/addons/rose/tests/dummy/app/templates/index.hbs @@ -75,49 +75,72 @@ -

Condensed Table Key/Value Lookalike

+

Condensed Table Key/Value Lookalike (edit mode)

Key - Value + Value - - Key - Value - - - Key - Value - - - Key - Value - - + Key - - Remove + + + Remove + - + + Key + + + + Choose an option + Value 1 + Value 2 + Value 3 + + + + + Add + + + Key - - Add + + + Add + + + + Key + + + + + Add + - Footer + Footer @@ -147,50 +170,6 @@
-

Key/Value

- - - Key - Value - - - Key - Value - - - Key - Value - - - - - - Key - - - - - - Add - - - - - - - - Key - - - - - - Remove - - - - -

Table

From 3133997e78408b6036afa8f05509877f8ad199f6 Mon Sep 17 00:00:00 2001 From: Carlos Alcaide Date: Wed, 19 May 2021 13:15:04 -0700 Subject: [PATCH 39/39] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Deleting=20forgot?= =?UTF-8?q?=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a test that is not need it anymore after last changes --- .../tests/integration/components/rose/table/row-test.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/addons/rose/tests/integration/components/rose/table/row-test.js b/addons/rose/tests/integration/components/rose/table/row-test.js index c61552778..2a0c1f758 100644 --- a/addons/rose/tests/integration/components/rose/table/row-test.js +++ b/addons/rose/tests/integration/components/rose/table/row-test.js @@ -42,10 +42,4 @@ module('Integration | Component | rose/table/row', function (hooks) { await render(hbs``); assert.ok(find('.rose-table-row-visually-hidden')); }); - - test('it can be key/value style', async function (assert) { - assert.expect(1); - await render(hbs``); - assert.ok(find('.rose-table-row-key-value')); - }); });
{{@caption}}