Fix HTTP basic auth (fix #1219) (#1220)

Should resolve #1219, as well adds a manual test for basic auth as suggested.

Co-authored-by: Ronan Jouchet <ronan@jouchet.fr>

.github/manual-test

@@ -9,13 +9,46 @@ if ! command -v uname > /dev/null; then echo "Missing uname"; missingDeps=true;
 if ! command -v node > /dev/null; then echo "Missing node"; missingDeps=true; fi
 if [ "$missingDeps" = true ]; then exit 1; fi
 
+function launch_app() {
+  printf "\n***** Running app *****\n"
+  if [ "$(uname -s)" = "Darwin" ]; then
+    open -a "$1/$2-darwin-x64/$2.app"
+  else
+    "$1/$2-linux-x64/$2"
+  fi
+}
 
+function do_cleanup() {
+  if [ -n "1" ]; then
+  printf "\n***** Deleting test dir %s *****\n" "$1"
+  rm -rf "$1"
+  printf "\n"
+fi
+}
+
+function request_feedback() {
+  printf "\nDid everything work as expected? [yN] "
+  read -r response
+
+  do_cleanup $1
+
+  if [ "$response" != 'y' ]; then
+    echo "Back to fixing"
+    exit 1
+  else
+    echo "Yayyyyyyyyyyy"
+  fi
+}
+
+printf "\n***** Setting up and building *****\n"
 script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 nativefier_dir="$script_dir/.."
 pushd "$nativefier_dir"
+npm run build
 
 printf "\n***** Creating test dirs & resources *****\n"
 tmp_dir=$(mktemp -d -t nativefier-manual-test-XXXXX)
+name="app"
 resources_dir="$tmp_dir/resources"
 mkdir "$resources_dir"
 injected_css="$resources_dir/inject.css"
@@ -27,7 +60,7 @@ printf "\n***** Building test app *****\n"
 node ./lib/cli.js 'https://npmjs.com/' \
   --inject "$injected_css" \
   --inject "$injected_js" \
-  --name "app" \
+  --name "$name" \
   "$tmp_dir"
 
 printf "\n***** Test checklist *****
@@ -39,23 +72,56 @@ printf "\n***** Test checklist *****
 - Console: no Electron runtime deprecation warnings/error logged
 "
 
-printf "\n***** Running app *****\n"
-if [ "$(uname -s)" = "Darwin" ]; then
-  open -a "$tmp_dir/app-darwin-x64/app.app"
-else
-  "$tmp_dir/app-linux-x64/app"
-fi
+launch_app $tmp_dir $name
 
-printf "\nDid everything work as expected? [yN] "
-read -r response
-if [ "$response" != 'y' ]; then
-  echo "Back to fixing"
-  exit 1
-else
-  echo "Yayyyyyyyyyyy"
-fi
+request_feedback $tmp_dir
 
-if [ -n "$tmp_dir" ]; then
-  printf "\n***** Deleting test dir %s *****\n" "$tmp_dir"
-  rm -rf "$tmp_dir"
-fi
+printf "\n***** Creating test dirs & resources *****\n"
+tmp_dir=$(mktemp -d -t nativefier-manual-test-auth-XXXXX)
+name="app"
+
+printf "\n***** Building test app *****\n"
+node ./lib/cli.js 'http://httpbin.org/basic-auth/foo/bar' \
+  --basic-auth-username foo \
+  --basic-auth-password bar \
+  --name "$name" \
+  "$tmp_dir"
+
+printf "\n***** Test checklist *****
+- Was successfully logged in via HTTP Basic Auth. Should see:
+    {
+      \"authenticated\": true, 
+      \"user\": \"foo\"
+    }
+- Console: no Electron runtime deprecation warnings/error logged
+"
+
+launch_app $tmp_dir $name
+
+request_feedback $tmp_dir
+
+
+printf "\n***** Creating test dirs & resources *****\n"
+tmp_dir=$(mktemp -d -t nativefier-manual-test-auth-prompt-XXXXX)
+name="app"
+
+printf "\n***** Building test app *****\n"
+node ./lib/cli.js 'http://httpbin.org/basic-auth/foo/bar' \
+  --name "$name" \
+  "$tmp_dir"
+
+printf "\n***** Test checklist *****
+- Should get a login window to access page.
+    - Username is \"foo\"
+    - Password is \"bar\".
+- Post login, you should see:
+    {
+      \"authenticated\": true, 
+      \"user\": \"foo\"
+    }
+- Console: no Electron runtime deprecation warnings/error logged
+"
+
+launch_app $tmp_dir $name
+
+request_feedback $tmp_dir

app/src/components/loginWindow.ts

@@ -8,7 +8,7 @@ export async function createLoginWindow(
   loginCallback,
   parent?: BrowserWindow,
 ): Promise<BrowserWindow> {
-  log.debug('createLoginWindow', loginCallback, parent);
+  log.debug('createLoginWindow', { loginCallback, parent });
 
   const loginWindow = new BrowserWindow({
     parent,
@@ -18,6 +18,7 @@ export async function createLoginWindow(
     resizable: false,
     webPreferences: {
       nodeIntegration: true, // TODO work around this; insecure
+      contextIsolation: false, // https://github.com/electron/electron/issues/28017
     },
   });
   await loginWindow.loadURL(
@@ -25,6 +26,7 @@ export async function createLoginWindow(
   );
 
   ipcMain.once('login-message', (event, usernameAndPassword) => {
+    log.debug('login-message', { event, username: usernameAndPassword[0] });
     loginCallback(usernameAndPassword[0], usernameAndPassword[1]);
     loginWindow.close();
   });

app/src/main.ts

@@ -37,6 +37,8 @@ if (process.argv.indexOf('--verbose') > -1) {
   process.traceProcessWarnings = true;
 }
 
+let mainWindow: BrowserWindow;
+
 const appArgs = JSON.parse(fs.readFileSync(APP_ARGS_FILE_PATH, 'utf8'));
 
 log.debug('appArgs', appArgs);
@@ -229,67 +231,61 @@ if (appArgs.widevine) {
   });
 }
 
-function initAppWindowEvents(mainWindow: BrowserWindow) {
-  app.on('activate', (event, hasVisibleWindows) => {
-    log.debug('app.activate', { event, hasVisibleWindows });
-    if (isOSX()) {
-      // this is called when the dock is clicked
-      if (!hasVisibleWindows) {
+app.on('activate', (event, hasVisibleWindows) => {
+  log.debug('app.activate', { event, hasVisibleWindows });
+  if (isOSX()) {
+    // this is called when the dock is clicked
+    if (!hasVisibleWindows) {
+      mainWindow.show();
+    }
+  }
+});
+
+// quit if singleInstance mode and there's already another instance running
+const shouldQuit = appArgs.singleInstance && !app.requestSingleInstanceLock();
+if (shouldQuit) {
+  app.quit();
+} else {
+  app.on('second-instance', () => {
+    log.debug('app.second-instance');
+    if (mainWindow) {
+      if (!mainWindow.isVisible()) {
+        // try
         mainWindow.show();
       }
-    }
-  });
-
-  // quit if singleInstance mode and there's already another instance running
-  const shouldQuit = appArgs.singleInstance && !app.requestSingleInstanceLock();
-  if (shouldQuit) {
-    app.quit();
-  } else {
-    app.on('second-instance', () => {
-      log.debug('app.second-instance');
-      if (mainWindow) {
-        if (!mainWindow.isVisible()) {
-          // try
-          mainWindow.show();
-        }
-        if (mainWindow.isMinimized()) {
-          // minimized
-          mainWindow.restore();
-        }
-        mainWindow.focus();
+      if (mainWindow.isMinimized()) {
+        // minimized
+        mainWindow.restore();
       }
-    });
-  }
-
-  app.on('new-window-for-tab', () => {
-    log.debug('app.new-window-for-tab');
-    if (mainWindow) {
-      mainWindow.emit('new-tab');
-    }
-  });
-
-  app.on('login', (event, webContents, request, authInfo, callback) => {
-    log.debug('app.login', { event, request });
-    // for http authentication
-    event.preventDefault();
-
-    if (
-      appArgs.basicAuthUsername !== null &&
-      appArgs.basicAuthPassword !== null
-    ) {
-      callback(appArgs.basicAuthUsername, appArgs.basicAuthPassword);
-    } else {
-      createLoginWindow(callback, mainWindow).catch((err) =>
-        log.error('createLoginWindow ERROR', err),
-      );
+      mainWindow.focus();
     }
   });
 }
 
-async function onReady(): Promise<void> {
-  const mainWindow = await createMainWindow(appArgs, setDockBadge);
+app.on('new-window-for-tab', () => {
+  log.debug('app.new-window-for-tab');
+  if (mainWindow) {
+    mainWindow.emit('new-tab');
+  }
+});
 
-  initAppWindowEvents(mainWindow);
+app.on('login', (event, webContents, request, authInfo, callback) => {
+  log.debug('app.login', { event, request });
+  // for http authentication
+  event.preventDefault();
+
+  if (appArgs.basicAuthUsername && appArgs.basicAuthPassword) {
+    callback(appArgs.basicAuthUsername, appArgs.basicAuthPassword);
+  } else {
+    createLoginWindow(callback, mainWindow).catch((err) =>
+      log.error('createLoginWindow ERROR', err),
+    );
+  }
+});
+
+async function onReady(): Promise<void> {
+  // Warning: `mainWindow` below is the *global* unique `mainWindow`, created at init time
+  mainWindow = await createMainWindow(appArgs, setDockBadge);
 
   createTrayIcon(appArgs, mainWindow);