>From 1cb395244d3ab3906af5b6c8b294e80055ad0a07 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <preichl@redhat.com>
Date: Tue, 9 Dec 2014 11:01:13 +0000
Subject: [PATCH 2/2] SYSDB: sysdb_search_object_by_sid returns ENOENT

sysdb_search_object_by_sid returns ENOENT if no results are found.

Part od solution for:
https://fedorahosted.org/sssd/ticket/1991
---
 src/db/sysdb.h                 |  2 +-
 src/db/sysdb_ops.c             | 70 ++++++------------------------------------
 src/responder/nss/nsssrv_cmd.c | 27 ++++++++--------
 src/responder/pac/pacsrv_cmd.c | 26 ++++++++++------
 src/tests/sysdb-tests.c        |  5 +--
 5 files changed, 42 insertions(+), 88 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 5bd7f90acb685bbaff5c98f433c7dce8175c33ca..9b88b4a63619456f8f3cc1961e29bbf3946ca5b8 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -1033,7 +1033,7 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
                                    struct sss_domain_info *domain,
                                    const char *sid_str,
                                    const char **attrs,
-                                   struct ldb_result **msg);
+                                   struct ldb_result **res);
 
 errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx,
                                     struct sss_domain_info *domain,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 998046a2ca1c746b2032f430e5f9c4a7151e1dbc..8d6b11c248fd7f895ff6a95c25d4372fc7f8445d 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2995,8 +2995,15 @@ int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
 
     ret = sysdb_search_object_by_sid(tmp_ctx, domain, sid_str, NULL, &res);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "search by sid failed: %d (%s)\n",
-              ret, strerror(ret));
+        if (ret == ENOENT) {
+            /* No existing entry. Just quit. */
+            DEBUG(SSSDBG_TRACE_FUNC,
+                  "search by sid did not return any results.\n");
+            ret = EOK;
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "search by sid failed: %d (%s)\n",
+                  ret, strerror(ret));
+        }
         goto done;
     }
 
@@ -3007,12 +3014,6 @@ int sysdb_delete_by_sid(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    if (res->count == 0) {
-        /* No existing entry. Just quit. */
-        ret = EOK;
-        goto done;
-    }
-
     ret = sysdb_delete_entry(sysdb, res->msgs[0]->dn, false);
     if (ret != EOK) {
         goto done;
@@ -3564,61 +3565,10 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx,
                                    struct sss_domain_info *domain,
                                    const char *sid_str,
                                    const char **attrs,
-                                   struct ldb_result **msg)
+                                   struct ldb_result **res)
 {
-/* TODO: use
     return sysdb_search_object_by_str_attr(mem_ctx, domain, SYSDB_SID_FILTER,
                                            sid_str, attrs, res);
-
-    when verified that all callers can handle ENOENT correctly. */
-
-    TALLOC_CTX *tmp_ctx;
-    const char *def_attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, SYSDB_GIDNUM,
-                                ORIGINALAD_PREFIX SYSDB_NAME,
-                                SYSDB_OBJECTCLASS, NULL };
-    struct ldb_dn *basedn;
-    int ret;
-    struct ldb_result *res = NULL;
-
-    tmp_ctx = talloc_new(NULL);
-    if (!tmp_ctx) {
-        return ENOMEM;
-    }
-
-    basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb, SYSDB_DOM_BASE, domain->name);
-    if (basedn == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new_fmt failed.\n");
-        ret = ENOMEM;
-        goto done;
-    }
-
-    ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res,
-                     basedn, LDB_SCOPE_SUBTREE, attrs?attrs:def_attrs,
-                     SYSDB_SID_FILTER, sid_str);
-    if (ret != EOK) {
-        ret = sysdb_error_to_errno(ret);
-        DEBUG(SSSDBG_OP_FAILURE, "ldb_search failed.\n");
-        goto done;
-    }
-
-    if (res->count > 1) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Search for SID [%s] returned more than " \
-                                    "one object.\n", sid_str);
-        ret = EINVAL;
-        goto done;
-    }
-
-    *msg = talloc_steal(mem_ctx, res);
-
-done:
-    if (ret == ENOENT) {
-        DEBUG(SSSDBG_TRACE_FUNC, "No such entry.\n");
-    } else if (ret) {
-        DEBUG(SSSDBG_OP_FAILURE, "Error: %d (%s)\n", ret, strerror(ret));
-    }
-
-    talloc_zfree(tmp_ctx);
-    return ret;
 }
 
 errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx,
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index ea58920bc3611c84958e8d0aca0e122d90c68e5c..d81737b6182bf283f61655c59dcffa135a1244d7 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4492,6 +4492,20 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
     ret = sysdb_search_object_by_sid(cmdctx, dom, cmdctx->secid, NULL,
                                      &dctx->res);
     if (ret != EOK) {
+        if (ret == ENOENT) {
+            if (!dctx->check_provider) {
+                DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n");
+
+                /* set negative cache only if not result of cache check */
+                ret = sss_ncache_set_sid(nctx->ncache, false, cmdctx->secid);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_MINOR_FAILURE,
+                          "Cannot set negative cache for %s\n", cmdctx->secid);
+                }
+            }
+            return ENOENT;
+        }
+
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n");
         return EIO;
     }
@@ -4502,19 +4516,6 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx)
         return ENOENT;
     }
 
-    if (dctx->res->count == 0 && !dctx->check_provider) {
-        DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n");
-
-        /* set negative cache only if not result of cache check */
-        ret = sss_ncache_set_sid(nctx->ncache, false, cmdctx->secid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  "Cannot set negative cache for %s\n", cmdctx->secid);
-        }
-
-        return ENOENT;
-    }
-
     /* 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) {
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index cc92592893899b1fa269188facc1a8154f80991d..5cedea14747b578dd7896d4d9a80c2e81fc04c17 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -298,16 +298,17 @@ static void pac_lookup_sids_done(struct tevent_req *req)
             ret = sysdb_search_object_by_sid(pr_ctx, dom, entries[c].key.str,
                                              NULL, &msg);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \
-                                          "failed.\n");
+                if (ret == ENOENT) {
+                    DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
+                          entries[c].key.str);
+                } else {
+                    DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \
+                          "failed.\n");
+                }
                 continue;
             }
 
-            if (msg->count == 0) {
-                DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n",
-                                          entries[c].key.str);
-                continue;
-            } else if (msg->count > 1) {
+            if (msg->count > 1) {
                 DEBUG(SSSDBG_CRIT_FAILURE, "More then one result returned " \
                                             "for SID [%s].\n",
                                             entries[c].key.str);
@@ -912,9 +913,14 @@ pac_store_membership(struct pac_req_ctx *pr_ctx,
     ret = sysdb_search_object_by_sid(tmp_ctx, grp_dom, grp_sid_str,
                                      group_attrs, &group);
     if (ret != EOK) {
-        DEBUG(SSSDBG_TRACE_INTERNAL, "sysdb_search_object_by_sid " \
-                                      "for SID [%s] failed [%d][%s].\n",
-                                      grp_sid_str, ret, strerror(ret));
+        if (ret == ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "Unexpected number of groups returned.\n");
+            ret = EINVAL;
+        } else {
+            DEBUG(SSSDBG_TRACE_INTERNAL, "sysdb_search_object_by_sid "  \
+                  "for SID [%s] failed [%d][%s].\n",
+                  grp_sid_str, ret, strerror(ret));
+        }
         goto done;
     }
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index d303982647547eefdce7c37ac5b70e1ffbe869ce..92b41e90d08c2d50775289ba8c922f39350ce625 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -4861,13 +4861,10 @@ START_TEST (test_sysdb_search_return_ENOENT)
     talloc_zfree(res);
 
     /* Search object */
-    /* TODO: Should return ENOENT */
     ret = sysdb_search_object_by_sid(test_ctx, test_ctx->domain,
                                      "S-5-4-3-2-1", NULL, &res);
-    fail_unless(ret == EOK, "sysdb_search_object_by_sid_str failed with "
+    fail_unless(ret == ENOENT, "sysdb_search_object_by_sid_str failed with "
                              "[%d][%s].", ret, strerror(ret));
-    fail_unless(res->count == 0, "sysdb_search_object_by_sid_str should not "
-                                 "return anything.");
     talloc_zfree(res);
 
     /* Search can return more results */
-- 
1.9.3

