From fe9be8c4a1aaca2a3bc3d8e5f0849a1716e19ca9 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Mar 16 2022 10:18:35 +0000
Subject: Convert values using _SYNTAX_MAPPING with --delattr


When an entry is loaded the incoming values are converted
into python datatypes automatically based on the _SYNTAX_MAPPING
value in ipaldap.

When using delattr to remove a mapped value it will fail because
the datatypes do not match up. For example date types are
datetime.datetime structions and won't match a generalized time
string.

So try to map the value to delete using _SYNTAX_MAPPING before
trying to remove the value. Fall back to trying to remove the
raw value if the mapping fails.

This won't work for some mapping types, DNs for example. Providing
only the RDN value for a DN-type, manager for example, lacks the
context to know how to construct the DN (RDN and contaner).

Fixes: https://pagure.io/freeipa/issue/9004

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

---

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index d0fbdc1..5c122d6 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -1135,7 +1135,11 @@ last, after all sets and adds."""),
 
                 for delval in deldict.get(attr, []):
                     try:
-                        entry_attrs[attr].remove(delval)
+                        try:
+                            val = ldap.decode(delval.encode('utf-8'), attr)
+                            entry_attrs[attr].remove(val)
+                        except ValueError:
+                            entry_attrs[attr].remove(delval)
                     except ValueError:
                         if isinstance(delval, bytes):
                             # This is a Binary value, base64 encode it
diff --git a/ipatests/test_xmlrpc/test_attr.py b/ipatests/test_xmlrpc/test_attr.py
index bbb1bf2..484fe4e 100644
--- a/ipatests/test_xmlrpc/test_attr.py
+++ b/ipatests/test_xmlrpc/test_attr.py
@@ -22,11 +22,14 @@
 Test --setattr and --addattr and other attribute-specific issues
 """
 
+from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, raises_exact
 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
 import pytest
 
+from datetime import datetime
+
 
 @pytest.fixture(scope='class')
 def user(request, xmlrpc_setup):
@@ -34,6 +37,12 @@ def user(request, xmlrpc_setup):
     return tracker.make_fixture(request)
 
 
+@pytest.fixture(scope='class')
+def manager(request, xmlrpc_setup):
+    tracker = UserTracker(name=u'manager', givenname=u'Test', sn=u'Manager')
+    return tracker.make_fixture(request)
+
+
 @pytest.mark.tier1
 class TestAttrOnUser(XMLRPC_test):
     def test_add_user_with_singlevalue_addattr(self):
@@ -188,6 +197,60 @@ class TestAttrOnUser(XMLRPC_test):
                          delattr=u'nsaccountlock=TRUE'),
                     dict(addattr='', delattr='', nsaccountlock=False))
 
+    def test_add_and_delete_datetime(self, user):
+        """ Delete a datetime data type """
+        user.ensure_exists()
+        # Set to a known value, then delete that value
+        expdate = u'20220210144006Z'
+        user.update(
+            dict(setattr=u'krbpasswordexpiration=' + expdate),
+            dict(krbpasswordexpiration=[
+                datetime.strptime(expdate, LDAP_GENERALIZED_TIME_FORMAT)
+            ], setattr='')
+        )
+        user.update(
+            dict(delattr=u'krbpasswordexpiration=' + expdate),
+            dict(delattr='')
+        )
+
+    def test_delete_nonexistent_datetime(self, user):
+        """ Delete a datetime data type that isn't in the entry """
+        user.ensure_exists()
+        expdate = u'20220210144006Z'
+        bad_expdate = u'20280210144006Z'
+        user.update(
+            dict(setattr=u'krbpasswordexpiration=' + expdate),
+            dict(krbpasswordexpiration=[
+                datetime.strptime(expdate, LDAP_GENERALIZED_TIME_FORMAT)
+            ], setattr='')
+        )
+        command = user.make_update_command(
+            dict(delattr=u'krbpasswordexpiration=' + bad_expdate),
+        )
+        with raises_exact(errors.AttrValueNotFound(
+                attr='krbpasswordexpiration', value=bad_expdate)):
+            command()
+
+    def test_add_and_delete_DN(self, user, manager):
+        """ Delete a DN data type """
+        user.ensure_exists()
+        manager.ensure_exists()
+        user.update(
+            dict(setattr=u'manager=manager'),
+            dict(manager=['manager'], setattr='')
+        )
+        command = user.make_update_command(
+            dict(delattr=u'manager=manager'),
+        )
+        # Setting works because the user plugin knows the container
+        # to convert a string to a DN. Passing in just the uid we
+        # don't have the context in ldap.decode() to know the entry
+        # type so `ipa user-mod someuser --delattr manager=foo` will
+        # fail.
+        with raises_exact(errors.AttrValueNotFound(
+                attr='manager', value='manager')):
+            command()
+
 
 @pytest.mark.tier1
 class TestAttrOnConfigs(XMLRPC_test):