From 0faa69992d1c9768f3ec92112e8691a9b7c4a3a2 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Thu, 10 Dec 2009 10:07:04 -0500 Subject: 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. --- server/config/SSSDConfig.py | 106 +++++++++++++++++---- server/config/SSSDConfigTest.py | 42 +++++++- .../config/testconfigs/sssd-invalid-badbool.conf | 43 +++++++++ server/config/testconfigs/sssd-valid.conf | 1 + 4 files changed, 169 insertions(+), 23 deletions(-) create mode 100644 server/config/testconfigs/sssd-invalid-badbool.conf (limited to 'server') diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py index 045aac31a..d31fbe2c4 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 e2efa43fb..d3e452618 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 000000000..25c27f498 --- /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 82b3fd819..79016eb4f 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 -- cgit