>From bf7861ae8c42bc56cac9794191b96033d6e5193d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Thu, 13 Aug 2009 13:39:50 +0200
Subject: [PATCH] Move parsing parameters in tools

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. Also allows printing the --help and --usage messages
without being root.

Fixes: Trac ticket #75
---
 server/tools/sss_groupadd.c |   37 +++++++-------
 server/tools/sss_groupdel.c |   37 +++++++-------
 server/tools/sss_groupmod.c |   94 +++++++++++++++++++++++--------------
 server/tools/sss_useradd.c  |   53 +++++++++++---------
 server/tools/sss_userdel.c  |   37 +++++++-------
 server/tools/sss_usermod.c  |  110 +++++++++++++++++++++++++++----------------
 6 files changed, 214 insertions(+), 154 deletions(-)

diff --git a/server/tools/sss_groupadd.c b/server/tools/sss_groupadd.c
index 5c4733d..a6484e4 100644
--- a/server/tools/sss_groupadd.c
+++ b/server/tools/sss_groupadd.c
@@ -169,24 +169,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,6 +189,25 @@ int main(int argc, const char **argv)
         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;
+
     ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
diff --git a/server/tools/sss_groupdel.c b/server/tools/sss_groupdel.c
index 9678208..183465a 100644
--- a/server/tools/sss_groupdel.c
+++ b/server/tools/sss_groupdel.c
@@ -166,24 +166,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 ops_ctx */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
@@ -203,6 +185,25 @@ int main(int argc, const char **argv)
         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;
+
     ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
diff --git a/server/tools/sss_groupmod.c b/server/tools/sss_groupmod.c
index 175d08d..6463fff 100644
--- a/server/tools/sss_groupmod.c
+++ b/server/tools/sss_groupmod.c
@@ -331,7 +331,7 @@ int main(int argc, const char **argv)
     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 +346,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 +382,27 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    debug_level = pc_debug;
+
+    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;
+
     ret = parse_name_domain(data, pc_groupname);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
@@ -411,6 +411,28 @@ int main(int argc, const char **argv)
 
     data->gid = pc_gid;
 
+    if (addgroups) {
+        ret = parse_groups(ctx,
+                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(ctx,
+                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) {
@@ -496,6 +518,8 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
     talloc_free(ctx);
     exit(ret);
diff --git a/server/tools/sss_useradd.c b/server/tools/sss_useradd.c
index 51f0eed..ed61112 100644
--- a/server/tools/sss_useradd.c
+++ b/server/tools/sss_useradd.c
@@ -359,26 +359,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 +370,6 @@ int main(int argc, const char **argv)
                 ret = -1;
                 break;
             }
-
-            ret = parse_groups(ctx, groups, &data->groups);
-            if (ret != EOK) {
-                break;
-            }
         }
     }
 
@@ -412,12 +389,40 @@ int main(int argc, const char **argv)
         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;
+
     ret = parse_name_domain(data, pc_username);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
         goto fini;
     }
 
+    if (groups) {
+        ret = parse_groups(ctx, 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);
diff --git a/server/tools/sss_userdel.c b/server/tools/sss_userdel.c
index be35b4f..cb413d0 100644
--- a/server/tools/sss_userdel.c
+++ b/server/tools/sss_userdel.c
@@ -167,6 +167,25 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
+
+    /* parse parameters */
+    pc = poptGetContext(NULL, argc, argv, long_options, 0);
+    poptSetOtherOptionHelp(pc, "USERNAME");
+    if ((ret = poptGetNextOpt(pc)) < -1) {
+        usage(pc, poptStrerror(ret));
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
+    debug_level = pc_debug;
+
+    pc_username = poptGetArg(pc);
+    if (pc_username == NULL) {
+        usage(pc, _("Specify user to delete\n"));
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     CHECK_ROOT(ret, debug_prg_name);
 
     ret = init_sss_tools(&ctx);
@@ -186,24 +205,6 @@ int main(int argc, const char **argv)
     data->ctx = ctx;
     data->ev = ctx->ev;
 
-    /* parse ops_ctx */
-    pc = poptGetContext(NULL, argc, argv, long_options, 0);
-    poptSetOtherOptionHelp(pc, "USERNAME");
-    if ((ret = poptGetNextOpt(pc)) < -1) {
-        usage(pc, poptStrerror(ret));
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    debug_level = pc_debug;
-
-    pc_username = poptGetArg(pc);
-    if (pc_username == NULL) {
-        usage(pc, _("Specify user to delete\n"));
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
     ret = parse_name_domain(data, pc_username);
     if (ret != EOK) {
         ret = EXIT_FAILURE;
diff --git a/server/tools/sss_usermod.c b/server/tools/sss_usermod.c
index 0e1055d..700d182 100644
--- a/server/tools/sss_usermod.c
+++ b/server/tools/sss_usermod.c
@@ -388,7 +388,7 @@ int main(int argc, const char **argv)
     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,6 +403,52 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
+
+    /* parse parameters */
+    pc = poptGetContext(NULL, argc, argv, long_options, 0);
+    poptSetOtherOptionHelp(pc, "USERNAME");
+    while ((ret = poptGetNextOpt(pc)) > 0) {
+        switch (ret) {
+            case 'a':
+                addgroups = poptGetOptArg(pc);
+                if (addgroups == NULL) {
+                    ret = -1;
+                }
+                break;
+
+            case 'r':
+                rmgroups = poptGetOptArg(pc);
+                if (rmgroups == NULL) {
+                    ret = -1;
+                }
+                break;
+
+            case 'L':
+                pc_lock = DO_LOCK;
+                break;
+
+            case 'U':
+                pc_lock = DO_UNLOCK;
+                break;
+        }
+    }
+
+    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) {
+        usage(pc, _("Specify user to modify\n"));
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     CHECK_ROOT(ret, debug_prg_name);
 
     ret = init_sss_tools(&ctx);
@@ -429,52 +475,32 @@ int main(int argc, const char **argv)
         return ENOMEM;
     }
 
-    /* parse ops_ctx */
-    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;
-                break;
-            }
-
-            ret = parse_groups(ctx,
-                    groups,
-                    (ret == 'a') ? (&data->addgroups) : (&data->rmgroups));
-
-            free(groups);
-            if (ret != EOK) {
-                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 = parse_name_domain(data, pc_username);
+    if (ret != EOK) {
         ret = EXIT_FAILURE;
         goto fini;
     }
 
-    /* username is an argument without --option */
-    pc_username = poptGetArg(pc);
-    if (pc_username == NULL) {
-        usage(pc, _("Specify user to modify\n"));
-        ret = EXIT_FAILURE;
-        goto fini;
+    if (addgroups) {
+        ret = parse_groups(ctx,
+                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;
+        }
     }
 
-    ret = parse_name_domain(data, pc_username);
-    if (ret != EOK) {
-        ret = EXIT_FAILURE;
-        goto fini;
+    if (rmgroups) {
+        ret = parse_groups(ctx,
+                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);
@@ -623,6 +649,8 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
     talloc_free(ctx);
     exit(ret);
-- 
1.6.2.5

