Merge remote-tracking branch 'refs/remotes/kali/bug/fix-bad-eip-status' into develop

5 files changed, 49 insertions, 5 deletions

diff --git a/changes/bug_5955_fix-vpnlauncher-error-ui b/changes/bug_5955_fix-vpnlauncher-error-ui
new file mode 100644
index 00000000..d322288e
--- /dev/null
+++ b/changes/bug_5955_fix-vpnlauncher-error-ui
@@ -0,0 +1 @@
+- Fix the handling of vpn launcher errors in the UI. Closes: #5955
diff --git a/src/leap/bitmask/backend/backend.py b/src/leap/bitmask/backend/backend.py
index 67ffe35a..37535f37 100644
--- a/src/leap/bitmask/backend/backend.py
+++ b/src/leap/bitmask/backend/backend.py
@@ -14,6 +14,11 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# FIXME this is missing module documentation. It would be fine to say a couple
+# of lines about the whole backend architecture.
+# TODO use txzmq bindings instead.
+
 import json
 import threading
 import time
@@ -38,7 +43,11 @@ class Backend(object):
     Backend server.
     Receives signals from backend_proxy and emit signals if needed.
     """
+    # XXX this should not be hardcoded. Make it configurable.
     PORT = '5556'
+
+    # XXX we might want to make this configurable per-platform,
+    # and use the most performant socket type on each one.
     BIND_ADDR = "tcp://127.0.0.1:%s" % PORT
 
     PING_INTERVAL = 2  # secs
@@ -67,6 +76,7 @@ class Backend(object):
         # Start an authenticator for this context.
         auth = ThreadAuthenticator(context)
         auth.start()
+        # XXX do not hardcode this here.
         auth.allow('127.0.0.1')
 
         # Tell authenticator to use the certificate in a directory
diff --git a/src/leap/bitmask/gui/eip_status.py b/src/leap/bitmask/gui/eip_status.py
index f283b148..1e764a8c 100644
--- a/src/leap/bitmask/gui/eip_status.py
+++ b/src/leap/bitmask/gui/eip_status.py
@@ -367,6 +367,7 @@ class EIPStatusWidget(QtGui.QWidget):
         leap_assert_type(error, bool)
         if error:
             logger.error(status)
+            self.hide_eip_cancel_button()
         else:
             logger.debug(status)
         self._eip_status = status
@@ -723,9 +724,17 @@ class EIPStatusWidget(QtGui.QWidget):
     def _on_eip_vpn_launcher_exception(self):
         # XXX We should implement again translatable exceptions so
         # we can pass a translatable string to the panel (usermessage attr)
-        self.set_eip_status("VPN Launcher error.", error=True)
+        # FIXME this logic should belong to the backend, not to this
+        # widget.
         self.set_eipstatus_off()
 
+        st = self.tr("VPN Launcher error. See the logs for more info.")
+        self.set_eip_status(st, error=True)
+
+        msg = self.tr("Encrypted Internet failed to start")
+        self.set_eip_message(msg)
+        self.show_fw_down_button()
+
         self.aborted()
 
     def _on_eip_no_polkit_agent_error(self):
diff --git a/src/leap/bitmask/services/eip/conductor.py b/src/leap/bitmask/services/eip/conductor.py
index b755f283..0ee56628 100644
--- a/src/leap/bitmask/services/eip/conductor.py
+++ b/src/leap/bitmask/services/eip/conductor.py
@@ -16,6 +16,9 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
 EIP Conductor module.
+
+This handles Qt Signals and triggers the calls to the backend,
+where the VPNProcess has been initialized.
 """
 import logging
 
@@ -90,7 +93,7 @@ class EIPConductor(object):
 
     def start_eip_machine(self, action):
         """
-        Initializes and starts the EIP state machine.
+        Initialize and start the EIP state machine.
         Needs the reference to the eip_status widget not to be empty.
 
         :action: QtAction
@@ -124,8 +127,12 @@ class EIPConductor(object):
     @QtCore.Slot()
     def _start_eip(self):
         """
-        Starts EIP.
+        Start EIP.
+
+        This set a couple of status flags and calls the start procedure in the
+        backend.
         """
+        # TODO status should be kept in a singleton in the backend.
         st = self._eip_status
         is_restart = st and st.is_restart
 
@@ -271,7 +278,7 @@ class EIPConductor(object):
         TRIGGERS:
             Signaler.eip_process_finished
 
-        Triggered when the EIP/VPN process finishes to set the UI
+        Triggered when the EIP/VPN process finishes, in order to set the UI
         accordingly.
 
         Ideally we would have the right exit code here,
diff --git a/src/leap/bitmask/services/eip/vpnprocess.py b/src/leap/bitmask/services/eip/vpnprocess.py
index c6a7b98b..c7159a93 100644
--- a/src/leap/bitmask/services/eip/vpnprocess.py
+++ b/src/leap/bitmask/services/eip/vpnprocess.py
@@ -202,7 +202,24 @@ class VPN(object):
                              "aborting openvpn launch.")
                 return
 
-        cmd = vpnproc.getCommand()
+        # FIXME it would be good to document where the
+        # errors here are catched, since we currently handle them
+        # at the frontend layer. This *should* move to be handled entirely
+        # in the backend.
+        # exception is indeed technically catched in backend, then converted
+        # into a signal, that is catched in the eip_status widget in the
+        # frontend, and converted into a signal to abort the connection that is
+        # sent to the backend again.
+
+        # the whole exception catching should be done in the backend, without
+        # the ping-pong to the frontend, and without adding any logical checks
+        # in the frontend. We should just communicate UI changes to frontend,
+        # and abstract us away from anything else.
+        try:
+            cmd = vpnproc.getCommand()
+        except Exception:
+            logger.error("Error while getting vpn command...")
+            raise
         env = os.environ
         for key, val in vpnproc.vpn_env.items():
             env[key] = val