From c09927d16a507456802db95560b5422b921bfb4f Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Dec 11 2018 11:14:32 +0000
Subject: Handle service_del with bad service name


The command 'ipa service-del badservice' used to fail with an internal
server error, because check_required_principal() could not handle a
principal that is not a service principal. All del commands have less
strict error checking of primary keys so they can reference any stored
key, even illegal ones.

check_required_principal() skips required principal check if the
principal is not a service principal. A non-service principal can never
be a required principal.

Fixes: https://pagure.io/freeipa/issue/7793
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

---

diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 00d1990..4bc4408 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -276,9 +276,13 @@ def set_certificate_attrs(entry_attrs):
 
 def check_required_principal(ldap, principal):
     """
-    Raise an error if the host of this prinicipal is an IPA master and one
+    Raise an error if the host of this principal is an IPA master and one
     of the principals required for proper execution.
     """
+    if not principal.is_service:
+        # bypass check if principal is not a service principal,
+        # see https://pagure.io/freeipa/issue/7793
+        return
     try:
         host_is_master(ldap, principal.hostname)
     except errors.ValidationError:
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index 665d815..262fae1 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -42,6 +42,7 @@ fqdn2 = u'testhost2.%s' % api.env.domain
 fqdn3 = u'TestHost3.%s' % api.env.domain
 service1_no_realm = u'HTTP/%s' % fqdn1
 service1 = u'%s@%s' % (service1_no_realm, api.env.realm)
+badservice = u'badservice@%s' % api.env.realm  # no hostname
 hostprincipal1 = u'host/%s@%s'  % (fqdn1, api.env.realm)
 service1dn = DN(('krbprincipalname',service1),('cn','services'),('cn','accounts'),api.env.basedn)
 host1dn = DN(('fqdn',fqdn1),('cn','computers'),('cn','accounts'),api.env.basedn)
@@ -119,6 +120,12 @@ class test_service(Declarative):
                 reason=u'%s: service not found' % service1),
         ),
 
+        dict(
+            desc='Try to delete service without hostname %r' % badservice,
+            command=('service_del', [badservice], {}),
+            expected=errors.NotFound(
+                reason=u'%s: service not found' % badservice),
+        ),
 
         dict(
             desc='Create %r' % fqdn1,
@@ -757,6 +764,16 @@ class test_service(Declarative):
                 reason=u'%s: service not found' % service1),
         ),
 
+        dict(
+            desc='Try to update service without hostname %r' % badservice,
+            command=(
+                'service_mod',
+                [badservice],
+                dict(usercertificate=servercert)
+            ),
+            expected=errors.NotFound(
+                reason=u'%s: service not found' % badservice),
+        ),
 
         dict(
             desc='Try to delete non-existent %r' % service1,