[bug] report global connecting state for vpn when user started

- Resolves: #8850

2 files changed, 27 insertions, 7 deletions

diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py
index b04ea9f6..874c30d3 100644
--- a/src/leap/bitmask/vpn/service.py
+++ b/src/leap/bitmask/vpn/service.py
@@ -48,7 +48,7 @@ class VPNService(HookableService):
 
     def __init__(self, basepath=None):
         """
-        Initialize VPN service
+        Initialize VPN service. This launches both the firewall and the vpn.
         """
         super(VPNService, self).__init__()
 
@@ -73,7 +73,10 @@ class VPNService(HookableService):
 
     @defer.inlineCallbacks
     def start_vpn(self, domain):
-        # TODO check if the VPN is started and return an error if it is.
+        if self._started:
+            exc = Exception('VPN already started')
+            exc.expected = True
+            raise exc
         yield self._setup(domain)
         try:
             self._vpn.start()
@@ -109,6 +112,7 @@ class VPNService(HookableService):
             'error': None,
             'childrenStatus': {}
         }
+
         if self._vpn:
             status = self._vpn.get_status()
 
@@ -116,7 +120,6 @@ class VPNService(HookableService):
             status['domain'] = self._domain
         else:
             status['domain'] = self._read_last()
-
         return status
 
     def do_check(self, domain=None):
diff --git a/src/leap/bitmask/vpn/vpn.py b/src/leap/bitmask/vpn/vpn.py
index d32d534b..677c9b91 100644
--- a/src/leap/bitmask/vpn/vpn.py
+++ b/src/leap/bitmask/vpn/vpn.py
@@ -16,8 +16,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import commands
-
 from colorama import Fore
 
 from leap.bitmask.util import merge_status
@@ -32,14 +30,18 @@ class VPNManager(object):
         self._vpn = TunnelManager(
             provider, remotes, cert, key, ca, flags)
         self._firewall = FirewallManager(remotes)
+        self.starting = False
 
     def start(self):
+        # TODO we should have some way of switching this flag to False
+        # other than parsing the result of the status command.
+        self.starting = True
         print(Fore.BLUE + "Firewall: starting..." + Fore.RESET)
         fw_ok = self._firewall.start()
         if not fw_ok:
             print(Fore.RED + "Firewall: problem!")
+            self.starting = False
             return False
-
         print(Fore.GREEN + "Firewall: started" + Fore.RESET)
 
         vpn_ok = self._vpn.start()
@@ -47,11 +49,12 @@ class VPNManager(object):
             print (Fore.RED + "VPN: Error starting." + Fore.RESET)
             self._firewall.stop()
             print(Fore.GREEN + "Firewall: stopped." + Fore.RESET)
+            self.starting = False
             return False
-
         print(Fore.GREEN + "VPN: started" + Fore.RESET)
 
     def stop(self):
+        self.starting = False
         print(Fore.BLUE + "Firewall: stopping..." + Fore.RESET)
         fw_ok = self._firewall.stop()
 
@@ -81,4 +84,18 @@ class VPNManager(object):
             "vpn": self._vpn.status,
             "firewall": self._firewall.status
         }
+        if self.starting:
+            # XXX small correction to the merge: if we are starting fw+vpn,
+            # we report vpn as starting so that is consistent with the ui or
+            # cli action. this state propagates from the parent
+            # object to the vpn child, and we revert it when we reach
+            # the 'on' state. this needs to be revisited in the formal state
+            # machine, and mainly needs a way of setting that state directly
+            # and resetting the 'starting' flag without resorting to hijack
+            # this command.
+            vpnstatus = childrenStatus['vpn']['status']
+            if vpnstatus == 'off':
+                childrenStatus['vpn']['status'] = 'starting'
+            if vpnstatus == 'on':
+                self.starting = False
         return merge_status(childrenStatus)