Add support for removing members from groups. Updated convenience functions for adding/removing user accounts and POSIX groups to the groups.

Also modified the add/remove member functions to be a single
interface taking a flag for add or removal, since the code only
differs by one LDB flag.

Added associated unit tests.

3 files changed, 184 insertions, 68 deletions

diff --git a/server/db/sysdb.c b/server/db/sysdb.c
index edf3593f..31233e58 100644
--- a/server/db/sysdb.c
+++ b/server/db/sysdb.c
@@ -1264,11 +1264,12 @@ done:
 }
 
 /* Wrapper around adding a user account to a POSIX group */
-int sysdb_add_acct_to_posix_group(TALLOC_CTX *mem_ctx,
-                                  struct sysdb_ctx *sysdb,
-                                  const char *domain,
-                                  const char *group,
-                                  const char *username)
+int sysdb_add_remove_posix_group_acct(TALLOC_CTX *mem_ctx,
+                                     struct sysdb_ctx *sysdb,
+                                     int flag,
+                                     const char *domain,
+                                     const char *group,
+                                     const char *username)
 {
     TALLOC_CTX *tmp_ctx;
     int ret;
@@ -1289,17 +1290,26 @@ int sysdb_add_acct_to_posix_group(TALLOC_CTX *mem_ctx,
     account = talloc_asprintf(tmp_ctx,
                               SYSDB_PW_NAME"=%s,"SYSDB_TMPL_USER_BASE,
                               username, domain);
-    if (account == NULL) goto done;
+    if (account == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
 
     acct_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, account);
-    if (acct_dn == NULL) goto done;
+    if (acct_dn == NULL) {
+        ret = errno;
+        goto done;
+    }
 
     group_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
                               SYSDB_GR_NAME"=%s,"SYSDB_TMPL_GROUP_BASE,
                               group, domain);
-    if (group_dn == NULL) goto done;
+    if (group_dn == NULL) {
+        ret = errno;
+        goto done;
+    }
 
-    ret = sysdb_add_member_to_posix_group(tmp_ctx, sysdb, acct_dn, group_dn);
+    ret = sysdb_add_remove_posix_group_member(tmp_ctx, sysdb, flag, acct_dn, group_dn);
 
 done:
     talloc_free(tmp_ctx);
@@ -1307,11 +1317,12 @@ done:
 }
 
 /* Wrapper around adding a POSIX group to a POSIX group */
-int sysdb_add_group_to_posix_group(TALLOC_CTX *mem_ctx,
-                                  struct sysdb_ctx *sysdb,
-                                  const char *domain,
-                                  const char *group,
-                                  const char *member_group)
+int sysdb_add_remove_posix_group_group(TALLOC_CTX *mem_ctx,
+                                      struct sysdb_ctx *sysdb,
+                                      int flag,
+                                      const char *domain,
+                                      const char *group,
+                                      const char *member_group)
 {
     TALLOC_CTX *tmp_ctx;
     int ret;
@@ -1332,36 +1343,58 @@ int sysdb_add_group_to_posix_group(TALLOC_CTX *mem_ctx,
     member_group_canonical = talloc_asprintf(tmp_ctx,
                                              SYSDB_GR_NAME"=%s,"SYSDB_TMPL_GROUP_BASE,
                                              member_group, domain);
-    if (member_group_canonical == NULL) goto done;
+    if (member_group_canonical == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
 
     member_group_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, member_group_canonical);
-    if (member_group_dn == NULL) goto done;
+    if (member_group_dn == NULL) {
+        ret = errno;
+        goto done;
+    }
 
     group_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
                               SYSDB_GR_NAME"=%s,"SYSDB_TMPL_GROUP_BASE,
                               group, domain);
-    if (group_dn == NULL) goto done;
+    if (group_dn == NULL) {
+        ret = errno;
+        goto done;
+    }
 
-    ret = sysdb_add_member_to_posix_group(tmp_ctx, sysdb, member_group_dn, group_dn);
+    ret = sysdb_add_remove_posix_group_member(tmp_ctx, sysdb, flag, member_group_dn, group_dn);
 
 done:
     talloc_free(tmp_ctx);
     return ret;
 }
 
-int sysdb_add_member_to_posix_group(TALLOC_CTX *mem_ctx,
-                                    struct sysdb_ctx *sysdb,
-                                    struct ldb_dn *member_dn,
-                                    struct ldb_dn *group_dn)
+int sysdb_add_remove_posix_group_member(TALLOC_CTX *mem_ctx,
+                                        struct sysdb_ctx *sysdb,
+                                        int flag,
+                                        struct ldb_dn *member_dn,
+                                        struct ldb_dn *group_dn)
 {
     TALLOC_CTX *tmp_ctx;
-    int ret, lret;
+    int ret, lret, ldb_flag;
     struct ldb_message *msg;
     struct ldb_request *req;
 
     tmp_ctx = talloc_new(mem_ctx);
     if (!tmp_ctx) return ENOMEM;
 
+    switch (flag) {
+    case SYSDB_FLAG_MOD_ADD:
+        ldb_flag = LDB_FLAG_MOD_ADD;
+        break;
+    case SYSDB_FLAG_MOD_DELETE:
+        ldb_flag = LDB_FLAG_MOD_DELETE;
+        break;
+    default:
+        DEBUG(0, ("Group modification requested with invalid flag\n"));
+        return EINVAL;
+    }
+
     /* Start LDB Transaction */
     lret = ldb_transaction_start(sysdb->ldb);
     if (lret != LDB_SUCCESS) {
@@ -1370,14 +1403,14 @@ int sysdb_add_member_to_posix_group(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
-    /* Add the user as a member of the group */
+    /* Add or remove the member_dn as a member of the group */
     msg = ldb_msg_new(tmp_ctx);
     if(msg == NULL) {
         ret = ENOMEM;
         goto done;
     }
     msg->dn = group_dn;
-    lret = ldb_msg_add_empty(msg, SYSDB_GR_MEMBER, LDB_FLAG_MOD_ADD, NULL);
+    lret = ldb_msg_add_empty(msg, SYSDB_GR_MEMBER, ldb_flag, NULL);
     if (lret == LDB_SUCCESS) {
         lret = ldb_msg_add_fmt(msg, SYSDB_GR_MEMBER, "%s", ldb_dn_alloc_linearized(tmp_ctx, member_dn));
     }
@@ -1410,9 +1443,11 @@ int sysdb_add_member_to_posix_group(TALLOC_CTX *mem_ctx,
     }
 
     ret = EOK;
+
 done:
     /* Cancel LDB Transaction */
     if (ret != EOK) {
+        DEBUG(1, ("Cancelling ldb transaction (%d)\n", ret));
         lret = ldb_transaction_cancel(sysdb->ldb);
         if (lret != LDB_SUCCESS) {
             DEBUG(1, ("Failed to cancel ldb transaction (%d)\n", lret));
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index 014e2ce9..656cb8fe 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -82,6 +82,13 @@ struct confdb_ctx;
 
 typedef void (*sysdb_callback_t)(void *, int, struct ldb_result *);
 
+enum sysdb_flags {
+    SYSDB_FLAG_MOD_NONE = 0,
+    SYSDB_FLAG_MOD_ADD,
+    SYSDB_FLAG_MOD_DELETE,
+    SYSDB_FLAG_MOD_MODIFY
+};
+
 int sysdb_init(TALLOC_CTX *mem_ctx,
                struct event_context *ev,
                struct confdb_ctx *cdb,
@@ -155,20 +162,24 @@ int sysdb_store_group_posix(TALLOC_CTX *memctx,
                             const char *domain,
                             const char *name, gid_t gid);
 
-int sysdb_add_acct_to_posix_group(TALLOC_CTX *mem_ctx,
-                                  struct sysdb_ctx *sysdb,
-                                  const char *domain,
-                                  const char *gname,
-                                  const char *username);
-
-int sysdb_add_group_to_posix_group(TALLOC_CTX *mem_ctx,
-                                  struct sysdb_ctx *sysdb,
-                                  const char *domain,
-                                  const char *group,
-                                  const char *member_group);
-
-int sysdb_add_member_to_posix_group(TALLOC_CTX *mem_ctx,
-                                    struct sysdb_ctx *sysdb,
-                                    struct ldb_dn *member_dn,
-                                    struct ldb_dn *group_dn);
+int sysdb_add_remove_posix_group_acct(TALLOC_CTX *mem_ctx,
+                                     struct sysdb_ctx *sysdb,
+                                     int flag,
+                                     const char *domain,
+                                     const char *group,
+                                     const char *username);
+
+/* Wrapper around adding a POSIX group to a POSIX group */
+int sysdb_add_remove_posix_group_group(TALLOC_CTX *mem_ctx,
+                                      struct sysdb_ctx *sysdb,
+                                      int flag,
+                                      const char *domain,
+                                      const char *group,
+                                      const char *member_group);
+
+int sysdb_add_remove_posix_group_member(TALLOC_CTX *mem_ctx,
+                                        struct sysdb_ctx *sysdb,
+                                        int flag,
+                                        struct ldb_dn *member_dn,
+                                        struct ldb_dn *group_dn);
 #endif /* __SYS_DB_H__ */
diff --git a/server/tests/sysdb-tests.c b/server/tests/sysdb-tests.c
index d1cddec6..9d8ebffa 100644
--- a/server/tests/sysdb-tests.c
+++ b/server/tests/sysdb-tests.c
@@ -228,11 +228,12 @@ START_TEST (test_sysdb_add_acct_to_posix_group)
     username = talloc_asprintf(test_ctx, "testuser%d", _i);
     group = talloc_asprintf(test_ctx, "%s%d",SYSDB_POSIX_TEST_GROUP, _i);
 
-    ret = sysdb_add_acct_to_posix_group(test_ctx,
-                                        test_ctx->sysdb,
-                                        "LOCAL",
-                                        group,
-                                        username);
+    ret = sysdb_add_remove_posix_group_acct(test_ctx,
+                                            test_ctx->sysdb,
+                                            SYSDB_FLAG_MOD_ADD,
+                                            "LOCAL",
+                                            group,
+                                            username);
     fail_if(ret != EOK,
             "Failed to add user %s to group %s.",
             username, group, ret);
@@ -385,13 +386,20 @@ START_TEST (test_sysdb_add_invalid_member)
         return;
     }
 
+    group_name = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i);
+    group = talloc_asprintf(test_ctx,
+                            SYSDB_GR_NAME"=%s,"SYSDB_TMPL_GROUP_BASE,
+                            group_name, "LOCAL");
+    fail_if(group == NULL, "Could not allocate group dn");
+
     /* Add nonexistent user to test group */
     username = talloc_asprintf(test_ctx, "nonexistentuser%d", _i);
-    ret = sysdb_add_acct_to_posix_group(test_ctx,
-                                        test_ctx->sysdb,
-                                        "LOCAL",
-                                        SYSDB_POSIX_TEST_GROUP,
-                                        username);
+    ret = sysdb_add_remove_posix_group_acct(test_ctx,
+                                            test_ctx->sysdb,
+                                            SYSDB_FLAG_MOD_ADD,
+                                            "LOCAL",
+                                            group,
+                                            username);
     fail_if(ret == EOK,
             "Unexpected success adding user %s to group %s. Error was: %d",
             username, SYSDB_POSIX_TEST_GROUP, ret);
@@ -403,12 +411,6 @@ START_TEST (test_sysdb_add_invalid_member)
                              username, "LOCAL");
     fail_if(member == NULL, "Could not allocate member dn");
 
-    group_name = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i);
-    group = talloc_asprintf(test_ctx,
-                            SYSDB_GR_NAME"=%s,"SYSDB_TMPL_GROUP_BASE,
-                            group_name, "LOCAL");
-    fail_if(group == NULL, "Could not allocate group dn");
-
     group_dn = ldb_dn_new_fmt(test_ctx, test_ctx->sysdb->ldb, group);
     fail_if(group_dn == NULL, "Could not create group_dn object");
 
@@ -470,11 +472,12 @@ START_TEST (test_sysdb_add_group_to_posix_group)
     /* Add user to test group */
     member_group = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i-1);
     group = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i);
-    ret = sysdb_add_group_to_posix_group(test_ctx,
-                                         test_ctx->sysdb,
-                                         "LOCAL",
-                                         group,
-                                         member_group);
+    ret = sysdb_add_remove_posix_group_group(test_ctx,
+                                             test_ctx->sysdb,
+                                             SYSDB_FLAG_MOD_ADD,
+                                             "LOCAL",
+                                             group,
+                                             member_group);
     fail_if(ret != EOK,
             "Failed to add group %s to group %s. Error was: %d",
             member_group, group, ret);
@@ -483,6 +486,73 @@ START_TEST (test_sysdb_add_group_to_posix_group)
 }
 END_TEST
 
+START_TEST (test_sysdb_remove_acct_from_posix_group)
+{
+    int ret;
+    struct sysdb_test_ctx *test_ctx;
+    char *username;
+    char *group;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    if (ret != EOK) {
+        fail("Could not set up the test");
+        return;
+    }
+
+    /* Add user to test group */
+    username = talloc_asprintf(test_ctx, "testuser%d", _i);
+    group = talloc_asprintf(test_ctx, "%s%d",SYSDB_POSIX_TEST_GROUP, _i);
+
+    ret = sysdb_add_remove_posix_group_acct(test_ctx,
+                                            test_ctx->sysdb,
+                                            SYSDB_FLAG_MOD_DELETE,
+                                            "LOCAL",
+                                            group,
+                                            username);
+    fail_if(ret != EOK,
+            "Failed to remove user %s from group %s.",
+            username, group, ret);
+
+    talloc_free(test_ctx);
+}
+END_TEST
+
+START_TEST (test_sysdb_remove_group_from_posix_group)
+{
+    int ret;
+    struct sysdb_test_ctx *test_ctx;
+    char *member_group;
+    char *group;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    if (ret != EOK) {
+        fail("Could not set up the test");
+        return;
+    }
+
+    /* Add user to test group */
+    member_group = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i-1);
+    fail_if(member_group == NULL, "Could not allocate member_group");
+
+    group = talloc_asprintf(test_ctx, "%s%d", SYSDB_POSIX_TEST_GROUP, _i);
+    fail_if(member_group == NULL, "Could not allocate group");
+
+    ret = sysdb_add_remove_posix_group_group(test_ctx,
+                                             test_ctx->sysdb,
+                                             SYSDB_FLAG_MOD_DELETE,
+                                             "LOCAL",
+                                             group,
+                                             member_group);
+    fail_if(ret != EOK,
+            "Failed to remove group %s from group %s. Error was: %d",
+            member_group, group, ret);
+
+    talloc_free(test_ctx);
+}
+END_TEST
+
 Suite *create_sysdb_suite(void)
 {
     Suite *s = suite_create("sysdb");
@@ -502,12 +572,6 @@ Suite *create_sysdb_suite(void)
     /* Verify that the new group exists */
     tcase_add_loop_test(tc_posix_gr, test_sysdb_get_local_group_posix,27000,27010);
 
-    /* Change the gid of the group we created */
-    tcase_add_loop_test(tc_posix_gr, test_sysdb_store_local_group_posix,27001,27002);
-
-    /* Verify that the group has been changed */
-    tcase_add_loop_test(tc_posix_gr, test_sysdb_get_local_group_posix,27001,27002);
-
     /* Add users to the group */
     tcase_add_loop_test(tc_posix_gr, test_sysdb_add_acct_to_posix_group, 27000, 27010);
 
@@ -520,6 +584,12 @@ Suite *create_sysdb_suite(void)
     /* Add groups as members of groups */
     tcase_add_loop_test(tc_posix_gr, test_sysdb_add_group_to_posix_group, 27001, 27010);
 
+    /* Remove groups from their groups */
+    tcase_add_loop_test(tc_posix_gr, test_sysdb_remove_group_from_posix_group, 27001, 27010);
+
+    /* Remove users from their groups */
+    tcase_add_loop_test(tc_posix_gr, test_sysdb_remove_acct_from_posix_group, 27000, 27010);
+
 /* Add all test cases to the test suite */
     suite_add_tcase(s, tc_posix_users);
     suite_add_tcase(s, tc_posix_gr);