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

6 files changed, 70 insertions, 28 deletions

diff --git a/API.txt b/API.txt
index 1906e22f..7bd046c8 100644
--- a/API.txt
+++ b/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')
diff --git a/VERSION b/VERSION
index 962d476e..c1f1bcef 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=43
+IPA_API_VERSION_MINOR=44
diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
index 1c62e0d9..5f916903 100644
--- a/ipalib/plugins/config.py
+++ b/ipalib/plugins/config.py
@@ -192,8 +192,8 @@ class config(LDAPObject):
         ),
         StrEnum('ipakrbauthzdata*',
             cli_name='pac_type',
-            label=_('PAC type'),
-            doc=_('Default types of PAC for new services'),
+            label=_('Default PAC types'),
+            doc=_('Default types of PAC supported for services'),
             values=(u'MS-PAC', u'PAD'),
             csv=True,
         ),
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index a7201f52..120eb607 100644
--- a/ipalib/plugins/service.py
+++ b/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 role-add-member --hosts=web.example.com certadmin
+   ipa service-add-host --hosts=web.example.com HTTP/web.example.com
+   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'),
-            values=(u'MS-PAC', u'PAD'),
+            doc=_("Override default list of supported PAC types."
+                  " 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')
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 2010af8a..37b7e407 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/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],
                 ),
             ),
diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py
index a76bc918..29c94e31 100644
--- a/tests/test_xmlrpc/test_service_plugin.py
+++ b/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,
@@ -410,6 +398,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], {}),
             expected=dict(