URL: https://github.com/freeipa/freeipa/pull/298
Author: frasertweedale
 Title: #298: ipaldap: handle binary encoding option transparently
Action: opened

PR body:
"""
This patchset addresses
  https://fedorahosted.org/freeipa/ticket/6529.  I'm publishing it for
  discussion and review but it should not be hastily merged because
  there are compatibility implications for older server versions
  (discussed in https://fedorahosted.org/freeipa/ticket/6530).

Per RFC 4523, particular attribute syntaxes require use of the
';binary' attribute option.  Attributes using these syntaxes include
'userCertificate' and 'cACertificate'.  Our handling of this requirement is
inconsistent with no library support (i.e. it was up to individual plugin
authors to "do the right thing".

Also, because 389 DS currently does not always use this encoding option
(which is a defect), whether you need to read the
'userCertificate' or 'userCertificate;binary' attribute - or both - is
often unclear.  But technically, these both refer to the same attribute,
because the 'binary' option does not specify an attribute subtype.

This commit implements proper handling of the binary encoding option within
ipaldap.  Plugin code should now always use an attribute description
*without* the binary encoding option, and allow ipaldap to automatically
add it when sending attribute to the server, and strip it when reading
attributes in search results.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/298/head:pr298
git checkout pr298
From 6e8d83f2871551912d2837f1a1e0242125153f45 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Fri, 2 Dec 2016 14:20:42 +1000
Subject: [PATCH 1/2] ipaldap: handle binary encoding option transparently

Per RFC 4523, particular attribute syntaxes require use of the
';binary' attribute option.  Attributes using these syntaxes include
'userCertificate' and 'cACertificate'.  Our handling of this
requirement is inconsistent with no library support (i.e. it was up
to individual plugin authors to "do the right thing".

Also, because 389 DS currently does not always use this encoding
option (which is a defect), whether you need to read the
'userCertificate' or 'userCertificate;binary' attribute - or both -
is often unclear.  But technically, these both refer to the same
attribute, because the 'binary' option does not specify an attribute
subtype.

This commit implements proper handling of the binary encoding option
within ipaldap.  Plugin code should now always use an attribute
description *without* the binary encoding option, and allow ipaldap
to automatically add it when sending attribute to the server, and
strip it when reading attributes in search results.

A follow-up commit will remove explicit use of the binary encoding
option from code that uses ipaldap.

Part of: https://fedorahosted.org/freeipa/ticket/6529
---
 ipapython/ipaldap.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 94a5e1e..6828ad0 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -632,6 +632,22 @@ class LDAPClient(object):
     SCOPE_ONELEVEL = ldap.SCOPE_ONELEVEL
     SCOPE_SUBTREE = ldap.SCOPE_SUBTREE
 
+    _ENCODING_OPTION_BINARY = 'binary'
+
+    # although per RFC 4523 it is the /syntax/ that defines whether
+    # ;binary is required, 389DS punted on implementing the RFC 4523
+    # syntaxes, and uses 1.3.6.1.4.1.1466.115.121.1.40 Octet String
+    # instead.  So we must map by attribute name.
+    _ENCODING_OPTIONS = CIDict({
+        'userCertificate':              _ENCODING_OPTION_BINARY,
+        'cACertificate':                _ENCODING_OPTION_BINARY,
+        'crossCertificatePair':         _ENCODING_OPTION_BINARY,
+        'certificateRevocationList':    _ENCODING_OPTION_BINARY,
+        'authorityRevocationList':      _ENCODING_OPTION_BINARY,
+        'deltaRevocationList':          _ENCODING_OPTION_BINARY,
+        'supportedAlgorithms':          _ENCODING_OPTION_BINARY,
+    })
+
     _SYNTAX_MAPPING = {
         '1.3.6.1.4.1.1466.115.121.1.1'   : bytes, # ACI item
         '1.3.6.1.4.1.1466.115.121.1.4'   : bytes, # Audio
@@ -756,8 +772,14 @@ def modify_s(self, dn, modlist):
         # FIXME: for backwards compatibility only
         assert isinstance(dn, DN)
         dn = str(dn)
-        modlist = [(a, self.encode(b), self.encode(c)) for a, b, c in modlist]
-        return self.conn.modify_s(dn, modlist)
+        modlist_encoded = []
+        for modtype, attr, vals in modlist:
+            modlist_encoded.append((
+                modtype,
+                self.add_encoding_options(self.encode(attr)),
+                self.encode(vals)
+            ))
+        return self.conn.modify_s(dn, modlist_encoded)
 
     @property
     def conn(self):
@@ -831,6 +853,23 @@ def get_attribute_type(self, name_or_oid):
 
         return unicode
 
+    def add_encoding_options(self, name_or_oid):
+        """Add required encoding options, if any."""
+        option = self._ENCODING_OPTIONS.get(name_or_oid)
+        if option is not None:
+            name_or_oid = b';'.join([name_or_oid, option])
+        return name_or_oid
+
+    def strip_encoding_options(self, name):
+        """Remove non-subtyping encoding options from attribute description."""
+        parts = name.split(';')
+        parts[1:] = [
+            option for option in parts[1:]
+            if option != self._ENCODING_OPTIONS.get(parts[0])
+        ]
+        name = ';'.join(parts)
+        return name
+
     def has_dn_syntax(self, name_or_oid):
         """
         Check the schema to see if the attribute uses DN syntax.
@@ -886,7 +925,10 @@ def encode(self, val):
         elif isinstance(val, tuple):
             return tuple(self.encode(m) for m in val)
         elif isinstance(val, dict):
-            dct = dict((self.encode(k), self.encode(v)) for k, v in val.items())
+            dct = dict(
+                (self.add_encoding_options(self.encode(k)), self.encode(v))
+                for k, v in val.items()
+            )
             return dct
         elif isinstance(val, datetime.datetime):
             return val.strftime(LDAP_GENERALIZED_TIME_FORMAT)
@@ -956,6 +998,7 @@ def _convert_result(self, result):
             ipa_entry = LDAPEntry(self, DN(original_dn))
 
             for attr, original_values in original_attrs.items():
+                attr = self.strip_encoding_options(attr)
                 ipa_entry.raw[attr] = original_values
             ipa_entry.reset_modlist()
 
@@ -1559,9 +1602,7 @@ def update_entry(self, entry):
 
         # pass arguments to python-ldap
         with self.error_handler():
-            modlist = [(a, str(b), self.encode(c))
-                       for a, b, c in modlist]
-            self.conn.modify_s(str(entry.dn), modlist)
+            self.modify_s(entry.dn, modlist)
 
         entry.reset_modlist()
 

From f0631100c23f0cba510f421cf60bbe6e1cf48bb0 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Fri, 2 Dec 2016 14:39:22 +1000
Subject: [PATCH 2/2] Remove explicit use of LDAP binary encoding option

ipaldap now handles the binary encoding option automatically for
those attribute types that require it.  Avoid explicit use of this
option in code that uses ipaldap.

Fixes: https://fedorahosted.org/freeipa/ticket/6529
---
 ipalib/install/certstore.py                 | 22 +++++++++---------
 ipaserver/install/plugins/upload_cacrt.py   |  6 ++---
 ipaserver/plugins/baseuser.py               | 18 +--------------
 ipaserver/plugins/cert.py                   | 31 +++++++++++++------------
 ipaserver/plugins/idviews.py                | 35 +----------------------------
 ipaserver/plugins/user.py                   | 30 -------------------------
 ipatests/test_xmlrpc/tracker/user_plugin.py |  2 +-
 7 files changed, 32 insertions(+), 112 deletions(-)

diff --git a/ipalib/install/certstore.py b/ipalib/install/certstore.py
index 70ae942..37bd2ba 100644
--- a/ipalib/install/certstore.py
+++ b/ipalib/install/certstore.py
@@ -68,7 +68,7 @@ def init_ca_entry(entry, dercert, nickname, trusted, ext_key_usage):
     entry['ipaCertSubject'] = [subject]
     entry['ipaCertIssuerSerial'] = [issuer_serial]
     entry['ipaPublicKey'] = [public_key]
-    entry['cACertificate;binary'] = [dercert]
+    entry['cACertificate'] = [dercert]
 
     if trusted is not None:
         entry['ipaKeyTrust'] = ['trusted' if trusted else 'distrusted']
@@ -85,14 +85,14 @@ def update_compat_ca(ldap, base_dn, dercert):
     """
     dn = DN(('cn', 'CAcert'), ('cn', 'ipa'), ('cn', 'etc'), base_dn)
     try:
-        entry = ldap.get_entry(dn, attrs_list=['cACertificate;binary'])
-        entry.single_value['cACertificate;binary'] = dercert
+        entry = ldap.get_entry(dn, attrs_list=['cACertificate'])
+        entry.single_value['cACertificate'] = dercert
         ldap.update_entry(entry)
     except errors.NotFound:
         entry = ldap.make_entry(dn)
         entry['objectClass'] = ['nsContainer', 'pkiCA']
         entry.single_value['cn'] = 'CAcert'
-        entry.single_value['cACertificate;binary'] = dercert
+        entry.single_value['cACertificate'] = dercert
         ldap.add_entry(entry)
     except errors.EmptyModlist:
         pass
@@ -166,11 +166,11 @@ def update_ca_cert(ldap, base_dn, dercert, trusted=None, ext_key_usage=None,
         filter=filter,
         attrs_list=['cn', 'ipaCertSubject', 'ipaCertIssuerSerial',
                     'ipaPublicKey', 'ipaKeyTrust', 'ipaKeyExtUsage',
-                    'ipaConfigString', 'cACertificate;binary'])
+                    'ipaConfigString', 'cACertificate'])
     entry = result[0]
     dn = entry.dn
 
-    for old_cert in entry['cACertificate;binary']:
+    for old_cert in entry['cACertificate']:
         # Check if we are adding a new cert
         if old_cert == dercert:
             break
@@ -181,7 +181,7 @@ def update_ca_cert(ldap, base_dn, dercert, trusted=None, ext_key_usage=None,
         if entry.single_value['ipaPublicKey'] != public_key:
             raise ValueError("subject public key info mismatch")
         entry['ipaCertIssuerSerial'].append(issuer_serial)
-        entry['cACertificate;binary'].append(dercert)
+        entry['cACertificate'].append(dercert)
 
     # Update key trust
     if trusted is not None:
@@ -288,7 +288,7 @@ def get_ca_certs(ldap, base_dn, compat_realm, compat_ipa_ca,
             filter=ldap.combine_filters(filters, ldap.MATCH_ALL),
             attrs_list=['cn', 'ipaCertSubject', 'ipaCertIssuerSerial',
                         'ipaPublicKey', 'ipaKeyTrust', 'ipaKeyExtUsage',
-                        'cACertificate;binary'])
+                        'cACertificate'])
 
         for entry in result:
             nickname = entry.single_value['cn']
@@ -304,7 +304,7 @@ def get_ca_certs(ldap, base_dn, compat_realm, compat_ipa_ca,
                 ext_key_usage = set(str(p) for p in ext_key_usage)
                 ext_key_usage.discard(x509.EKU_PLACEHOLDER)
 
-            for cert in entry.get('cACertificate;binary', []):
+            for cert in entry.get('cACertificate', []):
                 try:
                     _parse_cert(cert)
                 except ValueError:
@@ -317,9 +317,9 @@ def get_ca_certs(ldap, base_dn, compat_realm, compat_ipa_ca,
         except errors.NotFound:
             # Fallback to cn=CAcert,cn=ipa,cn=etc,SUFFIX
             dn = DN(('cn', 'CAcert'), config_dn)
-            entry = ldap.get_entry(dn, ['cACertificate;binary'])
+            entry = ldap.get_entry(dn, ['cACertificate'])
 
-            cert = entry.single_value['cACertificate;binary']
+            cert = entry.single_value['cACertificate']
             try:
                 subject, _issuer_serial, _public_key_info = _parse_cert(cert)
             except ValueError:
diff --git a/ipaserver/install/plugins/upload_cacrt.py b/ipaserver/install/plugins/upload_cacrt.py
index 1a78108..358a7c6 100644
--- a/ipaserver/install/plugins/upload_cacrt.py
+++ b/ipaserver/install/plugins/upload_cacrt.py
@@ -89,11 +89,11 @@ def execute(self, **options):
                 entry = ldap.make_entry(dn)
                 entry['objectclass'] = ['nsContainer', 'pkiCA']
                 entry.single_value['cn'] = 'CAcert'
-                entry.single_value['cACertificate;binary'] = ca_cert
+                entry.single_value['cACertificate'] = ca_cert
                 ldap.add_entry(entry)
             else:
-                if b'' in entry['cACertificate;binary']:
-                    entry.single_value['cACertificate;binary'] = ca_cert
+                if b'' in entry['cACertificate']:
+                    entry.single_value['cACertificate'] = ca_cert
                     ldap.update_entry(entry)
 
         return False, []
diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py
index 4c7e9f0..be581c3 100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -149,7 +149,7 @@ class baseuser(LDAPObject):
         'telephonenumber', 'title', 'memberof', 'nsaccountlock',
         'memberofindirect', 'ipauserauthtype', 'userclass',
         'ipatokenradiusconfiglink', 'ipatokenradiususername',
-        'krbprincipalexpiration', 'usercertificate;binary',
+        'krbprincipalexpiration', 'usercertificate',
         'krbprincipalname', 'krbcanonicalname'
     ]
     search_display_attributes = [
@@ -432,16 +432,6 @@ def delete_user(self, user):
         assert isinstance(user, DN)
         return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
 
-    def convert_usercertificate_pre(self, entry_attrs):
-        if 'usercertificate' in entry_attrs:
-            entry_attrs['usercertificate;binary'] = entry_attrs.pop(
-                'usercertificate')
-
-    def convert_usercertificate_post(self, entry_attrs, **options):
-        if 'usercertificate;binary' in entry_attrs:
-            entry_attrs['usercertificate'] = entry_attrs.pop(
-                'usercertificate;binary')
-
     def convert_attribute_members(self, entry_attrs, *keys, **options):
         super(baseuser, self).convert_attribute_members(
             entry_attrs, *keys, **options)
@@ -470,11 +460,9 @@ def pre_common_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
                             **options):
         assert isinstance(dn, DN)
         set_krbcanonicalname(entry_attrs)
-        self.obj.convert_usercertificate_pre(entry_attrs)
 
     def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
         convert_sshpubkey_post(entry_attrs)
         radius_dn2pk(self.api, entry_attrs)
@@ -602,7 +590,6 @@ def pre_common_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
         self.check_userpassword(entry_attrs, **options)
 
         self.check_objectclass(ldap, dn, entry_attrs)
-        self.obj.convert_usercertificate_pre(entry_attrs)
         self.preserve_krbprincipalname_pre(ldap, entry_attrs, *keys, **options)
 
     def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -616,7 +603,6 @@ def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options):
                 pass
         convert_nsaccountlock(entry_attrs)
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         convert_sshpubkey_post(entry_attrs)
         remove_sshpubkey_from_output_post(self.context, entry_attrs)
         radius_dn2pk(self.api, entry_attrs)
@@ -650,7 +636,6 @@ def pre_common_callback(self, ldap, filters, attrs_list, base_dn, scope,
 
     def post_common_callback(self, ldap, entries, lockout=False, **options):
         for attrs in entries:
-            self.obj.convert_usercertificate_post(attrs, **options)
             if (lockout):
                 attrs['nsaccountlock'] = True
             else:
@@ -669,7 +654,6 @@ def pre_common_callback(self, ldap, dn, attrs_list, *keys, **options):
     def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         convert_sshpubkey_post(entry_attrs)
         remove_sshpubkey_from_output_post(self.context, entry_attrs)
         radius_dn2pk(self.api, entry_attrs)
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 3571ef1..5b14d6c 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -1310,24 +1310,23 @@ def _ldap_search(self, all, raw, pkey_only, no_members, timelimit,
             truncated = bool(truncated)
 
         for entry in entries:
-            for attr in ('usercertificate', 'usercertificate;binary'):
-                for cert in entry.get(attr, []):
-                    try:
-                        key = self._get_cert_key(cert)
-                    except ValueError:
-                        truncated = True
-                        continue
+            for cert in entry.get('usercertificate', []):
+                try:
+                    key = self._get_cert_key(cert)
+                except ValueError:
+                    truncated = True
+                    continue
 
-                    try:
-                        obj = result[key]
-                    except KeyError:
-                        obj = self._get_cert_obj(cert, all, raw, pkey_only)
-                        result[key] = obj
+                try:
+                    obj = result[key]
+                except KeyError:
+                    obj = self._get_cert_obj(cert, all, raw, pkey_only)
+                    result[key] = obj
 
-                    if not pkey_only and (all or not no_members):
-                        owners = obj.setdefault('owner', [])
-                        if entry.dn not in owners:
-                            owners.append(entry.dn)
+                if not pkey_only and (all or not no_members):
+                    owners = obj.setdefault('owner', [])
+                    if entry.dn not in owners:
+                        owners.append(entry.dn)
 
         if not raw:
             for obj in six.itervalues(result):
diff --git a/ipaserver/plugins/idviews.py b/ipaserver/plugins/idviews.py
index 3ee2be4..7a77264 100644
--- a/ipaserver/plugins/idviews.py
+++ b/ipaserver/plugins/idviews.py
@@ -835,7 +835,7 @@ class idoverrideuser(baseidoverride):
     possible_objectclasses = ['ipasshuser', 'ipaSshGroupOfPubKeys']
     default_attributes = baseidoverride.default_attributes + [
        'homeDirectory', 'uidNumber', 'uid', 'ipaOriginalUid', 'loginShell',
-       'ipaSshPubkey', 'gidNumber', 'gecos', 'usercertificate;binary',
+       'ipaSshPubkey', 'gidNumber', 'gecos', 'usercertificate',
     ]
 
     search_display_attributes = baseidoverride.default_attributes + [
@@ -907,17 +907,6 @@ def update_original_uid_reference(self, entry_attrs):
             # we have no way to update the original_uid
             pass
 
-    def convert_usercertificate_pre(self, entry_attrs):
-        if 'usercertificate' in entry_attrs:
-            entry_attrs['usercertificate;binary'] = entry_attrs.pop(
-                'usercertificate')
-
-    def convert_usercertificate_post(self, entry_attrs, **options):
-        if 'usercertificate;binary' in entry_attrs:
-            entry_attrs['usercertificate'] = entry_attrs.pop(
-                'usercertificate;binary')
-
-
 
 @register()
 class idoverridegroup(baseidoverride):
@@ -974,16 +963,8 @@ class idoverrideuser_add_cert(LDAPAddAttributeViaOption):
     takes_options = LDAPAddAttributeViaOption.takes_options + (
         fallback_to_ldap_option,)
 
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
-                     **options):
-        dn = self.obj.get_dn(*keys, **options)
-        self.obj.convert_usercertificate_pre(entry_attrs)
-
-        return dn
-
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         self.obj.convert_anchor_to_human_readable_form(entry_attrs, **options)
         return dn
 
@@ -997,16 +978,8 @@ class idoverrideuser_remove_cert(LDAPRemoveAttributeViaOption):
     takes_options = LDAPRemoveAttributeViaOption.takes_options + (
         fallback_to_ldap_option,)
 
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
-                     **options):
-        dn = self.obj.get_dn(*keys, **options)
-        self.obj.convert_usercertificate_pre(entry_attrs)
-
-        return dn
-
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         self.obj.convert_anchor_to_human_readable_form(entry_attrs, **options)
 
         return dn
@@ -1022,7 +995,6 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
                  entry_attrs, attrs_list, *keys, **options)
 
         entry_attrs['objectclass'].append('ipasshuser')
-        self.obj.convert_usercertificate_pre(entry_attrs)
 
         # Update the ipaOriginalUid
         self.obj.update_original_uid_reference(entry_attrs)
@@ -1032,7 +1004,6 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         dn = super(idoverrideuser_add, self).post_callback(ldap, dn,
                  entry_attrs, *keys, **options)
         convert_sshpubkey_post(entry_attrs)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         return dn
 
 
@@ -1064,14 +1035,12 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         if 'ipasshpubkey' in entry_attrs and 'ipasshuser' not in obj_classes:
             obj_classes.append('ipasshuser')
 
-        self.obj.convert_usercertificate_pre(entry_attrs)
         return dn
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         dn = super(idoverrideuser_mod, self).post_callback(ldap, dn,
                  entry_attrs, *keys, **options)
         convert_sshpubkey_post(entry_attrs)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         return dn
 
 
@@ -1086,7 +1055,6 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
             ldap, entries, truncated, *args, **options)
         for entry in entries:
             convert_sshpubkey_post(entry)
-            self.obj.convert_usercertificate_post(entry, **options)
         return truncated
 
 
@@ -1098,7 +1066,6 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         dn = super(idoverrideuser_show, self).post_callback(ldap, dn,
                  entry_attrs, *keys, **options)
         convert_sshpubkey_post(entry_attrs)
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
         return dn
 
 
diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py
index 5296093..a714b9b 100644
--- a/ipaserver/plugins/user.py
+++ b/ipaserver/plugins/user.py
@@ -1163,21 +1163,6 @@ class user_add_cert(LDAPAddAttributeViaOption):
     msg_summary = _('Added certificates to user "%(value)s"')
     attribute = 'usercertificate'
 
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
-                     **options):
-        dn = self.obj.get_either_dn(*keys, **options)
-
-        self.obj.convert_usercertificate_pre(entry_attrs)
-
-        return dn
-
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
-
-        return dn
-
 
 @register()
 class user_remove_cert(LDAPRemoveAttributeViaOption):
@@ -1185,21 +1170,6 @@ class user_remove_cert(LDAPRemoveAttributeViaOption):
     msg_summary = _('Removed certificates from user "%(value)s"')
     attribute = 'usercertificate'
 
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
-                     **options):
-        dn = self.obj.get_either_dn(*keys, **options)
-
-        self.obj.convert_usercertificate_pre(entry_attrs)
-
-        return dn
-
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-
-        self.obj.convert_usercertificate_post(entry_attrs, **options)
-
-        return dn
-
 
 @register()
 class user_add_manager(baseuser_add_manager):
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index 1b35a5c..06ee269 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -27,7 +27,7 @@ class UserTracker(KerberosAliasMixin, Tracker):
         u'telephonenumber', u'title', u'memberof', u'nsaccountlock',
         u'memberofindirect', u'ipauserauthtype', u'userclass',
         u'ipatokenradiusconfiglink', u'ipatokenradiususername',
-        u'krbprincipalexpiration', u'usercertificate;binary',
+        u'krbprincipalexpiration', u'usercertificate',
         u'has_keytab', u'has_password', u'memberof_group', u'sshpubkeyfp',
         u'krbcanonicalname', 'krbprincipalname'
     }
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to