[SSSD] SYSDB: User and group renames

Jakub Hrozek jhrozek at redhat.com
Fri May 11 15:45:24 UTC 2012


The attached patch fixes a regression in the local domain tools where
sss_groupadd no longer detected a GID duplicate. The check for EEXIST is
moved one level up into more high level function.

The patch also adds the same rename support for users. I found it odd
that we allowed a rename of groups but not users. There is a catch when
storing a user -- his cached password would be gone. I think that
renaming a user is such a rare operation that it's not severe, plus
there is a warning in the logs.

Unit test was amended, too.
-------------- next part --------------
>From d80d17f7812741f87689dd20ac6a6e98a8776747 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 11 May 2012 16:27:46 +0200
Subject: [PATCH] SYSDB: Handle user and group renames better

Fixes a regression in the local domain tools where sss_groupadd no longer
detected a GID duplicate. The check for EEXIST is moved one level up into
more high level function.

The patch also adds the same rename support for users. I found it odd that
we allowed a rename of groups but not users. There is a catch when storing
a user -- his cached password would be gone. I think that renaming a user
is such a rare operation that it's not severe, plus there is a warning in
the logs.
---
 src/db/sysdb_ops.c      |   46 +++++++++++++--
 src/tests/sysdb-tests.c |  143 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+), 7 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index bedd8cfa292cfdab2ba20260ff641730a41610b5..5b7dba3db960095036823863f5e326d355047172 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1165,13 +1165,8 @@ int sysdb_add_group(struct sysdb_ctx *sysdb,
         ret = sysdb_search_group_by_gid(tmp_ctx, sysdb,
                                         gid, NULL, &msg);
         if (ret != ENOENT) {
-            if (ret == EOK) {
-                ret = sysdb_delete_group(sysdb, NULL, gid);
-            }
-
-            if (ret != EOK) {
-                goto done;
-            }
+            if (ret == EOK) ret = EEXIST;
+            goto done;
         }
     }
 
@@ -1513,6 +1508,25 @@ int sysdb_store_user(struct sysdb_ctx *sysdb,
         /* users doesn't exist, turn into adding a user */
         ret = sysdb_add_user(sysdb, name, uid, gid,
                              gecos, homedir, shell, attrs, cache_timeout, now);
+        if (ret == EEXIST) {
+            /* This may be a user rename. If there is a user with the
+             * same UID, remove it and try to add the basic user again
+             */
+            ret = sysdb_delete_user(sysdb, NULL, uid);
+            if (ret == ENOENT) {
+                /* Not found by UID, return the original EEXIST,
+                 * this may be a conflict in MPG domain or something
+                 * else */
+                return EEXIST;
+            } else if (ret != EOK) {
+                goto done;
+            }
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("A user with the same UID [%llu] was removed from the "
+                   "cache\n", uid));
+            ret = sysdb_add_user(sysdb, name, uid, gid, gecos,
+                                 homedir, shell, attrs, cache_timeout, now);
+        }
         goto done;
     }
 
@@ -1641,6 +1655,24 @@ int sysdb_store_group(struct sysdb_ctx *sysdb,
     if (new_group) {
         /* group doesn't exist, turn into adding a group */
         ret = sysdb_add_group(sysdb, name, gid, attrs, cache_timeout, now);
+        if (ret == EEXIST) {
+            /* This may be a group rename. If there is a group with the
+             * same GID, remove it and try to add the basic group again
+             */
+            ret = sysdb_delete_group(sysdb, NULL, gid);
+            if (ret == ENOENT) {
+                /* Not found by GID, return the original EEXIST,
+                 * this may be a conflict in MPG domain or something
+                 * else */
+                return EEXIST;
+            } else if (ret != EOK) {
+                goto done;
+            }
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("A group with the same GID [%llu] was removed from the "
+                   "cache\n", gid));
+            ret = sysdb_add_group(sysdb, name, gid, attrs, cache_timeout, now);
+        }
         goto done;
     }
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index bcf3f732be6f172324b0d1b455c28076a2355128..765a7b9352fd8d71834ff3748076b45899d4553d 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -2168,6 +2168,145 @@ START_TEST (test_sysdb_attrs_to_list)
 }
 END_TEST
 
+START_TEST(test_group_rename)
+{
+    struct sysdb_test_ctx *test_ctx;
+    errno_t ret;
+    gid_t gid;
+    const gid_t grgid = 38001;
+    const char *name;
+    const char *fromname = "fromgroup";
+    const char *toname = "togroup";
+    struct ldb_result *res;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_unless(ret == EOK, "Could not set up the test");
+
+    /* Store and verify the first group */
+    ret = sysdb_store_group(test_ctx->sysdb, fromname, grgid, NULL, 0, 0);
+    fail_unless(ret == EOK, "Could not add first group");
+
+    ret = sysdb_getgrnam(test_ctx, test_ctx->sysdb, fromname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the group from cache\n");
+    if (res->count != 1) {
+        fail("Invalid number of replies. Expected 1, got %d", res->count);
+        goto done;
+    }
+
+    gid = ldb_msg_find_attr_as_uint(res->msgs[0], SYSDB_GIDNUM, 0);
+    fail_unless(gid == grgid,
+                "Did not find the expected GID (found %llu expected %llu)",
+                gid, grgid);
+    name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+    fail_unless(strcmp(fromname, name) == 0,
+                "Did not find the expected name (found %s expected %s)",
+                name, fromname);
+
+    /* Perform rename and check that GID is the same, but name changed */
+    ret = sysdb_add_group(test_ctx->sysdb, toname, grgid, NULL, 0, 0);
+    fail_unless(ret == EEXIST, "Group renamed with a low level call?");
+
+    ret = sysdb_store_group(test_ctx->sysdb, toname, grgid, NULL, 0, 0);
+    fail_unless(ret == EOK, "Could not add first group");
+
+    ret = sysdb_getgrnam(test_ctx, test_ctx->sysdb, toname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the group from cache\n");
+    if (res->count != 1) {
+        fail("Invalid number of replies. Expected 1, got %d", res->count);
+        goto done;
+    }
+
+    gid = ldb_msg_find_attr_as_uint(res->msgs[0], SYSDB_GIDNUM, 0);
+    fail_unless(gid == grgid,
+                "Did not find the expected GID (found %llu expected %llu)",
+                gid, grgid);
+    name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+    fail_unless(strcmp(toname, name) == 0,
+                "Did not find the expected GID (found %s expected %s)",
+                name, toname);
+
+    /* Verify the first name is gone */
+    ret = sysdb_getgrnam(test_ctx, test_ctx->sysdb, fromname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the group from cache\n");
+    fail_unless(res->count == 0, "Unexpectedly found the original user\n");
+
+done:
+    talloc_free(test_ctx);
+}
+END_TEST
+
+START_TEST(test_user_rename)
+{
+    struct sysdb_test_ctx *test_ctx;
+    errno_t ret;
+    uid_t uid;
+    const uid_t userid = 38002;
+    const char *name;
+    const char *fromname = "fromuser";
+    const char *toname = "touser";
+    struct ldb_result *res;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_unless(ret == EOK, "Could not set up the test");
+
+    /* Store and verify the first user */
+    ret = sysdb_store_user(test_ctx->sysdb, fromname, NULL, userid, 0,
+                           fromname, "/", "/bin/sh", NULL, NULL, 0, 0);
+    fail_unless(ret == EOK, "Could not add first user");
+
+    ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, fromname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the user from cache\n");
+    if (res->count != 1) {
+        fail("Invalid number of replies. Expected 1, got %d", res->count);
+        goto done;
+    }
+
+    uid = ldb_msg_find_attr_as_uint(res->msgs[0], SYSDB_UIDNUM, 0);
+    fail_unless(uid == userid,
+                "Did not find the expected UID (found %llu expected %llu)",
+                uid, userid);
+    name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+    fail_unless(strcmp(fromname, name) == 0,
+                "Did not find the expected name (found %s expected %s)",
+                name, fromname);
+
+    /* Perform rename and check that GID is the same, but name changed */
+    ret = sysdb_add_user(test_ctx->sysdb, toname, userid, 0,
+                         fromname, "/", "/bin/sh", NULL, 0, 0);
+    fail_unless(ret == EEXIST, "A second user added with low level call?");
+
+    ret = sysdb_store_user(test_ctx->sysdb, toname, NULL, userid, 0,
+                           fromname, "/", "/bin/sh", NULL, NULL, 0, 0);
+    fail_unless(ret == EOK, "Could not add second user");
+
+    ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, toname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the user from cache\n");
+    if (res->count != 1) {
+        fail("Invalid number of replies. Expected 1, got %d", res->count);
+        goto done;
+    }
+
+    uid = ldb_msg_find_attr_as_uint(res->msgs[0], SYSDB_UIDNUM, 0);
+    fail_unless(uid == userid,
+                "Did not find the expected UID (found %llu expected %llu)",
+                uid, userid);
+    name = ldb_msg_find_attr_as_string(res->msgs[0], SYSDB_NAME, NULL);
+    fail_unless(strcmp(toname, name) == 0,
+                "Did not find the expected name (found %s expected %s)",
+                name, fromname);
+
+    /* Verify the first name is gone */
+    ret = sysdb_getpwnam(test_ctx, test_ctx->sysdb, fromname, &res);
+    fail_unless(ret == EOK, "Could not retrieve the user from cache\n");
+    fail_unless(res->count == 0, "Unexpectedly found the original user\n");
+
+done:
+    talloc_free(test_ctx);
+}
+END_TEST
+
 START_TEST (test_sysdb_update_members)
 {
     struct sysdb_test_ctx *test_ctx;
@@ -3601,6 +3740,10 @@ Suite *create_sysdb_suite(void)
     /* Test originalDN searches */
     tcase_add_test(tc_sysdb, test_sysdb_original_dn_case_insensitive);
 
+    /* Test user and group renames */
+    tcase_add_test(tc_sysdb, test_group_rename);
+    tcase_add_test(tc_sysdb, test_user_rename);
+
 /* ===== NETGROUP TESTS ===== */
 
     /* Create a new netgroup */
-- 
1.7.7.6



More information about the sssd-devel mailing list