deprecated configparser. closes #500

5 files changed, 78 insertions, 212 deletions

diff --git a/src/leap/eip/config.py b/src/leap/eip/config.py
index c0819628..810a5a8d 100644
--- a/src/leap/eip/config.py
+++ b/src/leap/eip/config.py
@@ -1,18 +1,13 @@
-import ConfigParser  # to be deprecated
-import json
 import logging
 import os
 import platform
-import socket
 
-from leap.util.fileutil import (which, mkdir_p,
-                                check_and_fix_urw_only)
+from leap.util.fileutil import (which, check_and_fix_urw_only)
 
 from leap.base import config as baseconfig
 from leap.baseapp.permcheck import (is_pkexec_in_system,
                                     is_auth_agent_running)
 from leap.eip import exceptions as eip_exceptions
-from leap.eip import constants as eipconstants
 from leap.eip import specs as eipspecs
 
 logging.basicConfig()
@@ -104,7 +99,9 @@ def build_ovpn_options(daemon=False):
     ourplatform = platform.system()
     if ourplatform in ("Linux", "Mac"):
         opts.append('--management')
+
         # XXX get a different sock each time ...
+        # XXX #505
         opts.append('/tmp/.eip.sock')
         opts.append('unix')
     if ourplatform == "Windows":
@@ -130,14 +127,11 @@ def build_ovpn_options(daemon=False):
     return opts
 
 
-def build_ovpn_command(config, debug=False, do_pkexec_check=True):
+def build_ovpn_command(debug=False, do_pkexec_check=True, vpnbin=None):
     """
     build a string with the
     complete openvpn invocation
 
-    @param config: config object
-    @type config: ConfigParser instance
-
     @rtype [string, [list of strings]]
     @rparam: a list containing the command string
         and a list of options.
@@ -146,11 +140,11 @@ def build_ovpn_command(config, debug=False, do_pkexec_check=True):
     use_pkexec = True
     ovpn = None
 
-    if config.has_option('openvpn', 'use_pkexec'):
-        use_pkexec = config.get('openvpn', 'use_pkexec')
+    # XXX get use_pkexec from config instead.
+
     if platform.system() == "Linux" and use_pkexec and do_pkexec_check:
 
-        # XXX check for both pkexec (done)
+        # check for both pkexec
         # AND a suitable authentication
         # agent running.
         logger.info('use_pkexec set to True')
@@ -168,23 +162,15 @@ def build_ovpn_command(config, debug=False, do_pkexec_check=True):
             raise eip_exceptions.EIPNoPolkitAuthAgentAvailable
 
         command.append('pkexec')
-
-    if config.has_option('openvpn',
-                         'openvpn_binary'):
-        ovpn = config.get('openvpn',
-                          'openvpn_binary')
-    if not ovpn and config.has_option('DEFAULT',
-                                      'openvpn_binary'):
-        ovpn = config.get('DEFAULT',
-                          'openvpn_binary')
-
+    if vpnbin is None:
+        ovpn = which('openvpn')
+    else:
+        ovpn = vpnbin
     if ovpn:
         vpn_command = ovpn
     else:
         vpn_command = "openvpn"
-
     command.append(vpn_command)
-
     daemon_mode = not debug
 
     for opt in build_ovpn_options(daemon=daemon_mode):
@@ -195,77 +181,7 @@ def build_ovpn_command(config, debug=False, do_pkexec_check=True):
     return [command[0], command[1:]]
 
 
-# XXX deprecate
-def get_sensible_defaults():
-    """
-    gathers a dict of sensible defaults,
-    platform sensitive,
-    to be used to initialize the config parser
-    @rtype: dict
-    @rparam: default options.
-    """
-
-    # this way we're passing a simple dict
-    # that will initialize the configparser
-    # and will get written to "DEFAULTS" section,
-    # which is fine for now.
-    # if we want to write to a particular section
-    # we can better pass a tuple of triples
-    # (('section1', 'foo', '23'),)
-    # and config.set them
-
-    defaults = dict()
-    defaults['openvpn_binary'] = which('openvpn')
-    defaults['autostart'] = 'true'
-
-    # TODO
-    # - management.
-    return defaults
-
-
-# XXX to be deprecated. see dump_default_eipconfig
-# and the new JSONConfig classes.
-def get_config(config_file=None):
-    """
-    temporary method for getting configs,
-    mainly for early stage development process.
-    in the future we will get preferences
-    from the storage api
-
-    @rtype: ConfigParser instance
-    @rparam: a config object
-    """
-    defaults = get_sensible_defaults()
-    config = ConfigParser.ConfigParser(defaults)
-
-    if not config_file:
-        fpath = baseconfig.get_config_file('eip.cfg')
-        if not os.path.isfile(fpath):
-            dpath, cfile = os.path.split(fpath)
-            if not os.path.isdir(dpath):
-                mkdir_p(dpath)
-            with open(fpath, 'wb') as configfile:
-                config.write(configfile)
-        config_file = open(fpath)
-    config.readfp(config_file)
-    return config
-
-
-def dump_default_eipconfig(filepath):
-    """
-    writes a sample eip config
-    in the given location
-    """
-    # XXX TODO:
-    # use EIPConfigSpec istead
-    folder, filename = os.path.split(filepath)
-    if not os.path.isdir(folder):
-        mkdir_p(folder)
-    with open(filepath, 'w') as fp:
-        json.dump(eipconstants.EIP_SAMPLE_JSON, fp)
-
-
-def check_vpn_keys(config):
+def check_vpn_keys():
     """
     performs an existance and permission check
     over the openvpn keys file.
@@ -273,35 +189,24 @@ def check_vpn_keys(config):
     per provider, containing the CA cert,
     the provider key, and our client certificate
     """
+    provider_ca = eipspecs.provider_ca_path()
+    client_cert = eipspecs.client_cert_path()
 
-    keyopt = ('provider', 'keyfile')
-
-    # XXX at some point,
-    # should separate between CA, provider cert
-    # and our certificate.
-    # make changes in the default provider template
-    # accordingly.
-
-    # get vpn keys
-    if config.has_option(*keyopt):
-        keyfile = config.get(*keyopt)
-    else:
-        keyfile = baseconfig.get_config_file(
-            'openvpn.keys',
-            folder=baseconfig.get_default_provider_path())
-        logger.debug('keyfile = %s', keyfile)
+    logger.debug('provider ca = %s', provider_ca)
+    logger.debug('client cert = %s', client_cert)
 
     # if no keys, raise error.
     # should be catched by the ui and signal user.
 
-    if not os.path.isfile(keyfile):
-        logger.error('key file %s not found. aborting.',
-                     keyfile)
-        raise eip_exceptions.EIPInitNoKeyFileError
-
-    # check proper permission on keys
-    # bad perms? try to fix them
-    try:
-        check_and_fix_urw_only(keyfile)
-    except OSError:
-        raise eip_exceptions.EIPInitBadKeyFilePermError
+    for keyfile in (provider_ca, client_cert):
+        if not os.path.isfile(keyfile):
+            logger.error('key file %s not found. aborting.',
+                         keyfile)
+            raise eip_exceptions.EIPInitNoKeyFileError
+
+        # check proper permission on keys
+        # bad perms? try to fix them
+        try:
+            check_and_fix_urw_only(keyfile)
+        except OSError:
+            raise eip_exceptions.EIPInitBadKeyFilePermError
diff --git a/src/leap/eip/openvpnconnection.py b/src/leap/eip/openvpnconnection.py
index 1f2f6d8c..32fa55b1 100644
--- a/src/leap/eip/openvpnconnection.py
+++ b/src/leap/eip/openvpnconnection.py
@@ -77,8 +77,10 @@ to be triggered for each one of them.
         self.command = None
         self.args = None
 
+        # XXX get autostart from config
         self.autostart = True
-        self._get_or_create_config()
+        #self._get_or_create_config()
+        self._set_ovpn_command()
         self._check_vpn_keys()
 
         #
@@ -95,49 +97,21 @@ to be triggered for each one of them.
         self.port = port
         self.password = password
 
-    def _set_autostart(self):
-        config = self.config
-        if config.has_option('openvpn', 'autostart'):
-            autostart = config.getboolean('openvpn',
-                                          'autostart')
-            self.autostart = autostart
-        else:
-            if config.has_option('DEFAULT', 'autostart'):
-                autostart = config.getboolean('DEFAULT',
-                                              'autostart')
-                self.autostart = autostart
-
     def _set_ovpn_command(self):
-        config = self.config
-        if config.has_option('openvpn', 'command'):
-            commandline = config.get('openvpn', 'command')
-
-            command_split = commandline.split(' ')
-            command = command_split[0]
-            if len(command_split) > 1:
-                args = command_split[1:]
-            else:
-                args = []
-
-            self.command = command
-            self.args = args
-        else:
-        # no command in config, we build it up.
         # XXX check also for command-line --command flag
-            try:
-                command, args = eip_config.build_ovpn_command(
-                    config,
-                    debug=self.debug)
-            except eip_exceptions.EIPNoPolkitAuthAgentAvailable:
-                command = args = None
-                self.missing_auth_agent = True
-            except eip_exceptions.EIPNoPkexecAvailable:
-                command = args = None
-                self.missing_pkexec = True
-
-            # XXX if not command, signal error.
-            self.command = command
-            self.args = args
+        try:
+            command, args = eip_config.build_ovpn_command(
+                debug=self.debug)
+        except eip_exceptions.EIPNoPolkitAuthAgentAvailable:
+            command = args = None
+            self.missing_auth_agent = True
+        except eip_exceptions.EIPNoPkexecAvailable:
+            command = args = None
+            self.missing_pkexec = True
+
+        # XXX if not command, signal error.
+        self.command = command
+        self.args = args
 
     def _get_or_create_config(self):
         """
@@ -145,19 +119,16 @@ to be triggered for each one of them.
         home file, or config file passed in command line.
         populates command and args to be passed to subprocess.
         """
-        config = eip_config.get_config(
-            config_file=self.config_file)
-        self.config = config
-
-        self._set_autostart()
-        self._set_ovpn_command()
+        # XXX does nothing.
+        # XXX should get config? or get from checker?
+        pass
 
     def _check_vpn_keys(self):
         """
         checks for correct permissions on vpn keys
         """
         try:
-            eip_config.check_vpn_keys(self.config)
+            eip_config.check_vpn_keys()
         except eip_exceptions.EIPInitNoKeyFileError:
             self.missing_vpn_keyfile = True
         except eip_exceptions.EIPInitBadKeyFilePermError:
diff --git a/src/leap/eip/tests/test_config.py b/src/leap/eip/tests/test_config.py
index 87ef33ef..c73281cc 100644
--- a/src/leap/eip/tests/test_config.py
+++ b/src/leap/eip/tests/test_config.py
@@ -1,15 +1,16 @@
-import ConfigParser
 import os
 import platform
+import stat
 
 try:
     import unittest2 as unittest
 except ImportError:
     import unittest
 
-from leap.base import constants
-from leap.eip import config as eip_config
+#from leap.base import constants
+#from leap.eip import config as eip_config
 from leap.testing.basetest import BaseLeapTest
+from leap.util.fileutil import mkdir_p
 
 _system = platform.system()
 
@@ -29,19 +30,14 @@ class EIPConfigTest(BaseLeapTest):
     #
 
     def touch_exec(self):
+        path = os.path.join(
+            self.tempdir, 'bin')
+        mkdir_p(path)
         tfile = os.path.join(
-            self.tempfile,
-            'bin',
+            path,
             'openvpn')
-        open(tfile, 'bw').close()
-
-    def get_empty_config(self):
-        _config = ConfigParser.ConfigParser()
-        return _config
-
-    def get_minimal_config(self):
-        _config = ConfigParser.ConfigParser()
-        return _config
+        open(tfile, 'wb').close()
+        os.chmod(tfile, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
 
     def get_expected_openvpn_args(self):
         args = []
@@ -110,17 +106,18 @@ class EIPConfigTest(BaseLeapTest):
     # some checks.
 
     def test_build_ovpn_command_empty_config(self):
-        _config = self.get_empty_config()
-        command, args = eip_config.build_ovpn_command(
-            _config,
-            do_pkexec_check=False)
-        self.assertEqual(command, 'openvpn')
+        self.touch_exec()
+        from leap.eip import config as eipconfig
+        from leap.util.fileutil import which
+        path = os.environ['PATH']
+        vpnbin = which('openvpn', path=path)
+        print 'path =', path
+        print 'vpnbin = ', vpnbin
+        command, args = eipconfig.build_ovpn_command(
+            do_pkexec_check=False, vpnbin=vpnbin)
+        self.assertEqual(command, self.home + '/bin/openvpn')
         self.assertEqual(args, self.get_expected_openvpn_args())
 
-    # XXX TODO:
-    # - should use touch_exec to plant an "executable" in the path
-    # - should check that "which" for openvpn returns what's expected.
-
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/src/leap/eip/tests/test_eipconnection.py b/src/leap/eip/tests/test_eipconnection.py
index 26f6529e..23f645c3 100644
--- a/src/leap/eip/tests/test_eipconnection.py
+++ b/src/leap/eip/tests/test_eipconnection.py
@@ -1,4 +1,3 @@
-import ConfigParser
 import logging
 import platform
 import os
@@ -13,9 +12,9 @@ except ImportError:
 
 from mock import Mock, patch  # MagicMock
 
-from leap.base import constants
 from leap.eip.eipconnection import EIPConnection
 from leap.eip.exceptions import ConnectionRefusedError
+from leap.eip import specs as eipspecs
 from leap.testing.basetest import BaseLeapTest
 
 _system = platform.system()
@@ -29,7 +28,6 @@ class NotImplementedError(Exception):
 @patch('OpenVPNConnection._set_ovpn_command')
 class MockedEIPConnection(EIPConnection):
     def _get_or_create_config(self):
-        self.config = ConfigParser.ConfigParser()
         self._set_ovpn_command()
 
     def _set_ovpn_command(self):
@@ -56,11 +54,11 @@ class EIPConductorTest(BaseLeapTest):
         # XXX change to keys_checker invocation
         # (see config_checker)
 
-        filepath = os.path.expanduser(
-            '~/.config/leap/providers/%s/openvpn.keys'
-            % constants.DEFAULT_TEST_PROVIDER)
-        self.touch(filepath)
-        self.chmod600(filepath)
+        keyfiles = (eipspecs.provider_ca_path(),
+                    eipspecs.client_cert_path())
+        for filepath in keyfiles:
+            self.touch(filepath)
+            self.chmod600(filepath)
 
         # we init the manager with only
         # some methods mocked
@@ -85,13 +83,6 @@ class EIPConductorTest(BaseLeapTest):
         self.assertEqual(con.missing_provider, False)
         self.assertEqual(con.bad_provider, False)
 
-    def test_config_was_init(self):
-        """
-        is there a config object?
-        """
-        self.assertTrue(isinstance(self.con.config,
-                        ConfigParser.ConfigParser))
-
     def test_ovpn_command(self):
         """
         set_ovpn_command called
diff --git a/src/leap/util/fileutil.py b/src/leap/util/fileutil.py
index 429e4b12..aef4cfe0 100644
--- a/src/leap/util/fileutil.py
+++ b/src/leap/util/fileutil.py
@@ -21,7 +21,7 @@ def extend_path():
     # XXX add mac / win extended search paths?
 
 
-def which(program):
+def which(program, path=None):
     """
     an implementation of which
     that extends the path with
@@ -67,8 +67,10 @@ def which(program):
     else:
         # extended iterator
         # with extra path
+        if path is None:
+            path = os.environ['PATH']
         extended_path = chain(
-            iter_path(os.environ["PATH"]),
+            iter_path(path),
             iter_path(extend_path()))
         for candidate in extended_path:
             if candidate is not None: