Fix processing of Boolean values in SSSDConfig

Previously, we were just casting the strings to bool, but this
meant that all boolean values were "True".

This patch solves the problem and adds regression tests for it.

4 files changed, 169 insertions, 23 deletions

diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py
index 045aac31..d31fbe2c 100644
--- a/server/config/SSSDConfig.py
+++ b/server/config/SSSDConfig.py
@@ -249,21 +249,37 @@ class SSSDConfigSchema(SSSDChangeConf):
                              [split_option[DEFAULT]])
                     else:
                         try:
+                            if subtype == bool and \
+                            type(split_option[DEFAULT]) == str:
+                                parsed_options[option['name']] = \
+                                    (primarytype,
+                                     subtype,
+                                     desc,
+                                     [self.bool_lookup[split_option[DEFAULT].lower()]])
+                            else:
+                                parsed_options[option['name']] = \
+                                    (primarytype,
+                                     subtype,
+                                     desc,
+                                     [subtype(split_option[DEFAULT])])
+                        except ValueError, KeyError:
+                            raise ParsingError
+                else:
+                    try:
+                        if primarytype == bool and \
+                            type(split_option[DEFAULT]) == str:
+                                parsed_options[option['name']] = \
+                                    (primarytype,
+                                     subtype,
+                                     desc,
+                                     self.bool_lookup[split_option[DEFAULT].lower()])
+                        else:
                             parsed_options[option['name']] = \
                                 (primarytype,
                                  subtype,
                                  desc,
-                                 [subtype(split_option[DEFAULT])])
-                        except ValueError:
-                            raise ParsingError
-                else:
-                    try:
-                        parsed_options[option['name']] = \
-                            (primarytype,
-                             subtype,
-                             desc,
-                             primarytype(split_option[DEFAULT]))
-                    except ValueError:
+                                 primarytype(split_option[DEFAULT]))
+                    except ValueError, KeyError:
                         raise ParsingError
 
             elif optionlen > 3:
@@ -273,8 +289,12 @@ class SSSDConfigSchema(SSSDChangeConf):
                 for x in split_option[DEFAULT:]:
                     if type(x) != subtype:
                         try:
-                            fixed_options.extend([subtype(x)])
-                        except ValueError:
+                            if (subtype == bool and type(x) == str):
+                                newvalue = self.bool_lookup[x.lower()]
+                            else:
+                                newvalue = subtype(x)
+                            fixed_options.extend([newvalue])
+                        except ValueError, KeyError:
                             raise ParsingError
                     else:
                         fixed_options.extend([x])
@@ -504,6 +524,8 @@ class SSSDService(SSSDConfigObject):
             self.remove_option(optionname)
             return
 
+        raise_error = False
+
         # If we were expecting a list and didn't get one,
         # Create a list with a single entry. If it's the
         # wrong subtype, it will fail below
@@ -516,20 +538,41 @@ class SSSDService(SSSDConfigObject):
         if type(value) != option_schema[0]:
             # If it's possible to convert it, do so
             try:
-                value = option_schema[0](value)
+                if option_schema[0] == bool and \
+                type(value) == str:
+                    value = self.schema.bool_lookup[value.lower()]
+                else:
+                    value = option_schema[0](value)
             except ValueError:
+                raise_error = True
+            except KeyError:
+                raise_error = True
+
+            if raise_error:
                 raise TypeError('Expected %s for %s, received %s' %
-                            (option_schema[0], optionname, type(value)))
+                                (option_schema[0], optionname, type(value)))
 
         if type(value) == list:
             # Iterate through the list an ensure that all members
             # are of the appropriate subtype
             try:
-                value = [option_schema[1](x)
-                         for x in value]
+                newvalue = []
+                for x in value:
+                    if option_schema[1] == bool and \
+                    type(x) == str:
+                        newvalue.extend([self.schema.bool_lookup[x.lower()]])
+                    else:
+                        newvalue.extend([option_schema[1](x)])
             except ValueError:
+                raise_error = True
+            except KeyError:
+                raise_error = True
+
+            if raise_error:
                 raise TypeError('Expected %s' % option_schema[1])
 
+            value = newvalue
+
         self.options[optionname] = value
 
 class SSSDDomain(SSSDConfigObject):
@@ -708,6 +751,7 @@ class SSSDDomain(SSSDConfigObject):
             return
 
         option_schema = options[option]
+        raise_error = False
 
         # If we were expecting a list and didn't get one,
         # Create a list with a single entry. If it's the
@@ -721,19 +765,39 @@ class SSSDDomain(SSSDConfigObject):
         if type(value) != option_schema[0]:
             # If it's possible to convert it, do so
             try:
-                value = option_schema[0](value)
+                if option_schema[0] == bool and \
+                type(value) == str:
+                    value = self.schema.bool_lookup[value.lower()]
+                else:
+                    value = option_schema[0](value)
             except ValueError:
+                raise_error = True
+            except KeyError:
+                raise_error = True
+
+            if raise_error:
                 raise TypeError('Expected %s for %s, received %s' %
-                            (option_schema[0], option, type(value)))
+                                (option_schema[0], option, type(value)))
 
         if type(value) == list:
             # Iterate through the list an ensure that all members
             # are of the appropriate subtype
             try:
-                value = [option_schema[1](x)
-                         for x in value]
+                newvalue = []
+                for x in value:
+                    if option_schema[1] == bool and \
+                    type(x) == str:
+                        newvalue.extend([self.schema.bool_lookup[x.lower()]])
+                    else:
+                        newvalue.extend([option_schema[1](x)])
             except ValueError:
+                raise_error = True
+            except KeyError:
+                raise_error = True
+
+            if raise_error:
                 raise TypeError('Expected %s' % option_schema[1])
+            value = newvalue
 
         # Check whether we're adding a provider entry.
         is_provider = option.rfind('_provider')
diff --git a/server/config/SSSDConfigTest.py b/server/config/SSSDConfigTest.py
index e2efa43f..d3e45261 100644
--- a/server/config/SSSDConfigTest.py
+++ b/server/config/SSSDConfigTest.py
@@ -157,6 +157,20 @@ class SSSDConfigTestValid(unittest.TestCase):
         self.assertEqual(ldap_domain.get_option('auth_provider'), 'ldap')
         self.assertEqual(ldap_domain.get_option('id_provider'), 'ldap')
 
+class SSSDConfigTestInvalid(unittest.TestCase):
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    def testBadBool(self):
+        sssdconfig = SSSDConfig.SSSDConfig("etc/sssd.api.conf",
+                                           "etc/sssd.api.d")
+        sssdconfig.import_config("testconfigs/sssd-invalid-badbool.conf")
+        self.assertRaises(TypeError,
+                          sssdconfig.get_domain,'IPA')
+
 class SSSDConfigTestSSSDService(unittest.TestCase):
     def setUp(self):
         self.schema = SSSDConfig.SSSDConfigSchema("etc/sssd.api.conf",
@@ -848,6 +862,7 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
             'services',
             'reconnection_retries',
             'domains',
+            'debug_timestamps',
             'config_file_version']
 
         for option in control_list:
@@ -961,12 +976,14 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
         # Negative Test - Not initialized
         self.assertRaises(SSSDConfig.NotInitializedError, sssdconfig.get_service, 'sssd')
 
-        sssdconfig.new_config()
+        sssdconfig.import_config('testconfigs/sssd-valid.conf')
 
         service = sssdconfig.get_service('sssd')
         self.assertTrue(isinstance(service, SSSDConfig.SSSDService))
 
-        # TODO verify the contents of this service
+        # Verify the contents of this service
+        self.assertEqual(type(service.get_option('debug_timestamps')), bool)
+        self.assertFalse(service.get_option('debug_timestamps'))
 
         # Negative Test - No such service
         self.assertRaises(SSSDConfig.NoServiceError, sssdconfig.get_service, 'nosuchservice')
@@ -1244,6 +1261,22 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
         self.assertEquals(len(sssdconfig.list_inactive_domains()),
                           len(inactivelist)-1)
 
+        # Positive test - Ensure that saved domains retain values
+        domain.set_option('ldap_krb5_init_creds', True)
+        domain.set_option('ldap_id_use_start_tls', False)
+        domain.set_option('ldap_user_search_base',
+                          'cn=accounts, dc=example, dc=com')
+        self.assertTrue(domain.get_option('ldap_krb5_init_creds'))
+        self.assertFalse(domain.get_option('ldap_id_use_start_tls'))
+        self.assertEqual(domain.get_option('ldap_user_search_base'),
+                         'cn=accounts, dc=example, dc=com')
+
+        sssdconfig.save_domain(domain)
+        sssdconfig.write('/tmp/testSaveDomain.out')
+
+        domain2 = sssdconfig.get_domain('example.com2')
+        self.assertTrue(domain2.get_option('ldap_krb5_init_creds'))
+        self.assertFalse(domain2.get_option('ldap_id_use_start_tls'))
 
     def testActivateDomain(self):
         sssdconfig = SSSDConfig.SSSDConfig("etc/sssd.api.conf",
@@ -1342,4 +1375,9 @@ if __name__ == "__main__":
     if not res.wasSuccessful():
         error |= 0x8
 
+    suite = unittest.TestLoader().loadTestsFromTestCase(SSSDConfigTestInvalid)
+    res = unittest.TextTestRunner().run(suite)
+    if not res.wasSuccessful():
+        error |= 0x10
+
     exit(error)
diff --git a/server/config/testconfigs/sssd-invalid-badbool.conf b/server/config/testconfigs/sssd-invalid-badbool.conf
new file mode 100644
index 00000000..25c27f49
--- /dev/null
+++ b/server/config/testconfigs/sssd-invalid-badbool.conf
@@ -0,0 +1,43 @@
+[nss]
+nss_filter_groups = root
+nss_entry_negative_timeout = 15
+debug_level = 0
+nss_filter_users_in_groups = true
+nss_filter_users = root
+nss_entry_cache_no_wait_timeout = 60
+nss_entry_cache_timeout = 600
+nss_enum_cache_timeout = 120
+
+[sssd]
+services = nss, pam
+reconnection_retries = 3
+domains = LOCAL, IPA
+config_file_version = 2
+
+[domain/PROXY]
+id_provider = proxy
+auth_provider = proxy
+debug_level = 0
+
+[domain/IPA]
+id_provider = ldap
+ldap_id_use_start_tls = Fal
+auth_provider = krb5
+debug_level = 0
+
+[domain/LOCAL]
+id_provider = local
+auth_provider = local
+debug_level = 0
+
+[domain/LDAP]
+id_provider  =    ldap
+auth_provider=ldap
+debug_level = 0
+
+[pam]
+debug_level = 0
+
+[dp]
+debug_level = 0
+
diff --git a/server/config/testconfigs/sssd-valid.conf b/server/config/testconfigs/sssd-valid.conf
index 82b3fd81..79016eb4 100644
--- a/server/config/testconfigs/sssd-valid.conf
+++ b/server/config/testconfigs/sssd-valid.conf
@@ -13,6 +13,7 @@ services = nss, pam
 reconnection_retries = 3
 domains = LOCAL, IPA
 config_file_version = 2
+debug_timestamps = False
 
 [domain/PROXY]
 id_provider = proxy