sysdb: convert sysdb_enumpwent

4 files changed, 144 insertions, 259 deletions

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index fa82e5797..7d4b25819 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -264,8 +264,7 @@ int sysdb_getpwuid(TALLOC_CTX *mem_ctx,
 int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
                     struct sysdb_ctx *ctx,
                     struct sss_domain_info *domain,
-                    const char *expression,
-                    sysdb_callback_t fn, void *ptr);
+                    struct ldb_result **res);
 
 int sysdb_getgrnam(TALLOC_CTX *mem_ctx,
                    struct sysdb_ctx *ctx,
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index 8d8a0e11a..2ac794987 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -176,41 +176,6 @@ static int get_gen_callback(struct ldb_request *req,
 
 /* users */
 
-static void user_search(struct tevent_req *treq)
-{
-    struct sysdb_search_ctx *sctx;
-    struct ldb_request *req;
-    struct ldb_dn *base_dn;
-    int ret;
-
-    sctx = tevent_req_callback_data(treq, struct sysdb_search_ctx);
-
-    ret = sysdb_operation_recv(treq, sctx, &sctx->handle);
-    if (ret) {
-        return request_error(sctx, ret);
-    }
-
-    base_dn = ldb_dn_new_fmt(sctx, sctx->ctx->ldb,
-                             SYSDB_TMPL_USER_BASE, sctx->domain->name);
-    if (!base_dn) {
-        return request_error(sctx, ENOMEM);
-    }
-
-    ret = ldb_build_search_req(&req, sctx->ctx->ldb, sctx,
-                               base_dn, LDB_SCOPE_SUBTREE,
-                               sctx->expression, sctx->attrs, NULL,
-                               sctx, get_gen_callback,
-                               NULL);
-    if (ret != LDB_SUCCESS) {
-        return request_ldberror(sctx, ret);
-    }
-
-    ret = ldb_request(sctx->ctx->ldb, req);
-    if (ret != LDB_SUCCESS) {
-        return request_ldberror(sctx, ret);
-    }
-}
-
 int sysdb_getpwnam(TALLOC_CTX *mem_ctx,
                    struct sysdb_ctx *ctx,
                    struct sss_domain_info *domain,
@@ -299,38 +264,42 @@ done:
 int sysdb_enumpwent(TALLOC_CTX *mem_ctx,
                     struct sysdb_ctx *ctx,
                     struct sss_domain_info *domain,
-                    const char *expression,
-                    sysdb_callback_t fn, void *ptr)
+                    struct ldb_result **_res)
 {
+    TALLOC_CTX *tmpctx;
     static const char *attrs[] = SYSDB_PW_ATTRS;
-    struct sysdb_search_ctx *sctx;
-    struct tevent_req *req;
+    struct ldb_dn *base_dn;
+    struct ldb_result *res;
+    int ret;
 
     if (!domain) {
         return EINVAL;
     }
 
-    sctx = init_src_ctx(mem_ctx, domain, ctx, fn, ptr);
-    if (!sctx) {
+    tmpctx = talloc_new(mem_ctx);
+    if (!tmpctx) {
         return ENOMEM;
     }
 
-    if (expression)
-        sctx->expression = expression;
-    else
-        sctx->expression = SYSDB_PWENT_FILTER;
-
-    sctx->attrs = attrs;
+    base_dn = ldb_dn_new_fmt(tmpctx, ctx->ldb,
+                             SYSDB_TMPL_USER_BASE, domain->name);
+    if (!base_dn) {
+        ret = ENOMEM;
+        goto done;
+    }
 
-    req = sysdb_operation_send(mem_ctx, ctx->ev, ctx);
-    if (!req) {
-        talloc_free(sctx);
-        return ENOMEM;
+    ret = ldb_search(ctx->ldb, tmpctx, &res, base_dn,
+                     LDB_SCOPE_SUBTREE, attrs, SYSDB_PWENT_FILTER);
+    if (ret) {
+        ret = sysdb_error_to_errno(ret);
+        goto done;
     }
 
-    tevent_req_set_callback(req, user_search, sctx);
+    *_res = talloc_steal(mem_ctx, res);
 
-    return EOK;
+done:
+    talloc_zfree(tmpctx);
+    return ret;
 }
 
 /* groups */
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 508f33459..32ee29881 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -927,104 +927,116 @@ done:
  * - use mutexes so that setpwent() can return immediately
  *   even if the data is still being fetched
  * - make getpwent() wait on the mutex
+ *
+ * Alternatively:
+ * - use a smarter search mechanism that keeps track of the
+ *   last user searched and return the next X users doing
+ *   an alphabetic sort and starting from the user following
+ *   the last returned user.
  */
 static int nss_cmd_getpwent_immediate(struct nss_cmd_ctx *cmdctx);
 
-static void nss_cmd_setpw_dp_callback(uint16_t err_maj, uint32_t err_min,
-                                      const char *err_msg, void *ptr);
+static void nss_cmd_getpwent_dp_callback(uint16_t err_maj, uint32_t err_min,
+                                         const char *err_msg, void *ptr);
 
-static void nss_cmd_setpwent_callback(void *ptr, int status,
-                                      struct ldb_result *res)
+static int nss_cmd_getpwent_search(struct nss_dom_ctx *dctx)
 {
-    struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx);
     struct nss_cmd_ctx *cmdctx = dctx->cmdctx;
+    struct sss_domain_info *dom = dctx->domain;
     struct cli_ctx *cctx = cmdctx->cctx;
-    struct sss_domain_info *dom;
     struct sysdb_ctx *sysdb;
+    struct ldb_result *res;
     struct getent_ctx *pctx;
     struct nss_ctx *nctx;
     int timeout;
     int ret;
 
-    if (status != LDB_SUCCESS) {
-        ret = nss_cmd_send_error(cmdctx, ENOENT);
-        if (ret != EOK) {
-            NSS_CMD_FATAL_ERROR(cctx);
-        }
-        sss_cmd_done(cctx, cmdctx);
-        return;
-    }
-
     nctx = talloc_get_type(cctx->rctx->pvt_ctx, struct nss_ctx);
     pctx = nctx->pctx;
     if (pctx == NULL) {
         pctx = talloc_zero(nctx, struct getent_ctx);
         if (!pctx) {
-            ret = nss_cmd_send_error(cmdctx, ENOMEM);
-            if (ret != EOK) {
-                NSS_CMD_FATAL_ERROR(cctx);
-            }
-            sss_cmd_done(cctx, cmdctx);
-            return;
+            return ENOMEM;
         }
         nctx->pctx = pctx;
     }
 
-    pctx->doms = talloc_realloc(pctx, pctx->doms, struct dom_ctx, pctx->num +1);
-    if (!pctx->doms) {
-        talloc_free(pctx);
-        nctx->pctx = NULL;
-        NSS_CMD_FATAL_ERROR(cctx);
-    }
+    while (dom) {
+        while (dom && dom->enumerate == 0) {
+            dom = dom->next;
+        }
 
-    pctx->doms[pctx->num].domain = dctx->domain;
-    pctx->doms[pctx->num].res = talloc_steal(pctx->doms, res);
-    pctx->doms[pctx->num].cur = 0;
+        if (!dom) break;
 
-    pctx->num++;
+        if (dom != dctx->domain) {
+            /* make sure we reset the check_provider flag when we check
+             * a new domain */
+            if (cmdctx->enum_cached) {
+                dctx->check_provider = false;
+            } else {
+                dctx->check_provider = NEED_CHECK_PROVIDER(dom->provider);
+            }
+        }
 
-    /* do not reply until all domain searches are done */
-    for (dom = dctx->domain->next; dom; dom = dom->next) {
-        if (dom->enumerate != 0) break;
-    }
-    dctx->domain = dom;
+        /* make sure to update the dctx if we changed domain */
+        dctx->domain = dom;
 
-    if (dctx->domain != NULL) {
-        if (cmdctx->enum_cached) {
-            dctx->check_provider = false;
-        } else {
-            dctx->check_provider = NEED_CHECK_PROVIDER(dctx->domain->provider);
+        DEBUG(4, ("Requesting info for domain [%s]\n", dom->name));
+
+        ret = sysdb_get_ctx_from_list(cctx->rctx->db_list, dom, &sysdb);
+        if (ret != EOK) {
+            DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n"));
+            return EIO;
         }
 
+        /* if this is a caching provider (or if we haven't checked the cache
+         * yet) then verify that the cache is uptodate */
         if (dctx->check_provider) {
+            dctx->check_provider = false;
             timeout = SSS_CLI_SOCKET_TIMEOUT;
             ret = sss_dp_send_acct_req(cctx->rctx, cmdctx,
-                                       nss_cmd_setpw_dp_callback, dctx,
+                                       nss_cmd_getpwent_dp_callback, dctx,
                                        timeout, dom->name, true,
                                        SSS_DP_USER, NULL, 0);
-        } else {
-            ret = sysdb_get_ctx_from_list(cctx->rctx->db_list,
-                                          dctx->domain, &sysdb);
-            if (ret != EOK) {
-                DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n"));
-                NSS_CMD_FATAL_ERROR(cctx);
+            if (ret == EOK) {
+                return ret;
+            } else {
+                DEBUG(2, ("Enum Cache refresh for domain [%s] failed."
+                          " Trying to return what we have in cache!\n",
+                          dom->name));
             }
-            ret = sysdb_enumpwent(dctx, sysdb,
-                                  dctx->domain, NULL,
-                                  nss_cmd_setpwent_callback, dctx);
         }
+
+        ret = sysdb_enumpwent(dctx, sysdb, dctx->domain, &res);
         if (ret != EOK) {
-            /* FIXME: shutdown ? */
-            DEBUG(1, ("Failed to send enumeration request for domain [%s]!\n",
+            DEBUG(1, ("Enum from cache failed, skipping domain [%s]\n",
                       dom->name));
+            dom = dom->next;
+            continue;
+        }
 
-            ret = nss_cmd_send_error(cmdctx, ret);
-            if (ret != EOK) {
-                NSS_CMD_FATAL_ERROR(cctx);
-            }
-            sss_cmd_done(cctx, cmdctx);
+        if (res->count == 0) {
+            DEBUG(4, ("Domain [%s] has no users, skipping.\n", dom->name));
+            dom = dom->next;
+            continue;
         }
-        return;
+
+        pctx->doms = talloc_realloc(pctx, pctx->doms,
+                                    struct dom_ctx, pctx->num +1);
+        if (!pctx->doms) {
+            talloc_free(pctx);
+            nctx->pctx = NULL;
+            return ENOMEM;
+        }
+
+        pctx->doms[pctx->num].domain = dctx->domain;
+        pctx->doms[pctx->num].res = talloc_steal(pctx->doms, res);
+        pctx->doms[pctx->num].cur = 0;
+
+        pctx->num++;
+
+        /* do not reply until all domain searches are done */
+        dom = dom->next;
     }
 
     /* set cache mark */
@@ -1033,28 +1045,25 @@ static void nss_cmd_setpwent_callback(void *ptr, int status,
     if (cmdctx->immediate) {
         /* this was a getpwent call w/o setpwent,
          * return immediately one result */
-        ret = nss_cmd_getpwent_immediate(cmdctx);
-        if (ret != EOK) NSS_CMD_FATAL_ERROR(cctx);
-        return;
+        return nss_cmd_getpwent_immediate(cmdctx);
     }
 
     /* create response packet */
     ret = sss_packet_new(cctx->creq, 0,
                          sss_packet_get_cmd(cctx->creq->in),
                          &cctx->creq->out);
-    if (ret != EOK) {
-        NSS_CMD_FATAL_ERROR(cctx);
+    if (ret == EOK) {
+        sss_cmd_done(cctx, cmdctx);
     }
-    sss_cmd_done(cctx, cmdctx);
+    return ret;
 }
 
-static void nss_cmd_setpw_dp_callback(uint16_t err_maj, uint32_t err_min,
-                                      const char *err_msg, void *ptr)
+static void nss_cmd_getpwent_dp_callback(uint16_t err_maj, uint32_t err_min,
+                                         const char *err_msg, void *ptr)
 {
     struct nss_dom_ctx *dctx = talloc_get_type(ptr, struct nss_dom_ctx);
     struct nss_cmd_ctx *cmdctx = dctx->cmdctx;
     struct cli_ctx *cctx = cmdctx->cctx;
-    struct sysdb_ctx *sysdb;
     int ret;
 
     if (err_maj) {
@@ -1064,37 +1073,20 @@ static void nss_cmd_setpw_dp_callback(uint16_t err_maj, uint32_t err_min,
                   (unsigned int)err_maj, (unsigned int)err_min, err_msg));
     }
 
-    ret = sysdb_get_ctx_from_list(cctx->rctx->db_list,
-                                  dctx->domain, &sysdb);
-    if (ret != EOK) {
-        DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n"));
-        NSS_CMD_FATAL_ERROR(cctx);
-    }
-    ret = sysdb_enumpwent(cmdctx, sysdb,
-                          dctx->domain, NULL,
-                          nss_cmd_setpwent_callback, dctx);
-    if (ret != EOK) {
-        DEBUG(1, ("Failed to make request to our cache!\n"));
+    ret = nss_cmd_getpwent_search(dctx);
 
-        ret = nss_cmd_send_error(cmdctx, ret);
-        if (ret != EOK) {
-            NSS_CMD_FATAL_ERROR(cctx);
-        }
-        sss_cmd_done(cctx, cmdctx);
+    if (ret) {
+        NSS_CMD_FATAL_ERROR(cctx);
     }
 }
 
 static int nss_cmd_setpwent_ext(struct cli_ctx *cctx, bool immediate)
 {
     struct sss_domain_info *dom;
-    struct sysdb_ctx *sysdb;
     struct nss_cmd_ctx *cmdctx;
     struct nss_dom_ctx *dctx;
     struct nss_ctx *nctx;
     time_t now = time(NULL);
-    int timeout;
-    uint8_t *body;
-    size_t blen;
     int ret;
 
     DEBUG(4, ("Requesting info for all users\n"));
@@ -1119,8 +1111,7 @@ static int nss_cmd_setpwent_ext(struct cli_ctx *cctx, bool immediate)
 
     /* do not query backends if we have a recent enumeration */
     if (nctx->enum_cache_timeout) {
-        if (nctx->last_user_enum +
-            nctx->enum_cache_timeout > now) {
+        if (nctx->last_user_enum + nctx->enum_cache_timeout > now) {
             cmdctx->enum_cached = true;
         }
     }
@@ -1133,71 +1124,25 @@ static int nss_cmd_setpwent_ext(struct cli_ctx *cctx, bool immediate)
 
     if (dctx->domain == NULL) {
         DEBUG(2, ("Enumeration disabled on all domains!\n"));
-        ret = ENOENT;
+        if (cmdctx->immediate) {
+            ret = ENOENT;
+        } else {
+            ret = sss_packet_new(cctx->creq, 0,
+                                 sss_packet_get_cmd(cctx->creq->in),
+                                 &cctx->creq->out);
+            if (ret == EOK) {
+                sss_cmd_done(cctx, cmdctx);
+            }
+        }
         goto done;
     }
 
-    if (cmdctx->enum_cached) {
-        dctx->check_provider = false;
-    } else {
-        dctx->check_provider = NEED_CHECK_PROVIDER(dctx->domain->provider);
-    }
-
-    if (dctx->check_provider) {
-        timeout = SSS_CLI_SOCKET_TIMEOUT;
-        ret = sss_dp_send_acct_req(cctx->rctx, cmdctx,
-                                   nss_cmd_setpw_dp_callback, dctx,
-                                   timeout, dom->name, true,
-                                   SSS_DP_USER, NULL, 0);
-    } else {
-        ret = sysdb_get_ctx_from_list(cctx->rctx->db_list,
-                                      dctx->domain, &sysdb);
-        if (ret != EOK) {
-            DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n"));
-            ret = EFAULT;
-            goto done;
-        }
-        ret = sysdb_enumpwent(dctx, sysdb,
-                              dctx->domain, NULL,
-                              nss_cmd_setpwent_callback, dctx);
-    }
-    if (ret != EOK) {
-        /* FIXME: shutdown ? */
-        DEBUG(1, ("Failed to send enumeration request for domain [%s]!\n",
-                  dom->name));
-    }
+    dctx->check_provider = NEED_CHECK_PROVIDER(dctx->domain->provider);
 
+    /* ok, start the searches */
+    ret = nss_cmd_getpwent_search(dctx);
 done:
-    if (ret != EOK) {
-        if (ret == ENOENT) {
-            if (cmdctx->immediate) {
-                /* we do not have any entry to return */
-                ret = sss_packet_new(cctx->creq, 2*sizeof(uint32_t),
-                                     sss_packet_get_cmd(cctx->creq->in),
-                                     &cctx->creq->out);
-                if (ret == EOK) {
-                    sss_packet_get_body(cctx->creq->out, &body, &blen);
-                    ((uint32_t *)body)[0] = 0; /* 0 results */
-                    ((uint32_t *)body)[1] = 0; /* reserved */
-                }
-            }
-            else {
-                /* create response packet */
-                ret = sss_packet_new(cctx->creq, 0,
-                                     sss_packet_get_cmd(cctx->creq->in),
-                                     &cctx->creq->out);
-            }
-        }
-        if (ret != EOK) {
-            ret = nss_cmd_send_error(cmdctx, ret);
-        }
-        if (ret == EOK) {
-            sss_cmd_done(cctx, cmdctx);
-        }
-        return ret;
-    }
-
-    return EOK;
+    return nss_cmd_done(cmdctx, ret);
 }
 
 static int nss_cmd_setpwent(struct cli_ctx *cctx)
@@ -1213,40 +1158,42 @@ static int nss_cmd_retpwent(struct cli_ctx *cctx, int num)
     struct ldb_message **msgs = NULL;
     struct dom_ctx *pdom = NULL;
     int n = 0;
-    int ret;
+    int ret = ENOENT;
 
     nctx = talloc_get_type(cctx->rctx->pvt_ctx, struct nss_ctx);
-    pctx = nctx->pctx;
+    if (!nctx->pctx) goto none;
 
-retry:
-    if (pctx->cur >= pctx->num) goto none;
+    pctx = nctx->pctx;
 
-    pdom = &pctx->doms[pctx->cur];
+    while (ret == ENOENT) {
+        if (pctx->cur >= pctx->num) break;
 
-    n = pdom->res->count - pdom->cur;
-    if (n == 0 && (pctx->cur+1 < pctx->num)) {
-        pctx->cur++;
         pdom = &pctx->doms[pctx->cur];
+
         n = pdom->res->count - pdom->cur;
-    }
+        if (n == 0 && (pctx->cur+1 < pctx->num)) {
+            pctx->cur++;
+            pdom = &pctx->doms[pctx->cur];
+            n = pdom->res->count - pdom->cur;
+        }
 
-    if (!n) goto none;
+        if (!n) break;
 
-    if (n > num) n = num;
+        if (n > num) n = num;
 
-    msgs = &(pdom->res->msgs[pdom->cur]);
-    pdom->cur += n;
+        msgs = &(pdom->res->msgs[pdom->cur]);
+        pdom->cur += n;
 
-    ret = fill_pwent(cctx->creq->out, pdom->domain, nctx, true, msgs, n);
-    if (ret == ENOENT) goto retry;
-    return ret;
+        ret = fill_pwent(cctx->creq->out, pdom->domain, nctx, true, msgs, n);
+    }
 
 none:
-    return fill_empty(cctx->creq->out);
+    if (ret == ENOENT) {
+        ret = fill_empty(cctx->creq->out);
+    }
+    return ret;
 }
 
-/* used only if a process calls getpwent() without first calling setpwent()
- */
 static int nss_cmd_getpwent_immediate(struct nss_cmd_ctx *cmdctx)
 {
     struct cli_ctx *cctx = cmdctx->cctx;
@@ -1289,9 +1236,6 @@ static int nss_cmd_getpwent(struct cli_ctx *cctx)
 
     /* see if we need to trigger an implicit setpwent() */
     if (nctx->pctx == NULL) {
-        nctx->pctx = talloc_zero(nctx, struct getent_ctx);
-        if (!nctx->pctx) return ENOMEM;
-
         return nss_cmd_setpwent_ext(cctx, true);
     }
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 62ad6d160..bfd640bcd 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -325,26 +325,6 @@ static void test_enumgrent(void *pvt, int error, struct ldb_result *res)
     data->error = EOK;
 }
 
-static void test_enumpwent(void *pvt, int error, struct ldb_result *res)
-{
-    struct test_data *data = talloc_get_type(pvt, struct test_data);
-    const int expected = 10;
-
-    data->finished = true;
-
-    if (error != EOK) {
-        data->error = error;
-        return;
-    }
-
-    if (res->count != expected) {
-        data->error = EINVAL;
-        return;
-    }
-
-    data->error = EOK;
-}
-
 static int test_set_user_attr(struct test_data *data)
 {
     int ret;
@@ -943,7 +923,7 @@ END_TEST
 START_TEST (test_sysdb_enumpwent)
 {
     struct sysdb_test_ctx *test_ctx;
-    struct test_data *data;
+    struct ldb_result *res;
     int ret;
 
     /* Setup */
@@ -953,23 +933,16 @@ START_TEST (test_sysdb_enumpwent)
         return;
     }
 
-    data = talloc_zero(test_ctx, struct test_data);
-    data->ctx = test_ctx;
-
     ret = sysdb_enumpwent(test_ctx,
                           test_ctx->sysdb,
-                          data->ctx->domain,
-                          NULL,
-                          test_enumpwent,
-                          data);
-    if (ret == EOK) {
-        ret = test_loop(data);
-    }
-
+                          test_ctx->domain,
+                          &res);
     fail_unless(ret == EOK,
                 "sysdb_enumpwent failed (%d: %s)",
                 ret, strerror(ret));
 
+    fail_if(res->count != 10, "Expected 10 users, got %d", res->count);
+
     talloc_free(test_ctx);
 }
 END_TEST