On Wed, 2014-01-15 at 15:31 -0500, Rob Crittenden wrote:
> Simo Sorce wrote:
> > Uniform fallback for global_policy and stop explicitly adding the
> > default policy to all user accounts.
> >
> > Tested.
> >
> > Simo.
> 
> ACK but you need this change in order for the unit tests to pass:
> 
> diff --git a/ipatests/test_xmlrpc/test_user_plugin.py 
> b/ipatests/test_xmlrpc/
> test_user_plugin.py
> index 14a4b50..9b17775 100644
> --- a/ipatests/test_xmlrpc/test_user_plugin.py
> +++ b/ipatests/test_xmlrpc/test_user_plugin.py
> @@ -99,10 +99,6 @@ def get_user_result(uid, givenname, sn, operation='sh
> ow', omit=[],
>               mepmanagedentry=[get_group_dn(uid)],
>               objectclass=add_oc(objectclasses.user, u'ipantuserattrs'),
>               krbprincipalname=[u'%s@%s' % (uid, api.env.realm)],
> -            krbpwdpolicyreference=[DN(('cn', 'global_policy'),
> -                                      ('cn', api.env.realm),
> -                                      ('cn', 'kerberos'),
> -                                      api.env.basedn)],
>           )
>       if operation in ('show', 'show-all', 'find', 'mod'):
>           result.update(
> 

Attached  both patches again, the first is the same as before, the
second is updated with the above snippet.

Thanks for the review.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
>From 541efc9a1b51ed976a8320da3aff979832b80091 Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Tue, 14 Jan 2014 10:18:43 -0500
Subject: [PATCH] Stop adding a default password policy reference

Both the password plugin and the kdb driver code automatically fall
back to the default password policy.
so stop adding an explicit reference to user objects and instead rely on the
fallback.
This way users created via the framework and users created via winsync plugin
behave the same way wrt password policies and no surprises will happen.

Also in case we need to change the default password policy DN this will allow
just code changes instead of having to change each user entry created, and
distinguish between the default policy and explicit admin changes.

Related: https://fedorahosted.org/freeipa/ticket/4085
---
 ipalib/plugins/user.py                   | 3 ---
 ipatests/test_xmlrpc/test_user_plugin.py | 4 ----
 2 files changed, 7 deletions(-)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 3c8353ffaac8cde6164ad03afde2fc85b99524f2..6cdaae334e5cba0899e1e7f5dc6cd1cc18ef5930 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -529,9 +529,6 @@ class user_add(LDAPCreate):
             homes_root = config.get('ipahomesrootdir', ['/home'])[0]
             # build user's home directory based on his uid
             entry_attrs['homedirectory'] = posixpath.join(homes_root, keys[-1])
-        entry_attrs.setdefault('krbpwdpolicyreference',
-                               DN(('cn', 'global_policy'), ('cn', api.env.realm), ('cn', 'kerberos'),
-                                  api.env.basedn))
         entry_attrs.setdefault('krbprincipalname', '%s@%s' % (entry_attrs['uid'], api.env.realm))
 
         if entry_attrs.get('gidnumber') is None:
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 14a4b501d49537eb1ddcc6c593ad4b75c15da6da..9b17775899ed79bbdfedc21c5e8acb74c2cc8356 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -99,10 +99,6 @@ def get_user_result(uid, givenname, sn, operation='show', omit=[],
             mepmanagedentry=[get_group_dn(uid)],
             objectclass=add_oc(objectclasses.user, u'ipantuserattrs'),
             krbprincipalname=[u'%s@%s' % (uid, api.env.realm)],
-            krbpwdpolicyreference=[DN(('cn', 'global_policy'),
-                                      ('cn', api.env.realm),
-                                      ('cn', 'kerberos'),
-                                      api.env.basedn)],
         )
     if operation in ('show', 'show-all', 'find', 'mod'):
         result.update(
-- 
1.8.4.2

>From f53bc36f1631c7835746ff96d62c1011944f2f00 Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Tue, 14 Jan 2014 10:09:37 -0500
Subject: [PATCH 1/2] Harmonize policy discovery to kdb driver

The KDB driver does not walk the tree back like the original password plugin.
Also we do not store the default policy in the base DN as we used to do in the
past anymore.
So doing a full subtree search and walking back the tree is just a waste of
time.
Instead hardcode the default policy like we do in the kdb driver.

Fixes: https://fedorahosted.org/freeipa/ticket/4085
---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c | 106 ++++-------------------
 1 file changed, 17 insertions(+), 89 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index 2538a4094bd9a166e61b0911e5ea93426092d88a..ef20c4c61bd764bffc426208ff8b99f5d0b782ec 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -436,75 +436,44 @@ static void pwd_values_free(Slapi_ValueSet** results,
     slapi_vattr_values_free(results, actual_type_name, buffer_flags);
 }
 
-static int ipapwd_rdn_count(const char *dn)
-{
-    int rdnc = 0;
-    LDAPDN ldn;
-    int ret;
-
-    ret = ldap_str2dn(dn, &ldn, LDAP_DN_FORMAT_LDAPV3);
-    if (ret != LDAP_SUCCESS) {
-        LOG_TRACE("ldap_str2dn(dn) failed ?!");
-        return -1;
-    }
-
-    for (rdnc = 0; ldn != NULL && ldn[rdnc]; rdnc++) /* count */ ;
-    ldap_dnfree(ldn);
-
-    return rdnc;
-}
-
 int ipapwd_getPolicy(const char *dn,
                      Slapi_Entry *target,
                      struct ipapwd_policy *policy)
 {
     const char *krbPwdPolicyReference;
-    const char *pdn;
-    const Slapi_DN *psdn;
-    Slapi_Backend *be;
+    char *pdn = NULL;
     Slapi_PBlock *pb = NULL;
     char *attrs[] = { "krbMaxPwdLife", "krbMinPwdLife",
                       "krbPwdMinDiffChars", "krbPwdMinLength",
                       "krbPwdHistoryLength", NULL};
     Slapi_Entry **es = NULL;
     Slapi_Entry *pe = NULL;
-    int ret, res, dist, rdnc, scope, i;
-    Slapi_DN *sdn = NULL;
+    int ret, res, scope, i;
     int buffer_flags=0;
     Slapi_ValueSet* results = NULL;
-    char* actual_type_name = NULL;
+    char *actual_type_name = NULL;
     int tmpint;
 
     LOG_TRACE("Searching policy for [%s]\n", dn);
 
-    sdn = slapi_sdn_new_dn_byref(dn);
-    if (sdn == NULL) {
-        LOG_OOM();
-        ret = -1;
-        goto done;
-    }
-
     pwd_get_values(target, "krbPwdPolicyReference",
                    &results, &actual_type_name, &buffer_flags);
     if (results) {
         Slapi_Value *sv;
         slapi_valueset_first_value(results, &sv);
         krbPwdPolicyReference = slapi_value_get_string(sv);
-        pdn = krbPwdPolicyReference;
-        scope = LDAP_SCOPE_BASE;
-        LOG_TRACE("using policy reference: %s\n", pdn);
+        pdn = slapi_ch_strdup(krbPwdPolicyReference);
     } else {
-        /* Find ancestor base DN */
-        be = slapi_be_select(sdn);
-        psdn = slapi_be_getsuffix(be, 0);
-        if (psdn == NULL) {
-            LOG_FATAL("Invalid DN [%s]\n", dn);
-            ret = -1;
-            goto done;
-        }
-        pdn = slapi_sdn_get_dn(psdn);
-        scope = LDAP_SCOPE_SUBTREE;
+        /* Fallback to hardcoded value */
+        pdn = slapi_ch_smprintf("cn=global_policy,%s", ipa_realm_dn);
     }
+    if (pdn == NULL) {
+        LOG_OOM();
+        ret = -1;
+        goto done;
+    }
+    LOG_TRACE("Using policy at [%s]\n", pdn);
+    scope = LDAP_SCOPE_BASE;
 
     pb = slapi_pblock_new();
     slapi_search_internal_set_pb(pb,
@@ -539,54 +508,13 @@ int ipapwd_getPolicy(const char *dn,
     /* if there is only one, return that */
     if (i == 1) {
         pe = es[0];
-        goto fill;
-    }
-
-    /* count number of RDNs in DN */
-    rdnc = ipapwd_rdn_count(dn);
-    if (rdnc == -1) {
-        LOG_TRACE("ipapwd_rdn_count(dn) failed");
-        ret = -1;
-        goto done;
-    }
-
-    pe = NULL;
-    dist = -1;
-
-    /* find closest entry */
-    for (i = 0; es[i]; i++) {
-        const Slapi_DN *esdn;
-
-        esdn = slapi_entry_get_sdn_const(es[i]);
-        if (esdn == NULL) continue;
-        if (0 == slapi_sdn_compare(esdn, sdn)) {
-            pe = es[i];
-            dist = 0;
-            break;
-        }
-        if (slapi_sdn_issuffix(sdn, esdn)) {
-            const char *dn1;
-            int c1;
-
-            dn1 = slapi_sdn_get_dn(esdn);
-            if (!dn1) continue;
-            c1 = ipapwd_rdn_count(dn1);
-            if (c1 == -1) continue;
-            if ((dist == -1) ||
-                ((rdnc - c1) < dist)) {
-                dist = rdnc - c1;
-                pe = es[i];
-            }
-        }
-        if (dist == 0) break; /* found closest */
-    }
-
-    if (pe == NULL) {
+    } else {
+        LOG_TRACE("Multiple entries from a base search ?!");
         ret = -1;
         goto done;
     }
 
-fill:
+    /* read data out of policy object */
     policy->min_pwd_life = slapi_entry_attr_get_int(pe, "krbMinPwdLife");
 
     tmpint = slapi_entry_attr_get_int(pe, "krbMaxPwdLife");
@@ -615,7 +543,7 @@ done:
         slapi_free_search_results_internal(pb);
         slapi_pblock_destroy(pb);
     }
-    if (sdn) slapi_sdn_free(&sdn);
+    slapi_ch_free_string(&pdn);
     return ret;
 }
 
-- 
1.8.4.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to