>From 3aeb7b3a5b80f7ea30a29ca5d75c1feeb4aab261 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 18 Aug 2009 10:50:20 +0200
Subject: [PATCH 1/3] Refactor tools code

Move parameter parsing in tools before attempting to do anything that
might fail - so that we have debug_level set correctly for potential
error messages. That allows printing the --help and --usage messages
without being root.

Fix code duplicates in tools and refactor its code a little to lay
ground for decoupling the synchronous interfaces.

Fixes: Trac ticket #75
Domain ranges: Fixes ticket #112
Fix typo in groupdel (#136)
---
 server/tools/sss_groupadd.c |   77 ++++++++--------------
 server/tools/sss_groupdel.c |   80 ++++++++--------------
 server/tools/sss_groupmod.c |  135 +++++++++++++++++++------------------
 server/tools/sss_useradd.c  |   89 ++++++++++---------------
 server/tools/sss_userdel.c  |   76 ++++++++-------------
 server/tools/sss_usermod.c  |  157 ++++++++++++++++++++++---------------------
 server/tools/tools_util.c   |   94 ++++++++++++++++++++------
 server/tools/tools_util.h   |    6 ++-
 8 files changed, 346 insertions(+), 368 deletions(-)

diff --git a/server/tools/sss_groupadd.c b/server/tools/sss_groupadd.c
index 5c4733d..ab313fb 100644
--- a/server/tools/sss_groupadd.c
+++ b/server/tools/sss_groupadd.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <unistd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 
 #include "util/util.h"
 #include "db/sysdb.h"
@@ -124,17 +122,7 @@ static int groupadd_legacy(struct ops_ctx *ctx)
     APPEND_PARAM(command, GROUPADD_GID, ctx->gid);
     APPEND_STRING(command, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -152,9 +140,7 @@ int main(int argc, const char **argv)
             0, _("The GID of the group"), NULL },
         POPT_TABLEEND
     };
-    struct sss_domain_info *dom;
     poptContext pc = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     struct ops_ctx *data = NULL;
     int ret = EXIT_SUCCESS;
@@ -169,24 +155,6 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory.\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
     /* parse params */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
@@ -207,29 +175,39 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    ret = parse_name_domain(data, pc_groupname);
-    if (ret != EOK) {
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if(ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
         ret = EXIT_FAILURE;
         goto fini;
     }
 
-    data->gid = pc_gid;
-
-    ret = get_domain_by_id(data->ctx, data->gid, &dom);
+    ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain && data->gid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected GID %llu\n",
-                data->domain->name, (unsigned long long int) data->gid);
+
+    data->gid = pc_gid;
+
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected GID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     ret = get_domain_type(data->ctx, data->domain);
     switch (ret) {
@@ -249,7 +227,7 @@ int main(int argc, const char **argv)
             goto fini;
 
         case ID_IN_OTHER:
-            DEBUG(1, ("Cannot add group to domain %s\n", dom->name));
+            DEBUG(1, ("Cannot add group to domain %s\n", data->domain->name));
             ERROR("Unsupported domain type");
             ret = EXIT_FAILURE;
             goto fini;
@@ -262,7 +240,7 @@ int main(int argc, const char **argv)
     }
 
     /* add_group */
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not add group.\n");
@@ -272,14 +250,14 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, add_group, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ev);
     }
 
     if (data->error) {
         ret = data->error;
         switch (ret) {
             case EEXIST:
-                ERROR("A group with the same name or UID already exists\n");
+                ERROR("A group with the same name or GID already exists\n");
                 break;
 
             default:
@@ -294,7 +272,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 fini:
     talloc_free(data);
-    talloc_free(ctx);
     poptFreeContext(pc);
     exit(ret);
 }
diff --git a/server/tools/sss_groupdel.c b/server/tools/sss_groupdel.c
index 1cf9ed0..3138e0d 100644
--- a/server/tools/sss_groupdel.c
+++ b/server/tools/sss_groupdel.c
@@ -24,8 +24,6 @@
 #include <talloc.h>
 #include <popt.h>
 #include <grp.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "db/sysdb.h"
@@ -122,17 +120,7 @@ static int groupdel_legacy(struct ops_ctx *ctx)
     APPEND_STRING(command, GROUPDEL);
     APPEND_PARAM(command, GROUPDEL_GROUPNAME, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -143,9 +131,7 @@ int main(int argc, const char **argv)
     int ret = EXIT_SUCCESS;
     int pc_debug = 0;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
-    struct sss_domain_info *dom;
     struct group *grp_info;
     const char *pc_groupname = NULL;
     enum id_domain domain_type;
@@ -167,28 +153,10 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
     /* parse ops_ctx */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
-    poptSetOtherOptionHelp(pc, "USERNAME");
+    poptSetOtherOptionHelp(pc, "GROUPNAME");
     if((ret = poptGetNextOpt(pc)) < -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
@@ -204,6 +172,16 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if(ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
@@ -216,22 +194,21 @@ int main(int argc, const char **argv)
         data->gid = grp_info->gr_gid;
     }
 
-    /* arguments processed, go on to actual work */
-    ret = get_domain_by_id(data->ctx, data->gid, &dom);
-    if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-    if (data->domain && data->gid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected GID %llu\n",
-                data->domain->name, (unsigned long long int) data->gid);
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected GID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     domain_type = get_domain_type(data->ctx, data->domain);
     switch (domain_type) {
@@ -253,7 +230,7 @@ int main(int argc, const char **argv)
             goto fini;
 
         case ID_IN_OTHER:
-            DEBUG(1, ("Cannot remove group from domain %s\n", dom->name));
+            DEBUG(1, ("Cannot remove group from domain %s\n", data->domain->name));
             ERROR("Unsupported domain type\n");
             ret = EXIT_FAILURE;
             goto fini;
@@ -266,7 +243,7 @@ int main(int argc, const char **argv)
     }
 
     /* groupdel */
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not remove group.\n");
@@ -276,7 +253,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, group_del, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ev);
     }
 
     if (data->error) {
@@ -290,7 +267,7 @@ int main(int argc, const char **argv)
                     ret = EXIT_SUCCESS;
                     goto fini;
                 }
-                ERROR("No such user\n");
+                ERROR("No such group\n");
                 break;
 
             default:
@@ -304,7 +281,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
-    talloc_free(ctx);
     talloc_free(data);
     poptFreeContext(pc);
     exit(ret);
diff --git a/server/tools/sss_groupmod.c b/server/tools/sss_groupmod.c
index cca9abc..8dfd157 100644
--- a/server/tools/sss_groupmod.c
+++ b/server/tools/sss_groupmod.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <grp.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -294,17 +292,7 @@ static int groupmod_legacy(struct tools_ctx *tools_ctx,
 
     APPEND_PARAM(command, GROUPMOD_GROUPNAME, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -327,11 +315,9 @@ int main(int argc, const char **argv)
         POPT_TABLEEND
     };
     poptContext pc = NULL;
-    struct sss_domain_info *dom;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
-    char *groups;
+    char *addgroups = NULL, *rmgroups = NULL;
     int ret;
     struct group *grp_info;
     gid_t old_gid = 0;
@@ -346,49 +332,28 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
 
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
-    poptSetOtherOptionHelp(pc, "USERNAME");
+    poptSetOtherOptionHelp(pc, "GROUPNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
-        if (ret == 'a' || ret == 'r') {
-            groups = poptGetOptArg(pc);
-            if (!groups) {
-                ret = -1;
+        switch (ret) {
+            case 'a':
+                addgroups = poptGetOptArg(pc);
+                if (addgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
-
-            ret = parse_groups(ctx,
-                    groups,
-                    (ret == 'a') ? (&data->addgroups) : (&data->rmgroups));
 
-            free(groups);
-            if (ret != EOK) {
+            case 'r':
+                rmgroups = poptGetOptArg(pc);
+                if (rmgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
         }
     }
 
-    debug_level = pc_debug;
-
     if (ret != -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
@@ -403,6 +368,18 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    debug_level = pc_debug;
+
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
@@ -411,27 +388,49 @@ int main(int argc, const char **argv)
 
     data->gid = pc_gid;
 
+    if (addgroups) {
+        ret = parse_groups(data,
+                addgroups,
+                &data->addgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the group to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
+    if (rmgroups) {
+        ret = parse_groups(data,
+                rmgroups,
+                &data->rmgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to remove the group from\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
     /* arguments processed, go on to actual work */
     grp_info = getgrnam(data->name);
     if (grp_info) {
        old_gid = grp_info->gr_gid;
     }
 
-    ret = get_domain_by_id(data->ctx, data->gid, &dom);
-    if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-    if (data->domain && data->gid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected GID %llu\n",
-                data->domain->name, (unsigned long long int) data->gid);
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected GID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     ret = get_domain_type(data->ctx, data->domain);
     switch (ret) {
@@ -439,7 +438,7 @@ int main(int argc, const char **argv)
             break;
 
         case ID_IN_LEGACY_LOCAL:
-            ret = groupmod_legacy(ctx, data, data->domain);
+            ret = groupmod_legacy(data->ctx, data, data->domain);
             if(ret != EOK) {
                 ERROR("Cannot delete group from domain using the legacy tools\n");
             }
@@ -451,7 +450,7 @@ int main(int argc, const char **argv)
             goto fini;
 
         case ID_IN_OTHER:
-            DEBUG(1, ("Cannot modify group from domain %s\n", dom->name));
+            DEBUG(1, ("Cannot modify group from domain %s\n", data->domain->name));
             ERROR("Unsupported domain type\n");
             ret = EXIT_FAILURE;
             goto fini;
@@ -463,7 +462,7 @@ int main(int argc, const char **argv)
             goto fini;
     }
 
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify group.\n");
@@ -473,7 +472,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, mod_group, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -500,7 +499,9 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     exit(ret);
 }
diff --git a/server/tools/sss_useradd.c b/server/tools/sss_useradd.c
index 51f0eed..2d7ee2e 100644
--- a/server/tools/sss_useradd.c
+++ b/server/tools/sss_useradd.c
@@ -25,8 +25,6 @@
 #include <talloc.h>
 #include <popt.h>
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -305,17 +303,7 @@ static int useradd_legacy(struct ops_ctx *ctx, char *grouplist)
 
     APPEND_PARAM(command, USERADD_USERNAME, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -345,7 +333,6 @@ int main(int argc, const char **argv)
     poptContext pc = NULL;
     struct sss_domain_info *dom = NULL;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     char *groups = NULL;
     int ret;
@@ -359,26 +346,8 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
 
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for ops_ctx context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
@@ -388,11 +357,6 @@ int main(int argc, const char **argv)
                 ret = -1;
                 break;
             }
-
-            ret = parse_groups(ctx, groups, &data->groups);
-            if (ret != EOK) {
-                break;
-            }
         }
     }
 
@@ -412,12 +376,31 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = parse_name_domain(data, pc_username);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
         goto fini;
     }
 
+    if (groups) {
+        ret = parse_groups(data, groups, &data->groups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the user to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
     /* Same as shadow-utils useradd, -g can specify gid or group name */
     if (pc_group != NULL) {
         ret = get_gid(data, pc_group);
@@ -480,21 +463,21 @@ int main(int argc, const char **argv)
     }
 
     /* arguments processed, go on to actual work */
-    ret = get_domain_by_id(data->ctx, data->uid, &dom);
-    if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-    if (data->domain && data->uid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected UID %llu\n",
-                data->domain->name, (unsigned long long int) data->uid);
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected UID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     ret = get_domain_type(data->ctx, data->domain);
     switch (ret) {
@@ -527,7 +510,7 @@ int main(int argc, const char **argv)
     }
 
     /* useradd */
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify user.\n");
@@ -537,7 +520,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, add_user, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -561,7 +544,7 @@ int main(int argc, const char **argv)
 
 fini:
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     free(groups);
     exit(ret);
 }
diff --git a/server/tools/sss_userdel.c b/server/tools/sss_userdel.c
index 7c796bb..fea12e1 100644
--- a/server/tools/sss_userdel.c
+++ b/server/tools/sss_userdel.c
@@ -23,8 +23,6 @@
 #include <stdlib.h>
 #include <talloc.h>
 #include <popt.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <pwd.h>
 #include <unistd.h>
 
@@ -123,17 +121,7 @@ static int userdel_legacy(struct ops_ctx *ctx)
     APPEND_STRING(command, USERDEL);
     APPEND_PARAM(command, USERDEL_USERNAME, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -143,7 +131,6 @@ int main(int argc, const char **argv)
 {
     int ret = EXIT_SUCCESS;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     struct sss_domain_info *dom;
     struct passwd *pwd_info;
@@ -168,26 +155,8 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
 
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     if ((ret = poptGetNextOpt(pc)) < -1) {
@@ -205,6 +174,16 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if(ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = parse_name_domain(data, pc_username);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
@@ -217,21 +196,21 @@ int main(int argc, const char **argv)
         data->uid = pwd_info->pw_uid;
     }
 
-    ret = get_domain_by_id(data->ctx, data->uid, &dom);
-    if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-    if (data->domain && data->uid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected UID %llu\n",
-                data->domain->name, (unsigned long long int) data->uid);
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected UID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     domain_type = get_domain_type(data->ctx, data->domain);
     switch (domain_type) {
@@ -253,7 +232,7 @@ int main(int argc, const char **argv)
             goto fini;
 
         case ID_IN_OTHER:
-            DEBUG(1, ("Cannot remove user from domain %s\n", dom->name));
+            DEBUG(1, ("Cannot remove user from domain %s\n", data->domain->name));
             ERROR("Unsupported domain type\n");
             ret = EXIT_FAILURE;
             goto fini;
@@ -266,7 +245,7 @@ int main(int argc, const char **argv)
     }
 
     /* userdel */
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not remove user.\n");
@@ -276,7 +255,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, user_del, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -304,7 +283,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
-    talloc_free(ctx);
     talloc_free(data);
     poptFreeContext(pc);
     exit(ret);
diff --git a/server/tools/sss_usermod.c b/server/tools/sss_usermod.c
index 1dfd2a7..126252c 100644
--- a/server/tools/sss_usermod.c
+++ b/server/tools/sss_usermod.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <pwd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -344,17 +342,7 @@ static int usermod_legacy(struct tools_ctx *tools_ctx, struct ops_ctx *ctx,
 
     APPEND_PARAM(command, USERMOD_USERNAME, ctx->name);
 
-    ret = system(command);
-    if (ret) {
-        if (ret == -1) {
-            DEBUG(1, ("system(3) failed\n"));
-        } else {
-            DEBUG(1, ("Could not exec '%s', return code: %d\n",
-                      command, WEXITSTATUS(ret)));
-        }
-        talloc_free(command);
-        return EFAULT;
-    }
+    ret = run_command(command);
 
     talloc_free(command);
     return ret;
@@ -384,11 +372,9 @@ int main(int argc, const char **argv)
         POPT_TABLEEND
     };
     poptContext pc = NULL;
-    struct sss_domain_info *dom;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
-    char *groups;
+    char *addgroups = NULL, *rmgroups = NULL;
     int ret;
     struct passwd *pwd_info;
     uid_t old_uid = 0;
@@ -403,66 +389,44 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
 
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    data->attrs = sysdb_new_attrs(ctx);
-    if (data->attrs == NULL) {
-        DEBUG(1, ("Could not allocate memory for sysdb_attrs context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
-        if (ret == 'a' || ret == 'r') {
-            groups = poptGetOptArg(pc);
-            if (!groups) {
-                ret = -1;
+        switch (ret) {
+            case 'a':
+                addgroups = poptGetOptArg(pc);
+                if (addgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
 
-            ret = parse_groups(ctx,
-                    groups,
-                    (ret == 'a') ? (&data->addgroups) : (&data->rmgroups));
+            case 'r':
+                rmgroups = poptGetOptArg(pc);
+                if (rmgroups == NULL) {
+                    ret = -1;
+                }
+                break;
 
-            free(groups);
-            if (ret != EOK) {
+            case 'L':
+                pc_lock = DO_LOCK;
+                break;
+
+            case 'U':
+                pc_lock = DO_UNLOCK;
                 break;
-            }
-        } else if (ret == 'L') {
-            pc_lock = DO_LOCK;
-        } else if (ret == 'U') {
-            pc_lock = DO_UNLOCK;
         }
     }
 
-    debug_level = pc_debug;
-
     if(ret != -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
         goto fini;
     }
 
+    debug_level = pc_debug;
+
     /* username is an argument without --option */
     pc_username = poptGetArg(pc);
     if (pc_username == NULL) {
@@ -471,32 +435,71 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    ret = parse_name_domain(data, pc_username);
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
     if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
         ret = EXIT_FAILURE;
         goto fini;
     }
 
-    pwd_info = getpwnam(data->name);
-    if (pwd_info) {
-        old_uid = pwd_info->pw_uid;
+    data->attrs = sysdb_new_attrs(data->ctx);
+    if (data->attrs == NULL) {
+        DEBUG(1, ("Could not allocate memory for sysdb_attrs context\n"));
+        ERROR("Out of memory\n");
+        return ENOMEM;
     }
 
-    ret = get_domain_by_id(data->ctx, data->uid, &dom);
+    ret = parse_name_domain(data, pc_username);
     if (ret != EOK) {
-        ERROR("Cannot get domain info\n");
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain && data->uid && data->domain != dom) {
-        ERROR("Selected domain %s conflicts with selected UID %llu\n",
-                data->domain->name, (unsigned long long int) data->uid);
+
+    if (addgroups) {
+        ret = parse_groups(data,
+                           addgroups,
+                           &data->addgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the user to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
+    if (rmgroups) {
+        ret = parse_groups(data,
+                           rmgroups,
+                           &data->rmgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to remove the user from\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
+    pwd_info = getpwnam(data->name);
+    if (pwd_info) {
+        old_uid = pwd_info->pw_uid;
+    }
+
+    ret = domain_lookup(data);
+    if (ret) {
+        switch (ret) {
+            case EINVAL:
+                ERROR("Selected domain %s conflicts with selected UID %llu\n",
+                        data->domain->name, (unsigned long long int) data->uid);
+                break;
+
+            default:
+                ERROR("Cannot get domain info\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
-    if (data->domain == NULL && dom) {
-        data->domain = dom;
-    }
 
     ret = get_domain_type(data->ctx, data->domain);
     switch (ret) {
@@ -504,7 +507,7 @@ int main(int argc, const char **argv)
             break;
 
         case ID_IN_LEGACY_LOCAL:
-            ret = usermod_legacy(ctx, data, pc_uid, pc_gid, pc_gecos,
+            ret = usermod_legacy(data->ctx, data, pc_uid, pc_gid, pc_gecos,
                                  pc_home, pc_shell, pc_lock, data->domain);
             if(ret != EOK) {
                 ERROR("Cannot delete user from domain using the legacy tools\n");
@@ -517,7 +520,7 @@ int main(int argc, const char **argv)
             goto fini;
 
         case ID_IN_OTHER:
-            DEBUG(1, ("Cannot modify user from domain %s\n", dom->name));
+            DEBUG(1, ("Cannot modify user from domain %s\n", data->domain->name));
             ERROR("Unsupported domain type\n");
             ret = EXIT_FAILURE;
             goto fini;
@@ -589,7 +592,7 @@ int main(int argc, const char **argv)
                   "Could not add attribute to changeset\n");
     }
 
-    req = sysdb_transaction_send(ctx, ctx->ev, ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify user.\n");
@@ -599,7 +602,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, mod_user, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -627,7 +630,9 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     exit(ret);
 }
diff --git a/server/tools/tools_util.c b/server/tools/tools_util.c
index 6201399..95a23d7 100644
--- a/server/tools/tools_util.c
+++ b/server/tools/tools_util.c
@@ -23,6 +23,9 @@
 #include <tevent.h>
 #include <popt.h>
 #include <errno.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 #include "util/util.h"
 #include "confdb/confdb.h"
@@ -116,21 +119,50 @@ int get_domain_by_id(struct tools_ctx *ctx,
         }
     }
 
-    if (dom == NULL && id == 0) {
-        dom = get_local_domain(ctx);
-        if (dom == NULL) {
-            DEBUG(1, ("Cannot find local domain info\n"));
-            ret = ENOENT;
+    *_dom = dom;
+    return ret;
+}
+
+int domain_lookup(struct ops_ctx *ops)
+{
+    int ret;
+    struct sss_domain_info *dom = NULL;
+
+    if (!ops) {
+        return EIO;
+    }
+
+    ret = get_domain_by_id(ops->ctx, ops->uid, &dom);
+    if (ret != EOK) {
+        return ret;
+    }
+
+    if (ops->domain) {
+        /* If domain given with FQDN, check conflict against the one selected
+         * with UID (if any) */
+        if (ops->uid && ops->domain != dom) {
+            return EINVAL;
+        }
+    } else {
+        if (!ops->uid) {
+            /* No hint given, default to local */
+            ops->domain = get_local_domain(domains);
+            if (!ops->domain) {
+                DEBUG(1, ("Cannot find local domain info\n"));
+                ret = ENOENT;
+            }
+        } else {
+            /* Only ID given, use that */
+            ops->domain = dom;
         }
     }
 
-    *_dom = dom;
-    return ret;
+
+    return EOK;
 }
 
 int setup_db(struct tools_ctx **tools_ctx)
 {
-    TALLOC_CTX *tmp_ctx;
     char *confdb_path;
     struct tools_ctx *ctx;
     int ret;
@@ -149,11 +181,7 @@ int setup_db(struct tools_ctx **tools_ctx)
         return EIO;
     }
 
-    tmp_ctx = talloc_new(ctx);
-    if (!tmp_ctx)
-        return ENOMEM;
-
-    confdb_path = talloc_asprintf(tmp_ctx, "%s/%s", DB_PATH, CONFDB_FILE);
+    confdb_path = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
     if (confdb_path == NULL) {
         talloc_free(ctx);
         return ENOMEM;
@@ -182,7 +210,7 @@ int setup_db(struct tools_ctx **tools_ctx)
         return ret;
     }
 
-    talloc_free(tmp_ctx);
+    talloc_free(confdb_path);
     *tools_ctx = ctx;
     return EOK;
 }
@@ -196,7 +224,6 @@ void usage(poptContext pc, const char *error)
     if (error) fprintf(stderr, "%s", error);
 }
 
-/* FIXME: avoid using strtok !! */
 int parse_groups(TALLOC_CTX *mem_ctx, const char *optstr, char ***_out)
 {
     char **out;
@@ -274,6 +301,23 @@ int parse_name_domain(struct ops_ctx *octx,
     return EOK;
 }
 
+int run_command(const char *command)
+{
+    int ret;
+
+    ret = system(command);
+    if (ret != EXIT_SUCCESS) {
+        if (ret == -1) {
+            DEBUG(1, ("system(3) failed\n"));
+        } else {
+            DEBUG(1, ("Could not exec '%s', return code: %d\n",
+                      command, WEXITSTATUS(ret)));
+        }
+    }
+
+    return ret;
+}
+
 int set_locale(void)
 {
     char *c;
@@ -298,26 +342,36 @@ int set_locale(void)
     return EOK;
 }
 
-int init_sss_tools(struct tools_ctx **_ctx)
+int init_sss_tools(struct ops_ctx **_octx)
 {
     int ret;
-    struct tools_ctx *ctx;
+    struct tools_ctx *tctx;
+    struct ops_ctx *octx;
 
     /* Connect to the database */
-    ret = setup_db(&ctx);
+    ret = setup_db(&tctx);
     if (ret != EOK) {
         DEBUG(1, ("Could not set up database\n"));
         ret = EXIT_FAILURE;
         goto fini;
     }
 
-    ret = sss_names_init(ctx, ctx->confdb, &ctx->snctx);
+    ret = sss_names_init(tctx, tctx->confdb, &tctx->snctx);
     if (ret != EOK) {
         DEBUG(1, ("Could not set up parsing\n"));
         goto fini;
     }
 
-    *_ctx = ctx;
+    octx = talloc_zero(tctx, struct ops_ctx);
+    if (octx == NULL) {
+        DEBUG(1, ("Could not allocate memory for data context\n"));
+        ERROR("Out of memory\n");
+        ret = ENOMEM;
+        goto fini;
+    }
+    octx->ctx = tctx;
+
+    *_octx = octx;
     ret = EOK;
 fini:
     return ret;
diff --git a/server/tools/tools_util.h b/server/tools/tools_util.h
index 772ba36..616608f 100644
--- a/server/tools/tools_util.h
+++ b/server/tools/tools_util.h
@@ -72,7 +72,7 @@ struct ops_ctx {
     bool done;
 };
 
-int init_sss_tools(struct tools_ctx **_ctx);
+int init_sss_tools(struct ops_ctx **_octx);
 
 int setup_db(struct tools_ctx **ctx);
 
@@ -87,6 +87,10 @@ int get_domain_by_id(struct tools_ctx *ctx,
                      uint32_t id,
                      struct sss_domain_info **_dom);
 
+int domain_lookup(struct ops_ctx *ops);
+
+int run_command(const char *command);
+
 int set_locale(void);
 
 int parse_name_domain(struct ops_ctx *octx,
-- 
1.6.2.5

