From 0a79836082b897ebf734d9073583769cb276937c Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Tue, 21 Dec 2010 22:39:55 -0500
Subject: Setting an empty set of target attributes should raise an exception.

It is possible to create an ACI with attributes and then try to set that
to None via a mod command later. We need to catch this and raise an exception.

If all attributes are set to None in an aci then the attr target is removed
from the ACI. This could result in an illegal ACI if there are no other
targets. Having no targets is a legal state, just not a legal final state.

ticket 647
---
 ipalib/aci.py                 |  4 +++
 ipalib/errors.py              |  4 +--
 ipalib/plugins/aci.py         | 61 +++++++++++++++++++++++--------------------
 ipalib/plugins/selfservice.py |  2 ++
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/ipalib/aci.py b/ipalib/aci.py
index fc94126a3..abb2ebc49 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -175,6 +175,10 @@ class ACI:
         self.target['targetfilter']['operator'] = operator
 
     def set_target_attr(self, attr, operator="="):
+        if not attr:
+            if 'targetattr' in self.target:
+                del self.target['targetattr']
+            return
         if not type(attr) in (tuple, list):
             attr = [attr]
         self.target['targetattr'] = {}
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 62c42feac..2cafb0109 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1319,11 +1319,11 @@ class OnlyOneValueAllowed(ExecutionError):
 
 class InvalidSyntax(ExecutionError):
     """
-    **4208** Raised when trying to set more than one value to single-value attributes
+    **4208** Raised when an value does not match the required syntax
 
     For example:
 
-    >> raise OnlyOneValueAllowed(attr='ipahomesrootdir')
+    >> raise InvalidSyntax(attr='ipahomesrootdir')
     Traceback (most recent call last):
       ...
     InvalidSyntax: ipahomesrootdir: Invalid syntax
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index ca0277afe..0193be5db 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -207,35 +207,38 @@ def _make_aci(current, aciname, kw):
         except errors.NotFound:
             raise errors.NotFound(reason=_("Group '%s' does not exist") % kw['group'])
 
-    a = ACI(current)
-    a.name = aciname
-    a.permissions = kw['permissions']
-    if 'selfaci' in kw and kw['selfaci']:
-        a.set_bindrule('userdn = "ldap:///self"')
-    else:
-        dn = entry_attrs['dn']
-        a.set_bindrule('groupdn = "ldap:///%s"' % dn)
-    if 'attrs' in kw:
-        a.set_target_attr(kw['attrs'])
-    if 'memberof' in kw:
-        entry_attrs = api.Command['group_show'](kw['memberof'])['result']
-        a.set_target_filter('memberOf=%s' % entry_attrs['dn'])
-    if 'filter' in kw:
-        a.set_target_filter(kw['filter'])
-    if 'type' in kw:
-        target = _type_map[kw['type']]
-        a.set_target(target)
-    if 'targetgroup' in kw:
-        # Purposely no try here so we'll raise a NotFound
-        entry_attrs = api.Command['group_show'](kw['targetgroup'])['result']
-        target = 'ldap:///%s' % entry_attrs['dn']
-        a.set_target(target)
-    if 'subtree' in kw:
-        # See if the subtree is a full URI
-        target = kw['subtree']
-        if not target.startswith('ldap:///'):
-            target = 'ldap:///%s' % target
-        a.set_target(target)
+    try:
+        a = ACI(current)
+        a.name = aciname
+        a.permissions = kw['permissions']
+        if 'selfaci' in kw and kw['selfaci']:
+            a.set_bindrule('userdn = "ldap:///self"')
+        else:
+            dn = entry_attrs['dn']
+            a.set_bindrule('groupdn = "ldap:///%s"' % dn)
+        if 'attrs' in kw:
+            a.set_target_attr(kw['attrs'])
+        if 'memberof' in kw:
+            entry_attrs = api.Command['group_show'](kw['memberof'])['result']
+            a.set_target_filter('memberOf=%s' % entry_attrs['dn'])
+        if 'filter' in kw:
+            a.set_target_filter(kw['filter'])
+        if 'type' in kw:
+            target = _type_map[kw['type']]
+            a.set_target(target)
+        if 'targetgroup' in kw:
+            # Purposely no try here so we'll raise a NotFound
+            entry_attrs = api.Command['group_show'](kw['targetgroup'])['result']
+            target = 'ldap:///%s' % entry_attrs['dn']
+            a.set_target(target)
+        if 'subtree' in kw:
+            # See if the subtree is a full URI
+            target = kw['subtree']
+            if not target.startswith('ldap:///'):
+                target = 'ldap:///%s' % target
+            a.set_target(target)
+    except SyntaxError, e:
+        raise errors.ValidationError(name='target', error=_('Syntax Error: %(error)s') % dict(error=str(e)))
 
     return a
 
diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py
index 9152895a5..b5f754ee8 100644
--- a/ipalib/plugins/selfservice.py
+++ b/ipalib/plugins/selfservice.py
@@ -157,6 +157,8 @@ class selfservice_mod(crud.Update):
 
     def execute(self, aciname, **kw):
         is_selfservice(aciname)
+        if 'attrs' in kw and kw['attrs'] is None:
+            raise errors.RequirementError(name='attrs')
         result = api.Command['aci_mod'](aciname, **kw)['result']
         return dict(
             result=result,
-- 
cgit