Make SSSDDomain.remove_provider() remove configured options

We will remove all options for a provider that are not also
required by another configured provider. (For example, we will not
remove krb5_realm when deleting the krb5 auth provider if the LDAP
provider is in use, since it may still require this argument).

2 files changed, 54 insertions, 6 deletions

diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py
index 1992a940..2abafe15 100644
--- a/server/config/SSSDConfig.py
+++ b/server/config/SSSDConfig.py
@@ -153,6 +153,13 @@ option_strings = {
 def striplist(l):
     return([x.strip() for x in l])
 
+def options_overlap(options1, options2):
+    overlap = []
+    for option in options1:
+        if option in options2:
+            overlap.append(option)
+    return overlap
+
 class SSSDConfigSchema(SSSDChangeConf):
     def __init__(self, schemafile, schemaplugindir):
         SSSDChangeConf.__init__(self)
@@ -729,7 +736,6 @@ class SSSDDomain(SSSDConfigObject):
                 raise TypeError('Expected %s' % option_schema[1])
 
         # Check whether we're adding a provider entry.
-        # This requires special handling
         is_provider = option.rfind('_provider')
         if (is_provider > 0):
             provider = option[:is_provider]
@@ -786,7 +792,6 @@ class SSSDDomain(SSSDConfigObject):
                                                      (provider,
                                                       provider_type)))
 
-
     def remove_provider(self, provider_type):
         """
         Remove a provider from the domain. If the provider is not present, it
@@ -812,10 +817,26 @@ class SSSDDomain(SSSDConfigObject):
         if not provider:
             return
 
-        # TODO: safely remove any unused options when removing
-        # the provider. This will require modifying the schema
-        # to account for multiple providers making use of the
-        # same options (such ask krb5_realm)
+        # Remove any unused options when removing the provider.
+        options = self.list_provider_options(provider, provider_type)
+        
+        # Trim any options that are used by other providers,
+        # if that provider is in use
+        for (prov, ptype) in self.providers:
+            # Ignore the one being removed
+            if (prov, ptype) == (provider, provider_type):
+                continue
+            
+            provider_options = self.list_provider_options(prov, ptype)
+            overlap = options_overlap(options.keys(), provider_options.keys())
+            for opt in overlap:
+                del options[opt]
+
+        # We should now have a list of options used only by this
+        # provider. So we remove them.
+        for option in options:
+            if self.options.has_key(option):
+                del self.options[option]
 
         self.providers.remove((provider, provider_type))
 
diff --git a/server/config/SSSDConfigTest.py b/server/config/SSSDConfigTest.py
index fa111819..3d8b596a 100644
--- a/server/config/SSSDConfigTest.py
+++ b/server/config/SSSDConfigTest.py
@@ -664,6 +664,30 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
                             'Option [%s] unexpectedly found' %
                             option)
 
+        # Remove the local ID provider and add an LDAP one
+        # LDAP ID providers can also use the krb5_realm
+        domain.remove_provider('id')
+
+        domain.add_provider('ldap', 'id')
+
+        # Set the krb5_realm option and the ldap_uri option
+        domain.set_option('krb5_realm', 'EXAMPLE.COM')
+        domain.set_option('ldap_uri', 'ldap://ldap.example.com')
+
+        self.assertEquals(domain.get_option('krb5_realm'),
+                          'EXAMPLE.COM')
+        self.assertEquals(domain.get_option('ldap_uri'),
+                          'ldap://ldap.example.com')
+
+        # Remove the LDAP provider and verify that krb5_realm remains
+        domain.remove_provider('id')
+        self.assertEquals(domain.get_option('krb5_realm'),
+                  'EXAMPLE.COM')
+        self.assertFalse(domain.options.has_key('ldap_uri'))
+
+        # Put the LOCAL provider back
+        domain.add_provider('local', 'id')
+
         # Remove the auth domain and verify that the options
         # revert to the backup_list
         domain.remove_provider('auth')
@@ -684,6 +708,9 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
                             'Option [%s] unexpectedly found' %
                             option)
 
+        # Ensure that the krb5_realm option is now gone
+        self.assertFalse(domain.options.has_key('krb5_realm'))
+
         # Test removing nonexistent provider - Real
         domain.remove_provider('id')