[feat] report missing polkit properly from main UI

also refactor and move polkit_agent so that it does not depend on having
bitmask on the path.

7 files changed, 70 insertions, 81 deletions

diff --git a/src/leap/bitmask/vpn/_checks.py b/src/leap/bitmask/vpn/_checks.py
index 9586d096..c9e40f57 100644
--- a/src/leap/bitmask/vpn/_checks.py
+++ b/src/leap/bitmask/vpn/_checks.py
@@ -18,10 +18,10 @@ class ImproperlyConfigured(Exception):
 
 
 def get_failure_for(provider):
-    if not _has_valid_cert(provider):
-        raise ImproperlyConfigured('Missing VPN certificate')
     if IS_LINUX and not is_pkexec_in_system():
         raise NoPkexecAvailable()
+    if not _has_valid_cert(provider):
+        raise ImproperlyConfigured('Missing VPN certificate')
 
 
 def is_service_ready(provider):
@@ -35,8 +35,11 @@ def is_service_ready(provider):
 
 def cert_expires(provider):
     path = get_vpn_cert_path(provider)
-    with open(path, 'r') as f:
-        cert = f.read()
+    try:
+        with open(path, 'r') as f:
+            cert = f.read()
+    except IOError:
+        return None
     _, to = get_cert_time_boundaries(cert)
     expiry_date = datetime.fromtimestamp(mktime(to))
     return expiry_date
diff --git a/src/leap/bitmask/vpn/autostart.py b/src/leap/bitmask/vpn/autostart.py
index 43abfdf5..af7b3669 100644
--- a/src/leap/bitmask/vpn/autostart.py
+++ b/src/leap/bitmask/vpn/autostart.py
@@ -34,7 +34,10 @@ Terminal=false
                 with open(autostart_file, 'w') as f:
                     f.write(AUTOSTART)
             elif status == 'off':
-                os.unlink(autostart_file)
+                try:
+                    os.unlink(autostart_file)
+                except OSError:
+                    pass
 
 if IS_MAC:
 
diff --git a/src/leap/bitmask/vpn/helpers/__init__.py b/src/leap/bitmask/vpn/helpers/__init__.py
index 8f8c1227..69b34e00 100644
--- a/src/leap/bitmask/vpn/helpers/__init__.py
+++ b/src/leap/bitmask/vpn/helpers/__init__.py
@@ -16,6 +16,7 @@ if IS_LINUX:
     from leap.bitmask.vpn.constants import OPENVPN_SYSTEM, OPENVPN_LOCAL
     from leap.bitmask.vpn.constants import POLKIT_SYSTEM, POLKIT_LOCAL
     from leap.bitmask.vpn.privilege import is_pkexec_in_system
+    from leap.bitmask.vpn.privilege import LinuxPolicyChecker
 
     def install():
         helper_from = _config.get_bitmask_helper_path()
@@ -40,6 +41,17 @@ if IS_LINUX:
         remove(POLKIT_LOCAL)
         remove(OPENVPN_LOCAL)
 
+    def privcheck(timeout=5):
+        has_pkexec = is_pkexec_in_system()
+        running = LinuxPolicyChecker.is_up()
+        if not running:
+            try:
+                LinuxPolicyChecker.get_usable_pkexec(timeout=timeout)
+                running = LinuxPolicyChecker.is_up()
+            except Exception:
+                running = False
+        return has_pkexec and running
+
     def check():
         helper = _is_up_to_date(_config.get_bitmask_helper_path(),
                                 BITMASK_ROOT_LOCAL,
@@ -51,7 +63,7 @@ if IS_LINUX:
                    _is_up_to_date(_config.get_bitmask_openvpn_path(),
                                   OPENVPN_LOCAL, ""))
 
-        return is_pkexec_in_system() and helper and polkit and openvpn
+        return helper and polkit and openvpn
 
     def _is_up_to_date(src, local, system):
         if src is None or not access(src, R_OK):
@@ -72,6 +84,9 @@ elif IS_MAC:
         # XXX check if bitmask-helper is running
         return True
 
+    def privcheck():
+        return True
+
 
 def digest(path):
     with open(path, 'r') as f:
diff --git a/src/leap/bitmask/vpn/launchers/linux.py b/src/leap/bitmask/vpn/launchers/linux.py
index b0cd4f7f..f3b46a42 100644
--- a/src/leap/bitmask/vpn/launchers/linux.py
+++ b/src/leap/bitmask/vpn/launchers/linux.py
@@ -176,7 +176,7 @@ class LinuxVPNLauncher(VPNLauncher):
 
         if os.getuid() != 0:
             policyChecker = LinuxPolicyChecker()
-            pkexec = policyChecker.maybe_pkexec()
+            pkexec = policyChecker.get_usable_pkexec()
             if pkexec:
                 command.insert(0, first(pkexec))
 
diff --git a/src/leap/bitmask/vpn/helpers/linux/polkit_agent.py b/src/leap/bitmask/vpn/polkit.py
index 5ca1a2f0..ae3f9000 100644
--- a/src/leap/bitmask/vpn/helpers/linux/polkit_agent.py
+++ b/src/leap/bitmask/vpn/polkit.py
@@ -21,9 +21,6 @@ Daemonizes polkit authentication agent.
 
 import os
 import subprocess
-import sys
-
-import daemon
 
 
 POLKIT_PATHS = (
@@ -40,6 +37,18 @@ POLKIT_PATHS = (
     # do you know some we're still missing? :)
 )
 
+POLKIT_PROC_NAMES = (
+    'polkit-gnome-authentication-agent-1',
+    'polkit-kde-authentication-agent-1',
+    'polkit-mate-authentication-agent-1',
+    'lxpolkit',
+    'lxsession',
+    'gnome-shell',
+    'gnome-flashback',
+    'fingerprint-polkit-agent',
+    'xfce-polkit',
+)
+
 
 # TODO write tests for this piece.
 def _get_polkit_agent():
@@ -51,38 +60,16 @@ def _get_polkit_agent():
     for polkit in POLKIT_PATHS:
         if os.path.isfile(polkit):
             return polkit
-
     return None
 
 
-def _launch_agent():
-    """
-    Launch a polkit authentication agent on a subprocess.
-    """
-    polkit_agent = _get_polkit_agent()
-
-    if polkit_agent is None:
-        print("No usable polkit was found.")
-        return
-
-    print('Launching polkit auth agent')
-    try:
-        # XXX fix KDE launch. See: #3755
-        subprocess.call(polkit_agent)
-    except Exception as e:
-        print('Error launching polkit authentication agent %r' % (e, ))
-
-
 def launch():
     """
     Launch a polkit authentication agent as a daemon.
     """
-    with daemon.DaemonContext():
-        _launch_agent()
+    agent = _get_polkit_agent()
+    subprocess.call("(setsid {polkit} &)".format(polkit=agent), shell=True)
 
 
 if __name__ == "__main__":
-    if '--nodaemon' in sys.argv:
-        _launch_agent()
-    else:
-        launch()
+    launch()
diff --git a/src/leap/bitmask/vpn/privilege.py b/src/leap/bitmask/vpn/privilege.py
index c7296878..1856ec8c 100644
--- a/src/leap/bitmask/vpn/privilege.py
+++ b/src/leap/bitmask/vpn/privilege.py
@@ -22,7 +22,6 @@ are operative under this client run.
 
 import commands
 import os
-import subprocess
 import psutil
 import time
 
@@ -30,8 +29,8 @@ from twisted.logger import Logger
 from twisted.python.procutils import which
 
 from leap.bitmask.util import STANDALONE, here
-
 from .constants import IS_LINUX
+from . import polkit
 
 log = Logger()
 
@@ -97,7 +96,7 @@ class LinuxPolicyChecker(object):
                 else self.LINUX_POLKIT_FILE)
 
     @classmethod
-    def maybe_pkexec(self):
+    def get_usable_pkexec(self, timeout=20):
         """
         Checks whether pkexec is available in the system, and
         returns the path if found.
@@ -117,20 +116,17 @@ class LinuxPolicyChecker(object):
             self.launch()
             seconds = 0
             while not self.is_up():
-                if seconds >= 20:
+                if seconds >= timeout:
                     log.warn('No polkit auth agent found. pkexec ' +
                              'will use its own auth agent.')
                     raise NoPolkitAuthAgentAvailable()
-
-                # XXX: sleep()!!!! we should do it the twisted way
                 time.sleep(1)
                 seconds += 1
 
-        pkexec_possibilities = which(self.PKEXEC_BIN)
-        if not pkexec_possibilities:
+        pkexec_choices = which(self.PKEXEC_BIN)
+        if not pkexec_choices:
             raise Exception("We couldn't find pkexec")
-
-        return pkexec_possibilities
+        return pkexec_choices
 
     @classmethod
     def launch(self):
@@ -138,17 +134,7 @@ class LinuxPolicyChecker(object):
         Tries to launch polkit agent.
         """
         if not self.is_up():
-            try:
-                # We need to quote the command because subprocess call
-                # will do "sh -c 'foo'", so if we do not quoute it we'll end
-                # up with a invocation to the python interpreter. And that
-                # is bad.
-                log.debug('Trying to launch polkit agent')
-                subprocess.call(
-                    ["python -m leap.bitmask.vpn.helpers.linux.polkit_agent"],
-                    shell=True)
-            except Exception:
-                log.failure('Error while launching vpn')
+            polkit.launch()
 
     @classmethod
     def is_up(self):
@@ -162,25 +148,12 @@ class LinuxPolicyChecker(object):
         # polkit-agent, it uses a polkit-agent within its own process so we
         # can't ps-grep a polkit process, we can ps-grep gnome-shell itself.
 
-        polkit_options = (
-            'polkit-gnome-authentication-agent-1',
-            'polkit-kde-authentication-agent-1',
-            'polkit-mate-authentication-agent-1',
-            'lxpolkit',
-            'lxsession',
-            'gnome-shell',
-            'gnome-flashback',
-            'fingerprint-polkit-agent',
-            'xfce-polkit',
-        )
-
-        is_running = False
+        running = False
         for proc in psutil.process_iter():
-            if any((polkit in proc.name() for polkit in polkit_options)):
-                is_running = True
+            if any((pk in proc.name() for pk in polkit.POLKIT_PROC_NAMES)):
+                running = True
                 break
-
-        return is_running
+        return running
 
 
 def is_pkexec_in_system():
diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py
index c12f66dd..0ca0f566 100644
--- a/src/leap/bitmask/vpn/service.py
+++ b/src/leap/bitmask/vpn/service.py
@@ -40,6 +40,7 @@ from leap.bitmask.vpn._checks import (
 
 from leap.bitmask.vpn import privilege, helpers
 from leap.bitmask.vpn import autostart
+from leap.bitmask.vpn.constants import IS_LINUX
 from leap.common.config import get_path_prefix
 from leap.common.files import check_and_fix_urw_only
 from leap.common.events import catalog, emit_async
@@ -101,8 +102,8 @@ class VPNService(HookableService):
     def startService(self):
         # TODO trigger a check for validity of the certificates,
         # and schedule a re-download if needed.
-        # TODO start a watchDog service (to push status events)
         super(VPNService, self).startService()
+
         if self._autostart:
             self.start_vpn()
 
@@ -117,6 +118,7 @@ class VPNService(HookableService):
     def start_vpn(self, domain=None):
         self._cfg.set('autostart', True)
         autostart.autostart_app('on')
+
         if self.do_status()['status'] == 'on':
             exc = Exception('VPN already started')
             exc.expected = True
@@ -134,9 +136,6 @@ class VPNService(HookableService):
             exc.expected = True
             raise exc
 
-        # XXX we can signal status to frontend, use
-        # get_failure_for(provider) -- no polkit, etc.
-
         fw_ok = self._firewall.start()
         if not fw_ok:
             raise Exception('Could not start firewall')
@@ -219,12 +218,21 @@ class VPNService(HookableService):
 
     def do_check(self, domain=None):
         """Check whether the VPN Service is properly configured,
-        and can be started"""
-        ret = {'installed': helpers.check()}
+        and can be started. This returns info about the helpers being
+        installed, a polkit agent being present, and optionally a valid
+        certificate present for a domain."""
+        hashelpers = helpers.check()
+        privcheck = helpers.privcheck(timeout=5)
+        ret = {'installed': hashelpers, 'privcheck': privcheck}
+        if not privcheck:
+            if IS_LINUX:
+                ret['error'] = 'nopolkit'
         if domain:
             ret['vpn_ready'] = is_service_ready(domain)
-            expiry = cert_expires(domain).strftime('%Y-%m-%dT%H:%M:%SZ')
-            ret['cert_expires'] = expiry
+            expiry = cert_expires(domain)
+            if expiry:
+                expiry_ts = expiry.strftime('%Y-%m-%dT%H:%M:%SZ')
+                ret['cert_expires'] = expiry_ts
         return ret
 
     @defer.inlineCallbacks