From ce7cdba0bc2a25133e982b1af2d98dde1c3c17cb Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 12:27:16 -0400 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 542b469ad8deb769a16090f7270d79292e91c6f8 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Wed, 12 May 2021 15:46:40 -0400 Subject: [PATCH 4/8] 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 f2f5a46cdfc6d5eddcae098d092bb5cfa8ab3538 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 15:08:06 -0400 Subject: [PATCH 5/8] 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 ee29f3c5991dc57cc46ccabb1f8de531f5cc362b Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Mon, 17 May 2021 16:33:34 -0400 Subject: [PATCH 6/8] 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 7/8] 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 8/8] 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.`);