* NEWS, src/userdel.c: Do not remove a group with the same name as

	the user (usergroup) if this group isn't the user's primary group.

diff --git a/ChangeLog b/ChangeLog
index 55362860e3058aeb6791047726e7432085e44a3c..d6ad13c46deef7c29cf4d06b4c163e9c19b9b51d 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-06-05  Nicolas François  <nicolas.francois@centraliens.net>
+
+       * NEWS, src/userdel.c: Do not remove a group with the same name as
+       the user (usergroup) if this group isn't the user's primary group.
+
 2011-06-04  Nicolas François  <nicolas.francois@centraliens.net>
 
        * NEWS, src/userdel.c: Check the existence of the user's mail

diff --git a/NEWS b/NEWS
index 0dd821073d2ebd68348f4691f82f0cab1bd7e69b..3b731e96af033d959d6e24b05664079808335ef6 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -47,6 +47,8 @@ shadow-4.1.4.3 -> shadow-4.1.5                                        UNRELEASED
 - userdel
   * Check the existence of the user's mail spool before trying to remove
     it. If it does not exist, a warning is issued, but no failure.
+  * Do not remove a group with the same name as the user (usergroup) if
+    this group isn't the user's primary group.
 - usermod
   * Accept options in any order (username not necessarily at the end)
 

diff --git a/src/userdel.c b/src/userdel.c
index 7c9463ca551a6aea1a5f321b4ae4baba6bfd023e..05c0a747768e317e454658dd505f7a6cc339bdd5 100644 (file)
--- a/src/userdel.c
+++ b/src/userdel.c
@@ -82,6 +82,7 @@ const char *Prog;
 
 static char *user_name;
 static uid_t user_id;
+static gid_t user_gid;
 static char *user_home;
 
 static bool fflg = false;
@@ -100,6 +101,7 @@ static bool spw_locked  = false;
 /* local function prototypes */
 static void usage (int status);
 static void update_groups (void);
+static void remove_usergroup (void);
 static void close_files (void);
 static void fail_exit (int);
 static void open_files (void);
@@ -145,10 +147,8 @@ static void update_groups (void)
 {
        const struct group *grp;
        struct group *ngrp;
-       struct passwd *pwd;
 
 #ifdef SHADOWGRP
-       bool deleted_user_group = false;
        const struct sgrp *sgrp;
        struct sgrp *nsgrp;
 #endif                         /* SHADOWGRP */
@@ -199,69 +199,10 @@ static void update_groups (void)
                         user_name, ngrp->gr_name));
        }
 
-       /*
-        * we've removed their name from all the groups above, so
-        * now if they have a group with the same name as their
-        * user name, with no members, we delete it.
-        * FIXME: below, the check for grp->gr_mem[0] is not sufficient.
-        *        We should retrieve the group with gr_locate and check
-        *        that gr_mem is empty.
-        */
-       grp = xgetgrnam (user_name);
-       if (   (NULL != grp)
-           && getdef_bool ("USERGROUPS_ENAB")
-           && (   (NULL == grp->gr_mem[0])
-               || (   (NULL == grp->gr_mem[1])
-                   && (strcmp (grp->gr_mem[0], user_name) == 0)))) {
-
-               pwd = NULL;
-               if (!fflg) {
-                       /*
-                        * Scan the passwd file to check if this group is still
-                        * used as a primary group.
-                        */
-                       setpwent ();
-                       while ((pwd = getpwent ()) != NULL) {
-                               if (strcmp (pwd->pw_name, user_name) == 0) {
-                                       continue;
-                               }
-                               if (pwd->pw_gid == grp->gr_gid) {
-                                       fprintf (stderr,
-                                                _("%s: group %s is the primary group of another user and is not removed.\n"),
-                                                Prog, grp->gr_name);
-                                       break;
-                               }
-                       }
-                       endpwent ();
-               }
-
-               if (NULL == pwd) {
-                       /*
-                        * We can remove this group, it is not the primary
-                        * group of any remaining user.
-                        */
-                       if (gr_remove (grp->gr_name) == 0) {
-                               fprintf (stderr,
-                                        _("%s: cannot remove entry '%s' from %s\n"),
-                                        Prog, grp->gr_name, gr_dbname ());
-                               fail_exit (E_GRP_UPDATE);
-                       }
-
-#ifdef SHADOWGRP
-                       deleted_user_group = true;
-#endif                         /* SHADOWGRP */
-
-#ifdef WITH_AUDIT
-                       audit_logger (AUDIT_DEL_GROUP, Prog,
-                                     "deleting group",
-                                     grp->gr_name, AUDIT_NO_ID,
-                                     SHADOW_AUDIT_SUCCESS);
-#endif                         /* WITH_AUDIT */
-                       SYSLOG ((LOG_INFO,
-                                "removed group '%s' owned by '%s'\n",
-                                grp->gr_name, user_name));
-               }
+       if (getdef_bool ("USERGROUPS_ENAB")) {
+               remove_usergroup ();
        }
+
 #ifdef SHADOWGRP
        if (!is_shadow_grp) {
                return;
@@ -317,19 +258,109 @@ static void update_groups (void)
                              SHADOW_AUDIT_SUCCESS);
 #endif                         /* WITH_AUDIT */
                SYSLOG ((LOG_INFO, "delete '%s' from shadow group '%s'\n",
-                        user_name, nsgrp->sg_name));
+                        user_name, nsgrp->sg_name));
        }
+#endif                         /* SHADOWGRP */
+}
 
-       if (   deleted_user_group
-           && (sgr_locate (user_name) != NULL)) {
-               if (sgr_remove (user_name) == 0) {
+/*
+ * remove_usergroup - delete the user's group if it is a usergroup
+ *
+ *     An usergroup is removed if
+ *       + it has the same name as the user
+ *       + it is the primary group of the user
+ *       + it has no other members
+ *       + it is not the primary group of any other user
+ */
+static void remove_usergroup (void)
+{
+       const struct group *grp;
+       const struct passwd *pwd = NULL;
+
+       grp = gr_locate (user_name);
+       if (NULL == grp) {
+               /* This user has no usergroup. */
+               return;
+       }
+
+       if (grp->gr_gid != user_gid) {
+               fprintf (stderr,
+                        _("%s: group %s not removed because it is not the primary group of user %s.\n"),
+                        Prog, grp->gr_name, user_name);
+               return;
+       }
+
+       if (NULL != grp->gr_mem[0]) {
+               /* The usergroup has other members. */
+               fprintf (stderr,
+                        _("%s: group %s not removed because it has other members.\n"),
+                        Prog, grp->gr_name);
+               return;
+       }
+
+       if (!fflg) {
+               /*
+                * Scan the passwd file to check if this group is still
+                * used as a primary group.
+                */
+               setpwent ();
+               while ((pwd = getpwent ()) != NULL) {
+                       if (strcmp (pwd->pw_name, user_name) == 0) {
+                               continue;
+                       }
+                       if (pwd->pw_gid == grp->gr_gid) {
+                               fprintf (stderr,
+                                        _("%s: group %s is the primary group of another user and is not removed.\n"),
+                                        Prog, grp->gr_name);
+                               break;
+                       }
+               }
+               endpwent ();
+       }
+
+       if (NULL == pwd) {
+               /*
+                * We can remove this group, it is not the primary
+                * group of any remaining user.
+                */
+               if (gr_remove (grp->gr_name) == 0) {
                        fprintf (stderr,
                                 _("%s: cannot remove entry '%s' from %s\n"),
-                                Prog, user_name, sgr_dbname ());
+                                Prog, grp->gr_name, gr_dbname ());
                        fail_exit (E_GRP_UPDATE);
                }
-       }
+
+#ifdef WITH_AUDIT
+               audit_logger (AUDIT_DEL_GROUP, Prog,
+                             "deleting group",
+                             grp->gr_name, AUDIT_NO_ID,
+                             SHADOW_AUDIT_SUCCESS);
+#endif                         /* WITH_AUDIT */
+               SYSLOG ((LOG_INFO,
+                        "removed group '%s' owned by '%s'\n",
+                        grp->gr_name, user_name));
+
+#ifdef SHADOWGRP
+               if (sgr_locate (user_name) != NULL) {
+                       if (sgr_remove (user_name) == 0) {
+                               fprintf (stderr,
+                                        _("%s: cannot remove entry '%s' from %s\n"),
+                                        Prog, user_name, sgr_dbname ());
+                               fail_exit (E_GRP_UPDATE);
+                       }
+#ifdef WITH_AUDIT
+                       audit_logger (AUDIT_DEL_GROUP, Prog,
+                                     "deleting shadow group",
+                                     grp->gr_name, AUDIT_NO_ID,
+                                     SHADOW_AUDIT_SUCCESS);
+#endif                         /* WITH_AUDIT */
+                       SYSLOG ((LOG_INFO,
+                                "removed shadow group '%s' owned by '%s'\n",
+                                grp->gr_name, user_name));
+
+               }
 #endif                         /* SHADOWGRP */
+       }
 }
 
 /*
@@ -925,6 +956,7 @@ int main (int argc, char **argv)
                        exit (E_NOTFOUND);
                }
                user_id = pwd->pw_uid;
+               user_gid = pwd->pw_gid;
                user_home = xstrdup (pwd->pw_dir);
        }
 #ifdef WITH_TCB