[bug] use username instead of provider in the vpn calls

Without active user we need to use the username instead of the provider
in the VPN API.

- Resolves: #8783

6 files changed, 95 insertions, 34 deletions

diff --git a/src/leap/bitmask/bonafide/service.py b/src/leap/bitmask/bonafide/service.py
index 69aac2df..dc579179 100644
--- a/src/leap/bitmask/bonafide/service.py
+++ b/src/leap/bitmask/bonafide/service.py
@@ -120,21 +120,16 @@ class BonafideService(HookableService):
     def do_provider_list(self, seeded=False):
         return self._bonafide.do_provider_list(seeded)
 
-    # TODO make username mandatory
-    # and move active_user to the cli machinery
-    def do_get_vpn_cert(self, username=None):
-        if not username:
-            username = self._active_user
+    def do_get_vpn_cert(self, username):
         if not username:
             return defer.fail(
-                RuntimeError('No active user, cannot get VPN cert.'))
+                RuntimeError('No username, cannot get VPN cert.'))
+
         d = self._bonafide.do_get_vpn_cert(username)
         d.addCallback(lambda response: (username, response))
         return d
 
-    def do_get_smtp_cert(self, username=None):
-        if not username:
-            username = self._active_user
+    def do_get_smtp_cert(self, username):
         if not username:
             return defer.fail(
                 RuntimeError('No username, cannot get SMTP cert.'))
diff --git a/src/leap/bitmask/cli/command.py b/src/leap/bitmask/cli/command.py
index a4757f80..068f19b5 100644
--- a/src/leap/bitmask/cli/command.py
+++ b/src/leap/bitmask/cli/command.py
@@ -42,9 +42,9 @@ def default_dict_printer(result):
     if not result:
         return
     for key, value in result.items():
-        if value is None:
+        if value is not str:
             value = str(value)
-        if value in ('OFF', 'OFFLINE', 'ABORTED'):
+        if value in ('OFF', 'OFFLINE', 'ABORTED', 'False'):
             color = Fore.RED
         else:
             color = Fore.GREEN
diff --git a/src/leap/bitmask/cli/vpn.py b/src/leap/bitmask/cli/vpn.py
index d80b7f39..69825159 100644
--- a/src/leap/bitmask/cli/vpn.py
+++ b/src/leap/bitmask/cli/vpn.py
@@ -17,6 +17,9 @@
 """
 Bitmask Command Line interface: vpn module
 """
+import argparse
+import sys
+
 from leap.bitmask.cli import command
 
 
@@ -40,6 +43,66 @@ SUBCOMMANDS:
 
 '''.format(name=command.appname)
 
-    commands = ['start', 'stop', 'status', 'check',
-                'get_cert', 'install', 'uninstall',
+    commands = ['stop', 'status', 'install', 'uninstall',
                 'enable', 'disable']
+
+    def start(self, raw_args):
+        parser = argparse.ArgumentParser(
+            description='Bitmask VPN start',
+            prog='%s %s %s' % tuple(sys.argv[:3]))
+        parser.add_argument('provider', nargs='?', default=None,
+                            help='provider to start the VPN')
+        subargs = parser.parse_args(raw_args)
+
+        provider = None
+        if subargs.provider:
+            provider = subargs.provider
+        else:
+            uid = self.cfg.get('bonafide', 'active', default=None)
+            try:
+                _, provider = uid.split('@')
+            except ValueError:
+                raise ValueError("A provider is needed to start the VPN")
+
+        self.data += ['start', provider]
+
+        return self._send(command.default_dict_printer)
+
+    def check(self, raw_args):
+        parser = argparse.ArgumentParser(
+            description='Bitmask VPN check',
+            prog='%s %s %s' % tuple(sys.argv[:3]))
+        parser.add_argument('provider', nargs='?', default=None,
+                            help='provider to check the VPN')
+        subargs = parser.parse_args(raw_args)
+
+        provider = None
+        if subargs.provider:
+            provider = subargs.provider
+        else:
+            uid = self.cfg.get('bonafide', 'active', default=None)
+            try:
+                _, provider = uid.split('@')
+            except ValueError:
+                raise ValueError("A provider is needed to start the VPN")
+
+        self.data += ['check', provider]
+
+        return self._send(command.default_dict_printer)
+
+    def get_cert(self, raw_args):
+        parser = argparse.ArgumentParser(
+            description='Bitmask VPN cert fetcher',
+            prog='%s %s %s' % tuple(sys.argv[:3]))
+        parser.add_argument('uid', nargs='?', default=None,
+                            help='uid to fetch the VPN cert')
+        subargs = parser.parse_args(raw_args)
+
+        uid = None
+        if subargs.uid:
+            uid = subargs.uid
+        else:
+            uid = self.cfg.get('bonafide', 'active', default=None)
+        self.data += ['get_cert', uid]
+
+        return self._send(command.default_dict_printer)
diff --git a/src/leap/bitmask/core/dispatcher.py b/src/leap/bitmask/core/dispatcher.py
index 3e50b11f..075dfd82 100644
--- a/src/leap/bitmask/core/dispatcher.py
+++ b/src/leap/bitmask/core/dispatcher.py
@@ -196,14 +196,11 @@ class VPNCmd(SubCommand):
 
     @register_method('dict')
     def do_START(self, vpn, *parts):
-        # TODO --- attempt to get active provider
-        # TODO or catch the exception and send error
         try:
             provider = parts[2]
         except IndexError:
             raise DispatchError(
                 'wrong number of arguments: expected 1, got none')
-        provider = parts[2]
         d = vpn.start_vpn(provider)
         return d
 
@@ -214,14 +211,22 @@ class VPNCmd(SubCommand):
 
     @register_method('dict')
     def do_CHECK(self, vpn, *parts):
-        d = vpn.do_check()
+        try:
+            provider = parts[2]
+        except IndexError:
+            raise DispatchError(
+                'wrong number of arguments: expected 1, got none')
+        d = vpn.do_check(provider)
         return d
 
     @register_method('dict')
     def do_GET_CERT(self, vpn, *parts):
-        # TODO -- attempt to get active provider
-        provider = parts[2]
-        d = vpn.do_get_cert(provider)
+        try:
+            username = parts[2]
+        except IndexError:
+            raise DispatchError(
+                'wrong number of arguments: expected 1, got none')
+        d = vpn.do_get_cert(username)
         return d
 
     @register_method('dict')
diff --git a/src/leap/bitmask/vpn/README.rst b/src/leap/bitmask/vpn/README.rst
index eff62ab4..f773e2f9 100644
--- a/src/leap/bitmask/vpn/README.rst
+++ b/src/leap/bitmask/vpn/README.rst
@@ -11,7 +11,7 @@ It only works through the cli right now::
   srp_token iye7s1J7M3_iCdB4gXEAhxs-if3XOCwpKNPnvTC8ycE
   uuid      b63ac83826c7e1e903ed18f6f7780491
 
-  bitmaskctl vpn get_cert demo.bitmask.net
+  bitmaskctl vpn get_cert
   get_cert  ok
 
   bitmaskctl vpn check
@@ -26,7 +26,7 @@ authentication)::
 
 And finally you should be able to launch the VPN::
 
-  bitmaskctl vpn start demo.bitmask.net
+  bitmaskctl vpn start
   start     ok
   result    started
 
diff --git a/src/leap/bitmask/vpn/service.py b/src/leap/bitmask/vpn/service.py
index e56546ea..60de0f66 100644
--- a/src/leap/bitmask/vpn/service.py
+++ b/src/leap/bitmask/vpn/service.py
@@ -27,8 +27,6 @@ from twisted.internet import defer
 from leap.bitmask.hooks import HookableService
 from leap.bitmask.vpn.vpn import VPNManager
 from leap.bitmask.vpn._checks import is_service_ready, get_vpn_cert_path
-from leap.bitmask.vpn._config import get_bitmask_helper_path
-from leap.bitmask.vpn._config import get_bitmask_polkit_policy_path
 from leap.bitmask.vpn import privilege
 from leap.common.config import get_path_prefix
 from leap.common.files import check_and_fix_urw_only
@@ -88,22 +86,22 @@ class VPNService(HookableService):
         status['domain'] = self._domain
         return status
 
-    def do_check(self):
+    def do_check(self, domain):
         """Check whether the VPN Service is properly configured,
         and can be started"""
-        # TODO either pass a provider, or set a given provider
-        _ready = is_service_ready('demo.bitmask.net')
-        if _ready:
-            result = 'ok'
-        else:
-            result = 'no'
-        return {'vpn_ready': result}
+        return {'vpn_ready': is_service_ready(domain)}
 
     @defer.inlineCallbacks
-    def do_get_cert(self, provider):
+    def do_get_cert(self, username):
+        try:
+            _, provider = username.split('@')
+        except ValueError:
+            raise ValueError(username + ' is not a valid username, it should'
+                             ' contain an @')
+
         # fetch vpn cert and store
         bonafide = self.parent.getServiceNamed("bonafide")
-        _, cert_str = yield bonafide.do_get_vpn_cert()
+        _, cert_str = yield bonafide.do_get_vpn_cert(username)
 
         cert_path = get_vpn_cert_path(provider)
         cert_dir = os.path.dirname(cert_path)