[pkg] improve osx installer

- install into global /Applications
- document how to troubleshoot helper
- uninstall app is visible on top-level folder
- improve detection of running processes for old and new binaries

- Closes: #441
- Closes: #445
- Closes: #435

10 files changed, 141 insertions, 85 deletions

diff --git a/Makefile b/Makefile
index 2ee11d2..0cd6c88 100644
--- a/Makefile
+++ b/Makefile
@@ -6,6 +6,7 @@
 .PHONY: all get build icon locales generate_locales clean check_qtifw HAS-qtifw relink_vendor
 
 XBUILD ?= no
+RELEASE ?= no
 SKIP_CACHECK ?= no
 VENDOR_PATH ?= providers
 APPNAME ?= $(shell VENDOR_PATH=${VENDOR_PATH} branding/scripts/getparam appname | tail -n 1)
@@ -34,10 +35,16 @@ endif
 
 QTBUILD = build/qt
 INSTALLER = build/installer
-INST_DATA = ${INSTALLER}/packages/bitmaskvpn/data/
 OSX_CERT="Developer ID Application: LEAP Encryption Access Project"
-MACDEPLOYQT_OPTS = -appstore-compliant -qmldir=gui/qml -always-overwrite -codesign="${OSX_CERT}"
-	
+MACDEPLOYQT_OPTS = -appstore-compliant -always-overwrite -codesign="${OSX_CERT}"
+
+ifeq ($(PLATFORM), darwin)
+INST_ROOT =${INSTALLER}/packages/bitmaskvpn/data/
+INST_DATA = ${INST_ROOT}/${APPNAME}.app/
+else
+INST_DATA = ${INSTALLER}/packages/bitmaskvpn/data/
+endif
+
 SCRIPTS = branding/scripts
 TEMPLATES = branding/templates
 
@@ -114,7 +121,7 @@ ifeq ($(PLATFORM), windows)
 endif
 ifeq ($(VENDOR_PATH), providers)
 	@unlink providers/assets || true
-	@ln -s ${PROVIDER}/assets providers/assets
+	@ln -s ${PROVIDER}/assets providers/assets || true
 endif
 endif
 	@echo "============RELINK VENDOR============="
@@ -180,13 +187,18 @@ ifeq (${PLATFORM}, darwin)
 	@cp "${TEMPLATES}/osx/bitmask.pf.conf" ${INST_DATA}helper/bitmask.pf.conf
 	@cp "${TEMPLATES}/osx/client.up.sh" ${INST_DATA}/
 	@cp "${TEMPLATES}/osx/client.down.sh" ${INST_DATA}/
-	@cp "${TEMPLATES}/qtinstaller/osx-data/post-install.py" ${INST_DATA}/
-	@cp "${TEMPLATES}/qtinstaller/osx-data/uninstall.py" ${INST_DATA}/
-	@cp "${TEMPLATES}/qtinstaller/osx-data/se.leap.bitmask-helper.plist" ${INST_DATA}/
+	@cp "${TEMPLATES}/qtinstaller/osx-data/post-install.py" ${INST_ROOT}/
+	@cp "${TEMPLATES}/qtinstaller/osx-data/uninstall.py" ${INST_ROOT}/
+	@cp "${TEMPLATES}/qtinstaller/osx-data/se.leap.bitmask-helper.plist" ${INST_DATA}
 	@cp $(OPENVPN_BIN) ${INST_DATA}/openvpn.leap
 	@cp build/bin/${PLATFORM}/bitmask-helper ${INST_DATA}/
-	@echo "[+] Running macdeployqt"
-	@macdeployqt ${QTBUILD}/release/${PROVIDER}-vpn.app ${MACDEPLOYQT_OPTS}
+ifeq (${RELEASE}, yes)
+	@echo "[+] Running macdeployqt (release mode)"
+	@macdeployqt ${QTBUILD}/release/${PROVIDER}-vpn.app -qmldir=gui/qml ${MACDEPLOYQT_OPTS}
+else
+	@echo "[+] Running macdeployqt (debug mode)"
+	@macdeployqt ${QTBUILD}/release/${PROVIDER}-vpn.app -qmldir=gui/qml
+endif
 	@cp -r "${QTBUILD}/release/${TARGET}.app"/ ${INST_DATA}/
 endif
 ifeq (${PLATFORM}, windows)
diff --git a/branding/scripts/gen-qtinstaller b/branding/scripts/gen-qtinstaller
index 04ecaf7..9be097d 100755
--- a/branding/scripts/gen-qtinstaller
+++ b/branding/scripts/gen-qtinstaller
@@ -14,12 +14,12 @@ from provider import getProviderData
 OS_CONFIG = {
     'osx': """
     <!-- osx -->
-    <TargetDir>@ApplicationsDir@/$APPNAME.app</TargetDir>
+    <TargetDir>/Applications/$APPNAME</TargetDir>
     <StartMenuDir>$APPNAME.app</StartMenuDir>
     <RunProgram>open</RunProgram>
     <RunProgramDescription>Start $APPNAME now!</RunProgramDescription>
       <RunProgramArguments>
-        <Argument>@TargetDir@</Argument>
+        <Argument>@TargetDir@/$APPNAME.app</Argument>
       </RunProgramArguments>
 
     <WizardStyle>mac</WizardStyle>
diff --git a/branding/templates/qtinstaller/installer.pro b/branding/templates/qtinstaller/installer.pro
index 6aab843..0c05442 100644
--- a/branding/templates/qtinstaller/installer.pro
+++ b/branding/templates/qtinstaller/installer.pro
@@ -13,27 +13,11 @@ QMAKE_TARGET_BUNDLE_PREFIX = se.leap
 QMAKE_BUNDLE = $$TARGET
 QMAKE_EXTRA_COMPILERS += inst
 
-OTHER_FILES += \
-# watch out... it chokes with dashes in the path
-    packages/riseupvpn/meta/package.xml \
-    packages/riseupvpn/meta/install.js \
-    packages/riseupvpn/data/README.txt \
+# OTHER_FILES += \
 
 macx {
-    OTHER_FILES += "packages/riseupvpn/data/riseup-vpn.app"
-    OTHER_FILES += "packages/riseupvpn/data/bitmask-helper"
-    OTHER_FILES += "packages/riseupvpn/data/installer.py"
-    OTHER_FILES += "packages/riseupvpn/data/se.leap.bitmask-helper.plist"
-    OTHER_FILES += "packages/riseupvpn/data/openvpn.leap"
-    OTHER_FILES += "packages/riseupvpn/data/helper/bitmask.pf.conf"
-    OTHER_FILES += "packages/riseupvpn/data/client.up.sh"
-    OTHER_FILES += "packages/riseupvpn/data/client.down.sh"
 }
 linux {
-    OTHER_FILES += "packages/riseupvpn/data/riseup-vpn"
-    OTHER_FILES += "packages/riseupvpn/data/bitmask-helper"
 }
 win32{
-    OTHER_FILES += "packages/riseupvpn/data/riseup-vpn.exe"
-    OTHER_FILES += "packages/riseupvpn/data/helper.exe"
 }	
diff --git a/branding/templates/qtinstaller/osx-data/post-install.py b/branding/templates/qtinstaller/osx-data/post-install.py
index 2c15845..83c8370 100755
--- a/branding/templates/qtinstaller/osx-data/post-install.py
+++ b/branding/templates/qtinstaller/osx-data/post-install.py
@@ -5,15 +5,18 @@
 # This means that, for the time being, you can only install ONE of the BitmaskVPN derivatives at the same time.
 # This might change in the future.
 
+import glob
 import os
 import shutil
 import sys
 import subprocess
+import time
 
 HELPER = "bitmask-helper"
 HELPER_PLIST = "/Library/LaunchDaemons/se.leap.bitmask-helper.plist"
 
 _dir = os.path.dirname(os.path.realpath(__file__))
+_appdir = glob.glob('{}/*VPN.app'.format(_dir))[0]
 
 def main():
     log = open(os.path.join(_dir, 'post-install.log'), 'w')
@@ -21,10 +24,9 @@ def main():
 
     _id = os.getuid()
     if _id != 0:
-      err  = "error: need to run as root. UID: %s\n" % str(_id)
-      logErr(log, err)
-    
-    # failure: sys.exit(1)
+      err  = "ERROR: need to run as root. UID: %s\n" % str(_id)
+      log.write(err)
+      sys.exit(1)
     
     if isHelperRunning():
         log.write("Trying to stop bitmask-helper...\n")
@@ -48,22 +50,19 @@ def main():
     log.write('post-install script: done\n')
     sys.exit(0)
 
-
-def logErr(log, msg):
-    log.write(msg)
-    sys.exit(1)
-
 def isHelperRunning():
     ps = _getProcessList()
     return HELPER in ps 
 
 def unloadHelper():
     out = subprocess.call(["launchctl", "unload", HELPER_PLIST])
+    time.sleep(0.5)
     out2 = subprocess.call(["pkill", "-9", "bitmask-helper"])  # just in case
+    time.sleep(0.5)
     return out == 0
 
 def fixHelperOwner(log):
-    path = os.path.join(_dir, HELPER)
+    path = os.path.join(_appdir, HELPER)
     try:
         os.chown(path, 0, 0)
     except OSError as exc:
@@ -74,8 +73,9 @@ def fixHelperOwner(log):
 def copyLaunchDaemon():
     plist = "se.leap.bitmask-helper.plist"
     path = os.path.join(_dir, plist)
-    _p = _dir.replace("/", "\/")
-    subprocess.call(["sed", "-i.back", "s/PATH/%s/" % _p, path])
+    _p = os.path.join(_dir, _appdir)
+    _p2= _p.replace("/", "\/")
+    subprocess.call(["sed", "-i.back", "s/PATH/%s/" % _p2, path])
     shutil.copy(path, HELPER_PLIST)
 
 def launchHelper():
@@ -83,7 +83,7 @@ def launchHelper():
     return out == 0
 
 def grantPermissionsOnLogFolder():
-    helperDir = os.path.join(_dir, 'helper')
+    helperDir = os.path.join(_appdir, 'helper')
     try:
         os.makedirs(helperDir)
     except Exception:
diff --git a/branding/templates/qtinstaller/osx-data/uninstall.py b/branding/templates/qtinstaller/osx-data/uninstall.py
index d47acc5..ef012e4 100755
--- a/branding/templates/qtinstaller/osx-data/uninstall.py
+++ b/branding/templates/qtinstaller/osx-data/uninstall.py
@@ -6,6 +6,7 @@ import os
 import shutil
 import sys
 import subprocess
+import time
 
 HELPER = "bitmask-helper"
 HELPER_PLIST = "/Library/LaunchDaemons/se.leap.bitmask-helper.plist"
@@ -21,16 +22,19 @@ def main(stage="uninstall"):
     log.write("UID: %s\n" % str(_id))
     if int(_id) != 0:
       err  = "error: need to run as root. UID: %s\n" % str(_id)
-      logErr(log, err)
-    
-    # failure: sys.exit(1)
+      log.write(err)
+      sys.exit(1)
 
     log.write('Checking if helper is running\n')
     
     if isHelperRunning():
         log.write("Trying to stop bitmask-helper...\n")
 	# if this fail, we can check if the HELPER_PLIST is there
-        ok = unloadHelper()
+        try:
+            ok = unloadHelper()
+        except Exception as exc:
+            log.write('error: %v' % exc)
+        time.sleep(0.5)
         log.write("success: %s \n" % str(ok))
 
     log.write("Removing LaunchDaemon\n")
@@ -41,14 +45,14 @@ def main(stage="uninstall"):
     log.write(stage + ' script: done\n')
     sys.exit(0)
 
-
-def logErr(log, msg):
-    log.write(msg)
-    sys.exit(1)
-
 def isHelperRunning():
-    ps = _getProcessList()
-    return HELPER in ps 
+    try:
+        pid = subprocess.check_output(['pgrep', HELPER])
+        if pid.strip() != '':
+            return True
+    except subprocess.CalledProcessError:
+        return False
+    return False
 
 def unloadHelper():
     out = subprocess.call(["launchctl", "unload", HELPER_PLIST])
@@ -58,17 +62,8 @@ def unloadHelper():
 def removeLaunchDaemon():
     return subprocess.call(["rm", "-f", HELPER_PLIST])
 
-def _getProcessList():
-    _out = []
-    output = subprocess.Popen(["ps", "-ceA"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    stdout, stderr = output.communicate()
-    for line  in stdout.split('\n'):
-        cmd = line.split(' ')[-1]
-        _out.append(cmd.strip())
-    return _out
-
 if __name__ == "__main__":
     stage="uninstall"
-    if len(sys.argv) > 2 and sys.argv[2] == "pre":
-        stage="pre-install"
+    if len(sys.argv) == 2 and sys.argv[1] == "pre":
+        stage="preinstall"
     main(stage)
diff --git a/branding/templates/qtinstaller/packages/bitmaskvpn/meta/install.js b/branding/templates/qtinstaller/packages/bitmaskvpn/meta/install.js
index 407ea39..dada918 100644
--- a/branding/templates/qtinstaller/packages/bitmaskvpn/meta/install.js
+++ b/branding/templates/qtinstaller/packages/bitmaskvpn/meta/install.js
@@ -58,6 +58,9 @@ function Component() {
 
 Component.prototype.createOperations = function ()
 {
+    if (systemInfo.productType === "osx") {
+        preInstallOSX();
+    }
     // This will actually install the files
     component.createOperations();
 
@@ -70,27 +73,13 @@ Component.prototype.createOperations = function ()
     if (systemInfo.productType === "windows") {
         postInstallWindows();
     } else if (systemInfo.productType === "osx") {
-        preInstallOSX();
+        uninstallOSX();
         postInstallOSX();
     } else {
         postInstallLinux();
     }
 }
 
-Component.prototype.installationFinished = function()
-{
-    console.log("DEBUG: running installationFinished");
-    if (installer.isInstaller() && installer.status == QInstaller.Success) {
-        var argList = ["-a", "@TargetDir@/$APPNAME.app"];
-        try {
-            installer.execute("touch", ["/tmp/install-finished"]);
-            installer.execute("open", argList);
-        } catch(e) {
-            console.log(e);
-        }
-    }
-}
-
 function postInstallWindows() {
     // TODO - check if we're on Windows10 or older, and use the needed tap-windows installer accordingly.
     console.log("Installing OpenVPN tap driver");
@@ -111,12 +100,26 @@ function postInstallWindows() {
 }
 
 function preInstallOSX() {
-    console.log("Pre-installation for OSX");
+    console.log("Pre-installation for OSX: check for running bitmask");
+    component.addOperation(
+	"Execute", "{1}", "pgrep", "bitmask-vpn$$", /* $$$$ is escaped by python template: the old app binary was called bitmask-vpn */ 
+	"errormessage=It seems that an old RiseupVPN client is running. Please exit the app and run this installer again.",
+    );
+    component.addOperation(
+	"Execute", "{1}", "pgrep", "bitmask$$", /* $$$$ is escaped by python template: we don't want to catch bitmask app */
+	"errormessage=It seems RiseupVPN, CalyxVPN or LibraryVPN are running. Please exit the app and run this installer again.",
+        "UNDOEXECUTE", "{1}", "pgrep", "bitmask$$", /* $$$$ is escaped: we dont want bitmask app */
+	"errormessage=It seems RiseupVPN, CalyxVPN or LibraryVPN are running. Please exit the app before trying to run the uninstaller again."
+    );
+}
+
+function uninstallOSX() {
+    console.log("Pre-installation for OSX: uninstall previous helpers");
     // TODO use installer filepath??
     component.addElevatedOperation(
 	"Execute", "{0}",
    	"@TargetDir@/uninstall.py", "pre",
-	"errormessage=There was an error during the pre-installation script, things might be broken. Please report this error and attach the pre-install.log file."
+	"errormessage=There was an error during the pre-installation script, things might be broken. Please report this error and attach /tmp/bitmask-uninstall.log"
     );
 }
 
diff --git a/docs/build.osx.rst b/docs/build.osx.rst
index 43d70ad..48836a7 100644
--- a/docs/build.osx.rst
+++ b/docs/build.osx.rst
@@ -9,11 +9,12 @@ have to sign and then notarize with their service. here are some notes that use
 ad-hoc targets in the main makefile, but we should keep an eye on any future
 integration of this process in the more or less official Qt tools (QTIFW).
 
-First, we build the regular installer
+First, we build the regular installer (use RELEASE=yes to do a codesign step
+with macqtdeploy, note that this increases build time considerably):
 
 ```
 make build
-make installer
+RELEASE=yes make installer
 make sign_installer
 ```
 
@@ -23,6 +24,18 @@ account. Contact their friendly support for more info, but don't expect they
 understand you do not really own any Apple Hardware. Sense of humor is not
 universal.
 
+Security -> App-specific passwords -> Generate
+If you need to revoke these tokens, click on 'view history'.
+
+https://appleid.apple.com/account/manage
+
+According to https://developer.apple.com/documentation/xcode/notarizing_macos_software_before_distribution/customizing_the_notarization_workflow:
+
+To avoid including your password as cleartext in a script, you can provide a
+reference to a keychain item, as shown in the previous example. This assumes
+the keychain holds a keychain item named AC_PASSWORD with an account value
+matching the username AC_USERNAME.
+
 ```
 export OSXAPPPASS=my-apple-app-pass
 make notarize_installer
@@ -37,7 +50,7 @@ altool[5281:91963] No errors uploading 'build/installer/RiseupVPN-installer-0.20
 RequestUUID = fe9a4324-bdcb-4c52-b857-f089dc904695
 
 OSXMORDORUID=fe9a4324-bdcb-4c52-b857-f089dc904695 make notarize_check
-xcrun altool --notarization-info fe9a4324-bdcb-4c52-b857-f089dc904695 -u "info@leap.se" -p nvaq-xdhq-wrho-ouuu
+xcrun altool --notarization-info fe9a4324-bdcb-4c52-b857-f089dc904695 -u "info@leap.se" -p my-apple-app-pass
 2020-12-11 22:21:59.940 altool[5787:96428] No errors getting notarization info.
 
    RequestUUID: fe9a4324-bdcb-4c52-b857-f089dc904695
diff --git a/docs/debug.rst b/docs/debug.rst
new file mode 100644
index 0000000..c746cf6
--- /dev/null
+++ b/docs/debug.rst
@@ -0,0 +1,47 @@
+Troubleshooting
+===============
+
+This document contains some useful debug information.
+
+OSX
+---
+If you're having troubles with old versions of RiseupVPN that did not have an
+uninstaller, and the new installer is not cleanly replacing the previous
+install, you might need to manually clean things up. You will need root access to
+stop the privileged helper.
+
+First, see if the helper is running:
+
+pgrep bitmask-helper
+
+To stop it:
+
+sudo launchctl unload /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+
+To start it:
+
+sudo launchctl load /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+sudo launchctl start /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+
+Check that it's running:
+
+pgrep bitmask-helper
+
+Manually check that the web api is running, and that it reports a version that matches what you currently have installed:
+
+curl http://localhost:7171/version
+
+Also, you can check that the path near the end of the file /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+matches the current path where you installed RiseupVPN.app.
+
+Cleaning up
+~~~~~~~~~~~
+If you have things messed up and you want to completely delete the bitmask-helper:
+
+sudo launchctl unload /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+sudo rm -rf /Library/LaunchDaemons/se.leap.bitmask-helper.plist
+
+Make sure that "pgrep bitmask-helper" does not return any pid.
+
+Now you can move /Applications/RiseupVPN.app to the Trash, and launch a
+recent installer to get a clean install.
diff --git a/gui/build.sh b/gui/build.sh
index 9dfcbda..0a6e681 100755
--- a/gui/build.sh
+++ b/gui/build.sh
@@ -111,7 +111,6 @@ function buildDefault {
     echo "[+] Done."
 }
 
-
 echo "[build.sh] VENDOR_PATH =" ${VENDOR_PATH}
 for i in "$@"
 do
@@ -136,4 +135,3 @@ then
 else
     buildDefault
 fi
-
diff --git a/gui/qml/main.qml b/gui/qml/main.qml
index 14f0776..e4cf956 100644
--- a/gui/qml/main.qml
+++ b/gui/qml/main.qml
@@ -9,6 +9,8 @@ ApplicationWindow {
     id: app
     visible: false
 
+    flags: Qt.FramelessWindowWint | Qt.WindowsStaysOnTopHint | Qt.Popup
+
     property var ctx
     property var loginDone
     property var allowEmptyPass
@@ -308,6 +310,8 @@ ApplicationWindow {
                 text: qsTr("About…")
                 onTriggered: {
                     about.visible = true
+                    app.focus = true
+                    requestActivate()
                 }
             }