Only use service PAC type as an override

PAC type (ipakrbauthzdata attribute) was being filled for all new
service automatically. However, the PAC type attribute was designed
to serve only as an override to default PAC type configured in
IPA config. With PAC type set in all services, users would have
to update all services to get new PAC types configured in IPA config.

Do not set PAC type for new services. Add new NONE value meaning that
we do not want any PAC for the service (empty/missing attribute means
that the default PAC type list from IPA config is read).

https://fedorahosted.org/freeipa/ticket/2184

API.txt

@@ -2738,7 +2738,7 @@ command: service_add
 args: 1,8,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, required=True)
 option: Bytes('usercertificate', attribute=True, cli_name='certificate', multivalue=False, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('force', autofill=True, default=False)
@@ -2775,7 +2775,7 @@ command: service_find
 args: 1,10,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='principal', multivalue=False, primary_key=True, query=True, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Int('timelimit?', autofill=False, minvalue=0)
 option: Int('sizelimit?', autofill=False, minvalue=0)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2792,7 +2792,7 @@ command: service_mod
 args: 1,9,3
 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, query=True, required=True)
 option: Bytes('usercertificate', attribute=True, autofill=False, cli_name='certificate', multivalue=False, required=False)
-option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD'))
+option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE'))
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')

VERSION

@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=43
+IPA_API_VERSION_MINOR=44

ipalib/plugins/config.py

@@ -192,8 +192,8 @@ class config(LDAPObject):
         ),
         StrEnum('ipakrbauthzdata*',
             cli_name='pac_type',
-            label=_('PAC type'),
+            label=_('Default PAC types'),
-            doc=_('Default types of PAC for new services'),
+            doc=_('Default types of PAC supported for services'),
             values=(u'MS-PAC', u'PAD'),
             csv=True,
         ),

ipalib/plugins/service.py

@@ -60,8 +60,11 @@ EXAMPLES:
    ipa service-add HTTP/web.example.com
 
  Allow a host to manage an IPA service certificate:
-  ipa service-add-host --hosts=web.example.com HTTP/web.example.com
+   ipa service-add-host --hosts=web.example.com HTTP/web.example.com
-  ipa role-add-member --hosts=web.example.com certadmin
+   ipa role-add-member --hosts=web.example.com certadmin
+
+ Override a default list of supported PAC types for the service:
+   ipa service-mod HTTP/web.example.com --pac-type=MS-PAC
 
  Delete an IPA service:
    ipa service-del HTTP/web.example.com
@@ -253,12 +256,28 @@ class service(LDAPObject):
         StrEnum('ipakrbauthzdata*',
             cli_name='pac_type',
             label=_('PAC type'),
-            doc=_('Types of PAC this service supports'),
+            doc=_("Override default list of supported PAC types."
-            values=(u'MS-PAC', u'PAD'),
+                  " Use 'NONE' to disable PAC support for this service"),
+            values=(u'MS-PAC', u'PAD', u'NONE'),
             csv=True,
         ),
     )
 
+    def validate_ipakrbauthzdata(self, entry):
+        new_value = entry.get('ipakrbauthzdata', [])
+
+        if not new_value:
+            return
+
+        if not isinstance(new_value, (list, tuple)):
+            new_value = set([new_value])
+        else:
+            new_value = set(new_value)
+
+        if u'NONE' in new_value and len(new_value) > 1:
+            raise errors.ValidationError(name='ipakrbauthzdata',
+                error=_('NONE value cannot be combined with other PAC types'))
+
 api.register(service)
 
 
@@ -287,6 +306,8 @@ class service_add(LDAPCreate):
                 reason=_("The host '%s' does not exist to add a service to.") %
                     hostname)
 
+        self.obj.validate_ipakrbauthzdata(entry_attrs)
+
         cert = options.get('usercertificate')
         if cert:
             dercert = x509.normalize_certificate(cert)
@@ -300,11 +321,6 @@ class service_add(LDAPCreate):
              util.validate_host_dns(self.log, hostname)
         if not 'managedby' in entry_attrs:
             entry_attrs['managedby'] = hostresult['dn']
-        if 'ipakrbauthzdata' not in entry_attrs:
-            config = ldap.get_ipa_config()[1]
-            default_pac_type = config.get('ipakrbauthzdata', [])
-            if default_pac_type:
-                entry_attrs['ipakrbauthzdata'] = default_pac_type
 
         # Enforce ipaKrbPrincipalAlias to aid case-insensitive searches
         # as krbPrincipalName/krbCanonicalName are case-sensitive in Kerberos
@@ -372,6 +388,9 @@ class service_mod(LDAPUpdate):
 
     def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
+
+        self.obj.validate_ipakrbauthzdata(entry_attrs)
+
         if 'usercertificate' in options:
             (service, hostname, realm) = split_principal(keys[-1])
             cert = options.get('usercertificate')

tests/test_xmlrpc/test_host_plugin.py

@@ -654,7 +654,6 @@ class test_host(Declarative):
                     krbprincipalname=[service1],
                     objectclass=objectclasses.service,
                     managedby_host=[fqdn1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     ipauniqueid=[fuzzy_uuid],
                 ),
             ),

tests/test_xmlrpc/test_service_plugin.py

@@ -181,7 +181,6 @@ class test_service(Declarative):
                     krbprincipalname=[service1],
                     objectclass=objectclasses.service,
                     ipauniqueid=[fuzzy_uuid],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -210,7 +209,6 @@ class test_service(Declarative):
                     dn=service1dn,
                     krbprincipalname=[service1],
                     has_keytab=False,
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -230,7 +228,6 @@ class test_service(Declarative):
                     objectclass=objectclasses.service,
                     ipauniqueid=[fuzzy_uuid],
                     managedby_host=[fqdn1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     has_keytab=False
                 ),
             ),
@@ -249,7 +246,6 @@ class test_service(Declarative):
                         dn=service1dn,
                         krbprincipalname=[service1],
                         managedby_host=[fqdn1],
-                        ipakrbauthzdata=[u'MS-PAC'],
                         has_keytab=False,
                     ),
                 ],
@@ -271,7 +267,6 @@ class test_service(Declarative):
                         ipakrbprincipalalias=[service1],
                         objectclass=objectclasses.service,
                         ipauniqueid=[fuzzy_uuid],
-                        ipakrbauthzdata=[u'MS-PAC'],
                         has_keytab=False,
                         managedby_host=[fqdn1],
                     ),
@@ -289,7 +284,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -305,7 +299,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -321,7 +314,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1, fqdn2],
                 ),
             ),
@@ -337,7 +329,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -353,7 +344,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1, fqdn3.lower()],
                 ),
             ),
@@ -369,7 +359,6 @@ class test_service(Declarative):
                 result=dict(
                     dn=service1dn,
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                 ),
             ),
@@ -394,7 +383,6 @@ class test_service(Declarative):
                 result=dict(
                     usercertificate=[base64.b64decode(servercert)],
                     krbprincipalname=[service1],
-                    ipakrbauthzdata=[u'MS-PAC'],
                     managedby_host=[fqdn1],
                     valid_not_before=fuzzy_date,
                     valid_not_after=fuzzy_date,
@@ -409,6 +397,42 @@ class test_service(Declarative):
         ),
 
 
+        dict(
+            desc='Try to update %r with invalid ipakrbauthz data '
+                 'combination' % service1,
+            command=('service_mod', [service1],
+                dict(ipakrbauthzdata=[u'MS-PAC', u'NONE'])),
+            expected=errors.ValidationError(name='ipakrbauthzdata',
+                error=u'NONE value cannot be combined with other PAC types')
+        ),
+
+
+        dict(
+            desc='Update %r with valid ipakrbauthz data '
+                 'combination' % service1,
+            command=('service_mod', [service1],
+                dict(ipakrbauthzdata=[u'MS-PAC'])),
+            expected=dict(
+                value=service1,
+                summary=u'Modified service "%s"' % service1,
+                result=dict(
+                    usercertificate=[base64.b64decode(servercert)],
+                    krbprincipalname=[service1],
+                    managedby_host=[fqdn1],
+                    ipakrbauthzdata=[u'MS-PAC'],
+                    valid_not_before=fuzzy_date,
+                    valid_not_after=fuzzy_date,
+                    subject=DN(('CN',api.env.host),x509.subject_base()),
+                    serial_number=fuzzy_digits,
+                    serial_number_hex=fuzzy_hex,
+                    md5_fingerprint=fuzzy_hash,
+                    sha1_fingerprint=fuzzy_hash,
+                    issuer=fuzzy_issuer,
+                ),
+            ),
+        ),
+
+
         dict(
             desc='Retrieve %r to verify update' % service1,
             command=('service_show', [service1], {}),