Fix error messages in tools

Add getpwnam, getgrnam sync versions

Fix ticket #164: Groupnames in non-local domains
Fix ticket #100: Error Message Modifying a user that doesn't Exist
Fix ticket #214: incorrect error message when MPG already exists
Fix ticket #188: Deleting and modifying users in non-local domain
Fix ticket #120: Adding a user to a full domain gives unhelpful error message

10 files changed, 342 insertions, 35 deletions

diff --git a/server/tools/sss_groupadd.c b/server/tools/sss_groupadd.c
index 1f6a2f11..82d4573d 100644
--- a/server/tools/sss_groupadd.c
+++ b/server/tools/sss_groupadd.c
@@ -125,6 +125,10 @@ done:
     if (tctx->error) {
         ret = tctx->error;
         switch (ret) {
+            case ERANGE:
+                ERROR("Could not allocate ID for the group - domain full?\n");
+                break;
+
             case EEXIST:
                 ERROR("A group with the same name or GID already exists\n");
                 break;
diff --git a/server/tools/sss_groupdel.c b/server/tools/sss_groupdel.c
index a4451c43..d6e3dfd6 100644
--- a/server/tools/sss_groupdel.c
+++ b/server/tools/sss_groupdel.c
@@ -23,8 +23,6 @@
 #include <stdlib.h>
 #include <talloc.h>
 #include <popt.h>
-#include <grp.h>
-#include <unistd.h>
 
 #include "db/sysdb.h"
 #include "util/util.h"
@@ -35,7 +33,6 @@ int main(int argc, const char **argv)
 {
     int ret = EXIT_SUCCESS;
     int pc_debug = 0;
-    struct group *grp_info;
     const char *pc_groupname = NULL;
     struct tools_ctx *tctx = NULL;
 
@@ -93,19 +90,6 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    /* arguments processed, go on to actual work */
-    grp_info = getgrnam(tctx->octx->name);
-    if (grp_info) {
-        tctx->octx->gid = grp_info->gr_gid;
-    }
-
-    /* arguments processed, go on to actual work */
-    if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) {
-        ERROR("The selected GID is outside the allowed range\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
     start_transaction(tctx);
     if (tctx->error != EOK) {
         goto done;
@@ -129,7 +113,8 @@ done:
         DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret)));
         switch (ret) {
             case ENOENT:
-                ERROR("No such group\n");
+                ERROR("No such group in local domain. "
+                      "Removing groups only allowed in local domain.\n");
                 break;
 
             default:
diff --git a/server/tools/sss_groupmod.c b/server/tools/sss_groupmod.c
index e821fdc3..d3a35988 100644
--- a/server/tools/sss_groupmod.c
+++ b/server/tools/sss_groupmod.c
@@ -53,6 +53,7 @@ int main(int argc, const char **argv)
     char *addgroups = NULL, *rmgroups = NULL;
     int ret;
     const char *pc_groupname = NULL;
+    char *badgroup = NULL;
 
     debug_prg_name = argv[0];
 
@@ -117,6 +118,17 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
+    /* check the username to be able to give sensible error message */
+    ret = sysdb_getgrnam_sync(tctx, tctx->ev, tctx->sysdb,
+                              tctx->octx->name, tctx->local,
+                              &tctx->octx);
+    if (ret != EOK) {
+        ERROR("Cannot find group in local domain, "
+              "modifying groups is allowed only in local domain\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
 
     tctx->octx->gid = pc_gid;
 
@@ -125,6 +137,7 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse groups to add the group to\n"));
             ERROR("Internal error while parsing parameters\n");
+            ret = EXIT_FAILURE;
             goto fini;
         }
 
@@ -132,6 +145,16 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse FQDN groups to add the group to\n"));
             ERROR("Member groups must be in the same domain as parent group\n");
+            ret = EXIT_FAILURE;
+            goto fini;
+        }
+
+        /* Check group names in the LOCAL domain */
+        ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup);
+        if (ret != EOK) {
+            ERROR("Cannot find group %s in local domain, "
+                  "only groups in local domain are allowed\n", badgroup);
+            ret = EXIT_FAILURE;
             goto fini;
         }
     }
@@ -141,6 +164,7 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse groups to remove the group from\n"));
             ERROR("Internal error while parsing parameters\n");
+            ret = EXIT_FAILURE;
             goto fini;
         }
 
@@ -148,6 +172,16 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse FQDN groups to remove the group from\n"));
             ERROR("Member groups must be in the same domain as parent group\n");
+            ret = EXIT_FAILURE;
+            goto fini;
+        }
+
+        /* Check group names in the LOCAL domain */
+        ret = check_group_names(tctx, tctx->octx->rmgroups, &badgroup);
+        if (ret != EOK) {
+            ERROR("Cannot find group %s in local domain, "
+                  "only groups in local domain are allowed\n", badgroup);
+            ret = EXIT_FAILURE;
             goto fini;
         }
     }
diff --git a/server/tools/sss_sync_ops.c b/server/tools/sss_sync_ops.c
index 68fe73e5..932a7122 100644
--- a/server/tools/sss_sync_ops.c
+++ b/server/tools/sss_sync_ops.c
@@ -44,6 +44,7 @@
 } while(0)
 
 struct sync_op_res {
+    struct ops_ctx *data;
     int error;
     bool done;
 };
@@ -1138,6 +1139,10 @@ int useradd_defaults(TALLOC_CTX *mem_ctx,
 
     if (homedir) {
         data->home = talloc_strdup(data, homedir);
+        if (data->home == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
     } else {
         ret = confdb_get_string(confdb, mem_ctx,
                                 conf_path, CONFDB_LOCAL_DEFAULT_BASEDIR,
@@ -1546,3 +1551,199 @@ static void end_transaction_done(struct tevent_req *req)
     talloc_zfree(req);
 }
 
+/*
+ * getpwnam, getgrnam and friends
+ */
+static void sss_getpwnam_done(void *ptr, int status,
+                              struct ldb_result *lrs);
+
+int sysdb_getpwnam_sync(TALLOC_CTX *mem_ctx,
+                        struct tevent_context *ev,
+                        struct sysdb_ctx *sysdb,
+                        const char *name,
+                        struct sss_domain_info *domain,
+                        struct ops_ctx **out)
+{
+    int ret;
+    struct sync_op_res *res = NULL;
+
+    res = talloc_zero(mem_ctx, struct sync_op_res);
+    if (!res) {
+        return ENOMEM;
+    }
+
+    if (out == NULL) {
+        DEBUG(1, ("NULL passed for storage pointer\n"));
+        return EINVAL;
+    }
+    res->data = *out;
+
+    ret = sysdb_getpwnam(mem_ctx,
+                         sysdb,
+                         domain,
+                         name,
+                         sss_getpwnam_done,
+                         res);
+
+    SYNC_LOOP(res, ret);
+
+    return ret;
+}
+
+static void sss_getpwnam_done(void *ptr, int status,
+                              struct ldb_result *lrs)
+{
+    struct sync_op_res *res = talloc_get_type(ptr, struct sync_op_res );
+    const char *str;
+
+    res->done = true;
+
+    if (status != LDB_SUCCESS) {
+        res->error = status;
+        return;
+    }
+
+    switch (lrs->count) {
+        case 0:
+            DEBUG(1, ("No result for sysdb_getpwnam call\n"));
+            res->error = ENOENT;
+            break;
+
+        case 1:
+            res->error = EOK;
+            /* fill ops_ctx */
+            res->data->uid = ldb_msg_find_attr_as_uint64(lrs->msgs[0],
+                                                         SYSDB_UIDNUM, 0);
+
+            res->data->gid = ldb_msg_find_attr_as_uint64(lrs->msgs[0],
+                                                         SYSDB_GIDNUM, 0);
+
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_NAME, NULL);
+            res->data->name = talloc_strdup(res, str);
+            if (res->data->name == NULL) {
+                res->error = ENOMEM;
+                return;
+            }
+
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_GECOS, NULL);
+            res->data->gecos = talloc_strdup(res, str);
+            if (res->data->gecos == NULL) {
+                res->error = ENOMEM;
+                return;
+            }
+
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_HOMEDIR, NULL);
+            res->data->home = talloc_strdup(res, str);
+            if (res->data->home == NULL) {
+                res->error = ENOMEM;
+                return;
+            }
+
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_SHELL, NULL);
+            res->data->shell = talloc_strdup(res, str);
+            if (res->data->shell == NULL) {
+                res->error = ENOMEM;
+                return;
+            }
+
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_DISABLED, NULL);
+            if (str == NULL) {
+                res->data->lock = DO_UNLOCK;
+            } else {
+                if (strcasecmp(str, "true") == 0) {
+                    res->data->lock = DO_LOCK;
+                } else if (strcasecmp(str, "false") == 0) {
+                    res->data->lock = DO_UNLOCK;
+                } else { /* Invalid value */
+                    DEBUG(2, ("Invalid value for %s attribute: %s\n",
+                              SYSDB_DISABLED, str ? str : "NULL"));
+                    res->error = EIO;
+                    return;
+                }
+            }
+            break;
+
+        default:
+            DEBUG(1, ("More than one result for sysdb_getpwnam call\n"));
+            res->error = EIO;
+            break;
+    }
+}
+
+static void sss_getgrnam_done(void *ptr, int status,
+                              struct ldb_result *lrs);
+
+int sysdb_getgrnam_sync(TALLOC_CTX *mem_ctx,
+                        struct tevent_context *ev,
+                        struct sysdb_ctx *sysdb,
+                        const char *name,
+                        struct sss_domain_info *domain,
+                        struct ops_ctx **out)
+{
+    int ret;
+    struct sync_op_res *res = NULL;
+
+    res = talloc_zero(mem_ctx, struct sync_op_res);
+    if (!res) {
+        return ENOMEM;
+    }
+
+    if (out == NULL) {
+        DEBUG(1, ("NULL passed for storage pointer\n"));
+        return EINVAL;
+    }
+    res->data = *out;
+
+    ret = sysdb_getgrnam(mem_ctx,
+                         sysdb,
+                         domain,
+                         name,
+                         sss_getgrnam_done,
+                         res);
+
+    SYNC_LOOP(res, ret);
+
+    return ret;
+}
+
+static void sss_getgrnam_done(void *ptr, int status,
+                              struct ldb_result *lrs)
+{
+    struct sync_op_res *res = talloc_get_type(ptr, struct sync_op_res );
+    const char *str;
+
+    res->done = true;
+
+    if (status != LDB_SUCCESS) {
+        res->error = status;
+        return;
+    }
+
+    switch (lrs->count) {
+        case 0:
+            DEBUG(1, ("No result for sysdb_getgrnam call\n"));
+            res->error = ENOENT;
+            break;
+
+            /* sysdb_getgrnam also returns members */
+        default:
+            res->error = EOK;
+            /* fill ops_ctx */
+            res->data->gid = ldb_msg_find_attr_as_uint64(lrs->msgs[0],
+                                                         SYSDB_GIDNUM, 0);
+            str = ldb_msg_find_attr_as_string(lrs->msgs[0],
+                                              SYSDB_NAME, NULL);
+            res->data->name = talloc_strdup(res, str);
+            if (res->data->name == NULL) {
+                res->error = ENOMEM;
+                return;
+            }
+            break;
+    }
+}
+
diff --git a/server/tools/sss_sync_ops.h b/server/tools/sss_sync_ops.h
index c99b9b91..3988992e 100644
--- a/server/tools/sss_sync_ops.h
+++ b/server/tools/sss_sync_ops.h
@@ -86,5 +86,19 @@ int groupmod(TALLOC_CTX *mem_ctx,
 void start_transaction(struct tools_ctx *tctx);
 void end_transaction(struct tools_ctx *tctx);
 
+int sysdb_getpwnam_sync(TALLOC_CTX *mem_ctx,
+                        struct tevent_context *ev,
+                        struct sysdb_ctx *sysdb,
+                        const char *name,
+                        struct sss_domain_info *domain,
+                        struct ops_ctx **out);
+
+int sysdb_getgrnam_sync(TALLOC_CTX *mem_ctx,
+                        struct tevent_context *ev,
+                        struct sysdb_ctx *sysdb,
+                        const char *name,
+                        struct sss_domain_info *domain,
+                        struct ops_ctx **out);
+
 #endif /* __SSS_OPS_H__ */
 
diff --git a/server/tools/sss_useradd.c b/server/tools/sss_useradd.c
index 07e741e4..f805b242 100644
--- a/server/tools/sss_useradd.c
+++ b/server/tools/sss_useradd.c
@@ -131,6 +131,7 @@ int main(int argc, const char **argv)
     poptContext pc = NULL;
     struct tools_ctx *tctx = NULL;
     char *groups = NULL;
+    char *badgroup = NULL;
     int ret;
 
     debug_prg_name = argv[0];
@@ -195,6 +196,7 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse groups to add the user to\n"));
             ERROR("Internal error while parsing parameters\n");
+            ret = EXIT_FAILURE;
             goto fini;
         }
 
@@ -202,6 +204,15 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse FQDN groups to add the user to\n"));
             ERROR("Groups must be in the same domain as user\n");
+            ret = EXIT_FAILURE;
+            goto fini;
+        }
+
+        /* Check group names in the LOCAL domain */
+        ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup);
+        if (ret != EOK) {
+            ERROR("Cannot find group %s in local domain\n", badgroup);
+            ret = EXIT_FAILURE;
             goto fini;
         }
     }
@@ -256,8 +267,12 @@ int main(int argc, const char **argv)
 done:
     if (tctx->error) {
         switch (tctx->error) {
+            case ERANGE:
+                ERROR("Could not allocate ID for the user - domain full?\n");
+                break;
+
             case EEXIST:
-                ERROR("A user with the same name or UID already exists\n");
+                ERROR("A user or group with the same name or ID already exists\n");
                 break;
 
             default:
diff --git a/server/tools/sss_userdel.c b/server/tools/sss_userdel.c
index 8c7c7bd7..d14ef3da 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 <pwd.h>
-#include <unistd.h>
 
 #include "db/sysdb.h"
 #include "util/util.h"
@@ -35,7 +33,6 @@ int main(int argc, const char **argv)
 {
     int ret = EXIT_SUCCESS;
     struct tools_ctx *tctx = NULL;
-    struct passwd *pwd_info;
     const char *pc_username = NULL;
 
     int pc_debug = 0;
@@ -93,18 +90,6 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    /* arguments processed, go on to actual work */
-    pwd_info = getpwnam(tctx->octx->name);
-    if (pwd_info) {
-        tctx->octx->uid = pwd_info->pw_uid;
-    }
-
-    if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) {
-        ERROR("The selected UID is outside the allowed range\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
     start_transaction(tctx);
     if (tctx->error != EOK) {
         goto done;
@@ -128,7 +113,8 @@ done:
         DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret)));
         switch (ret) {
             case ENOENT:
-                ERROR("No such user\n");
+                ERROR("No such user in local domain. "
+                      "Removing users only allowed in local domain.\n");
                 break;
 
             default:
diff --git a/server/tools/sss_usermod.c b/server/tools/sss_usermod.c
index 2c2b80ec..733f9982 100644
--- a/server/tools/sss_usermod.c
+++ b/server/tools/sss_usermod.c
@@ -60,6 +60,7 @@ int main(int argc, const char **argv)
     int ret;
     const char *pc_username = NULL;
     struct tools_ctx *tctx = NULL;
+    char *badgroup = NULL;
 
     debug_prg_name = argv[0];
 
@@ -133,6 +134,16 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
+    /* check the username to be able to give sensible error message */
+    ret = sysdb_getpwnam_sync(tctx, tctx->ev, tctx->sysdb,
+                              tctx->octx->name, tctx->local,
+                              &tctx->octx);
+    if (ret != EOK) {
+        ERROR("Cannot find user in local domain, "
+              "modifying users is allowed only in local domain\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
 
     if (id_in_range(tctx->octx->uid, tctx->octx->domain) != EOK) {
         ERROR("The selected UID is outside the allowed range\n");
@@ -145,6 +156,7 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse groups to add the user to\n"));
             ERROR("Internal error while parsing parameters\n");
+            ret = EXIT_FAILURE;
             goto fini;
         }
 
@@ -152,6 +164,16 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse FQDN groups to add the user to\n"));
             ERROR("Groups must be in the same domain as user\n");
+            ret = EXIT_FAILURE;
+            goto fini;
+        }
+
+        /* Check group names in the LOCAL domain */
+        ret = check_group_names(tctx, tctx->octx->addgroups, &badgroup);
+        if (ret != EOK) {
+            ERROR("Cannot find group %s in local domain, "
+                  "only groups in local domain are allowed\n", badgroup);
+            ret = EXIT_FAILURE;
             goto fini;
         }
     }
@@ -161,6 +183,7 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse groups to remove the user from\n"));
             ERROR("Internal error while parsing parameters\n");
+            ret = EXIT_FAILURE;
             goto fini;
         }
 
@@ -168,6 +191,16 @@ int main(int argc, const char **argv)
         if (ret != EOK) {
             DEBUG(1, ("Cannot parse FQDN groups to remove the user from\n"));
             ERROR("Groups must be in the same domain as user\n");
+            ret = EXIT_FAILURE;
+            goto fini;
+        }
+
+        /* Check group names in the LOCAL domain */
+        ret = check_group_names(tctx, tctx->octx->rmgroups, &badgroup);
+        if (ret != EOK) {
+            ERROR("Cannot find group %s in local domain, "
+                  "only groups in local domain are allowed\n", badgroup);
+            ret = EXIT_FAILURE;
             goto fini;
         }
     }
@@ -205,7 +238,7 @@ done:
                 break;
 
             case EFAULT:
-                ERROR("Could not modify user - check if username is correct\n");
+                ERROR("Could not modify user - user already member of groups?\n");
                 break;
 
             default:
diff --git a/server/tools/tools_util.c b/server/tools/tools_util.c
index 77721721..bc6b76f4 100644
--- a/server/tools/tools_util.c
+++ b/server/tools/tools_util.c
@@ -191,6 +191,37 @@ int parse_name_domain(struct tools_ctx *tctx,
     return EOK;
 }
 
+int check_group_names(struct tools_ctx *tctx,
+                      char **grouplist,
+                      char **badgroup)
+{
+    int ret;
+    int i;
+    struct ops_ctx *groupinfo;
+
+    groupinfo = talloc_zero(tctx, struct ops_ctx);
+    if (!groupinfo) {
+        return ENOMEM;
+    }
+
+    for (i=0; grouplist[i]; ++i) {
+        ret = sysdb_getgrnam_sync(tctx,
+                                  tctx->ev,
+                                  tctx->sysdb,
+                                  grouplist[i],
+                                  tctx->local,
+                                  &groupinfo);
+        if (ret) {
+            DEBUG(6, ("Cannot find group %s, ret: %d\n", grouplist[i], ret));
+            break;
+        }
+    }
+
+    talloc_zfree(groupinfo);
+    *badgroup = grouplist[i];
+    return ret;
+}
+
 int id_in_range(uint32_t id,
                 struct sss_domain_info *dom)
 {
diff --git a/server/tools/tools_util.h b/server/tools/tools_util.h
index c062dece..a56fc5eb 100644
--- a/server/tools/tools_util.h
+++ b/server/tools/tools_util.h
@@ -72,4 +72,8 @@ int parse_groups(TALLOC_CTX *mem_ctx,
 int parse_group_name_domain(struct tools_ctx *tctx,
                             char **groups);
 
+int check_group_names(struct tools_ctx *tctx,
+                      char **grouplist,
+                      char **badgroup);
+
 #endif  /* __TOOLS_UTIL_H__ */