Attachment 318214 Details for Bug 459729 – patch - fix code review issues

[patch] patch - fix code review issues

0013-Do-not-add-enabled-use-to-activated-group-clean-up.patch (text/plain), 5.20 KB, created by Rich Megginson on 2008-10-01 15:39:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Rich Megginson

Created: 2008-10-01 15:39:35 UTC

Size: 5.20 KB

patch

obsolete

>From 0d17a8094103a41c4a0ab06116979ce2f3744d74 Mon Sep 17 00:00:00 2001
>From: Rich Megginson <rmeggins@redhat.com>
>Date: Wed, 1 Oct 2008 09:38:43 -0600
>Subject: [PATCH] Do not add enabled use to activated group - clean up parse_acct_disable
>
>If a user needs to be enabled, just delete the user from the inactivated group,
>but do not add to the activated group.  If a user is in no group, the user is
>active by default.  IPA uses the activated group for override purposes.
>
>parse_acct_disable is only used when the config changes, but I cleaned it
>up anyway to make the code clearer.
>---
> .../ipa-winsync/ipa-winsync-config.c               |    9 ++---
> .../ipa-slapi-plugins/ipa-winsync/ipa-winsync.c    |   38 +++++++++++++-------
> 2 files changed, 28 insertions(+), 19 deletions(-)
>
>diff --git a/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c b/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c
>index a887e89..45efa6d 100644
>--- a/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c
>+++ b/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c
>@@ -166,14 +166,11 @@ parse_acct_disable(const char *theval)
>     }
>     if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_NONE)) {
>         retval = ACCT_DISABLE_NONE;
>-    }
>-    if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_TO_AD)) {
>+    } else if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_TO_AD)) {
>         retval = ACCT_DISABLE_TO_AD;
>-    }
>-    if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_TO_DS)) {
>+    } else if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_TO_DS)) {
>         retval = ACCT_DISABLE_TO_DS;
>-    }
>-    if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_BOTH)) {
>+    } else if (!PL_strcasecmp(theval, IPA_WINSYNC_ACCT_DISABLE_BOTH)) {
>         retval = ACCT_DISABLE_BOTH;
>     }
> 
>diff --git a/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync.c b/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync.c
>index f18a939..4a6c80d 100644
>--- a/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync.c
>+++ b/ipa-server/ipa-slapi-plugins/ipa-winsync/ipa-winsync.c
>@@ -1011,9 +1011,15 @@ sync_acct_disable(
>             char *adddn, *deldn;
>             const char *dsdn;
>             int rc;
>+            /* in the case of disabling a user, need to remove that user from
>+               the activated group, if in there, and add to the inactivated group
>+               however, in the case of enabling a user, we just have to remove
>+               the user from the inactivated group, if in there - if the user
>+               is not in any group, the user is activated by default
>+            */
>             if (ad_is_enabled) {
>                 /* add user to activated group, delete from inactivated group */
>-                adddn = ipaconfig->activated_group_dn;
>+                adddn = NULL; /* no group means active by default */
>                 deldn = ipaconfig->inactivated_group_dn;
>             } else {
>                 /* add user to inactivated group, delete from activated group */
>@@ -1046,7 +1052,11 @@ sync_acct_disable(
>             }
>             /* next, add the user to the adddn group - ignore (but log)
>                if the user is already in that group */
>-            rc = do_group_modify(adddn, "member", LDAP_MOD_ADD, dsdn);
>+            if (adddn) {
>+                rc = do_group_modify(adddn, "member", LDAP_MOD_ADD, dsdn);
>+            } else {
>+                rc = LDAP_SUCCESS;
>+            }
>             if (rc == LDAP_TYPE_OR_VALUE_EXISTS) {
>                 /* user already in that group */
>                 slapi_log_error(SLAPI_LOG_PLUGIN, ipa_winsync_plugin_name,
>@@ -1080,18 +1090,20 @@ sync_acct_disable(
>                                                   "memberOf", deldn);
>                     }
>                 }
>-                slapi_value_set_string(sv, adddn);
>-                if (!slapi_entry_attr_has_syntax_value(ds_entry,
>-                                                       "memberOf", sv)) {
>-                    if (smods) {
>-                        slapi_mods_add_string(smods, LDAP_MOD_ADD,
>-                                              "memberOf", adddn);
>-                        if (do_modify) {
>-                            *do_modify = 1; /* added mods */
>+                if (adddn) {
>+                    slapi_value_set_string(sv, adddn);
>+                    if (!slapi_entry_attr_has_syntax_value(ds_entry,
>+                                                           "memberOf", sv)) {
>+                        if (smods) {
>+                            slapi_mods_add_string(smods, LDAP_MOD_ADD,
>+                                                  "memberOf", adddn);
>+                            if (do_modify) {
>+                                *do_modify = 1; /* added mods */
>+                            }
>+                        } else if (update_entry) {
>+                            slapi_entry_add_string(update_entry,
>+                                                   "memberOf", adddn);
>                         }
>-                    } else if (update_entry) {
>-                        slapi_entry_add_string(update_entry,
>-                                               "memberOf", adddn);
>                     }
>                 }
>                 slapi_value_free(&sv);
>-- 
>1.5.5.1
>

Actions: View | Diff

Attachments on bug 459729: 314729 | 314730 | 314731 | 314732 | 314733 | 314844 | 314845 | 316460 | 317002 | 317807 | 318214 | 319412 | 319413 | 319414 | 319415 | 319416 | 319786 | 320039 | 320040