[SSSD] [PATCH] NSS: Handle ENOENT when doing initgroups by UPN

Jakub Hrozek jhrozek at redhat.com
Wed Mar 11 17:07:28 UTC 2015


On Wed, Mar 11, 2015 at 06:02:01PM +0100, Pavel Reichl wrote:
> 
> 
> On 03/11/2015 05:04 PM, Lukas Slebodnik wrote:
> >On (11/03/15 16:48), Jakub Hrozek wrote:
> >>On Wed, Mar 11, 2015 at 04:30:52PM +0100, Pavel Reichl wrote:
> >>>On 03/11/2015 03:46 PM, Jakub Hrozek wrote:
> >>>
> >>>
> >>>>@@ -4062,29 +4062,39 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
> >>>>          if (cmdctx->name_is_upn) {
> >>>>              ret = sysdb_search_user_by_upn(cmdctx, dom, name, user_attrs, &msg);
> >>>>-            if (ret != EOK && ret != ENOENT) {
> >>>>+            if (ret == EOK) {
> >>>>+                sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
> >>>>+                if (sysdb_name == NULL) {
> >>>>+                    DEBUG(SSSDBG_OP_FAILURE,
> >>>>+                        "Sysdb entry does not have a name.\n");
> >>>>+                    return EINVAL;
> >>>>+                }
> >>>>+
> >>>>+                ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
> >>>>+                if (ret == EOK && DOM_HAS_VIEWS(dom)) {
> >>>>+                    for (c = 0; c < dctx->res->count; c++) {
> >>>>+                        ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
> >>>>+                                                            NULL, NULL);
> >>>>+                        if (ret != EOK) {
> >>>>+                            DEBUG(SSSDBG_OP_FAILURE,
> >>>>+                                "sysdb_add_overrides_to_object failed.\n");
> >>>>+                            return ret;
> >>>>+                        }
> >>>>+                    }
> >>>>+                }
> >>>>+            } else if (ret != ENOENT) {
> >>>>                  DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn failed.\n");
> >>>>                  return ret;
> >>>>-            }
> >>>>-
> >>>>-            sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
> >>>>-            if (sysdb_name == NULL) {
> >>>>-                DEBUG(SSSDBG_OP_FAILURE,
> >>>>-                      "Sysdb entry does not have a name.\n");
> >>>>-                return EINVAL;
> >>>>-            }
> >>>>-
> >>>>-            ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
> >>>>-            if (ret == EOK && DOM_HAS_VIEWS(dom)) {
> >>>>-                for (c = 0; c < dctx->res->count; c++) {
> >>>>-                    ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
> >>>>-                                                        NULL, NULL);
> >>>>-                    if (ret != EOK) {
> >>>>-                        DEBUG(SSSDBG_OP_FAILURE,
> >>>>-                              "sysdb_add_overrides_to_object failed.\n");
> >>>>-                        return ret;
> >>>>-                    }
> >>>>+            } else {
> >>>>+                dctx->res = talloc_zero(cmdctx, struct ldb_result);
> >>>>+                if (dctx->res == NULL) {
> >>>>+                    DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
> >>>>+                    return ENOMEM;
> >>>>                  }
> >>>>+
> >>>>+                dctx->res->count = 0;
> >>>>+                dctx->res->msgs = NULL;
> >>>>+                ret = EOK;
> >>>>              }
> >>>>          } else {
> >>>>              ret = sysdb_initgroups_with_views(cmdctx, dom, name, &dctx->res);
> >>>>-- 2.1.0
> >>>Thanks for the patches.
> >>>All patches LGTM.
> >>>
> >>>I've tested that tests fail if the forth patch is not applied. I also wrote
> >>>simple program to reproduce the segfault and after applying the 4th patch
> >>>I'm no longer able to reproduce the bug.
> >>>
> >>>I have only one style nitpick - would it make more sense to test explicitly
> >>>for 'ret == ENOENT' and handle the other cases in else branch?
> >>>
> >>>If you don't agree then ignore then comment and push the patches (ACK to
> >>>all).
> >>>
> >>>Thanks!
> >>>
> >>>ci passed:
> >>>
> >>>http://sssd-ci.duckdns.org/logs/job/9/46/summary.html
> >>I'm not sure if I understood what you meant, but check the attached
> >>patches.
> >>
> >>Normally I prefer:
> >>    if (ret == ENOENT) {
> >>        /* handle ENOENT */
> >>    } else if (ret != EOK) {
> >>        /* Error! *//
> >>    }
> >>
> >>    /* Let's not put EOK into else and avoid extra indentation */
> >>
> >>But here the EOK must be in an else branch, I think.
> 
> I think this version looks a little better, but I'm fine with both version.
> So ACK from me.
> 
> 
> >>From c0daf41e1c9ca1e8331dc9e5e7098e7862fd8b7e Mon Sep 17 00:00:00 2001
> >>From: Jakub Hrozek <jhrozek at redhat.com>
> >>Date: Mon, 9 Mar 2015 16:59:39 +0100
> >>Subject: [PATCH 3/4] Add unit tests for initgroups
> >>
> >This patch should be applied as the last one (and not 3rd)
> >otherwise the test nss-srv-tests fails and our CI would complain.
> How would CI complain? It seems OK to me:
> 
> http://sssd-ci.duckdns.org/logs/job/9/47/summary.html

I think that's because the CI that runs post-commit runs the 'rigorous'
set of tests which applies the commits one by one. Developers usually
run the 'essential' set of tests.

The patch order was actually intentional, I was first trying to show the
bug and fix in the later patch. But I didn't realize the CI point. I
think Lukas is right.

Attached is the patchset I'd like to push.
-------------- next part --------------
>From c14978d23957b57b33a223ebc3f103151c0cd39f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 17:26:54 +0100
Subject: [PATCH 1/4] tests: ncache_hit must be an int to test UPNs

In order to detect faulty cases where negcache would be checked twice,
we need to convert the ncache_hit to integer and check exact amounts of
hits.
---
 src/tests/cmocka/test_nss_srv.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 4dbda3bd605a9559658cec58009c762c77dd1b1a..bf8bddb263f1f0b8eae1293c021fa958f329f1e0 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -48,7 +48,7 @@ struct nss_test_ctx {
     struct sss_cmd_table *nss_cmds;
     struct nss_ctx *nctx;
 
-    bool ncache_hit;
+    int ncache_hits;
 };
 
 const char *global_extra_attrs[] = {"phone", "mobile", NULL};
@@ -159,7 +159,7 @@ int __wrap_sss_ncache_check_user(struct sss_nc_ctx *ctx, int ttl,
 
     ret = __real_sss_ncache_check_user(ctx, ttl, dom, name);
     if (ret == EEXIST) {
-        nss_test_ctx->ncache_hit = true;
+        nss_test_ctx->ncache_hits++;
     }
     return ret;
 }
@@ -172,7 +172,7 @@ int __wrap_sss_ncache_check_uid(struct sss_nc_ctx *ctx, int ttl, uid_t uid)
 
     ret = __real_sss_ncache_check_uid(ctx, ttl, uid);
     if (ret == EEXIST) {
-        nss_test_ctx->ncache_hit = true;
+        nss_test_ctx->ncache_hits++;
     }
     return ret;
 }
@@ -347,7 +347,7 @@ void test_nss_getpwnam_neg(void **state)
     mock_input_user_or_group("testuser_neg");
     mock_account_recv_simple();
 
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
                           nss_test_ctx->nss_cmds);
@@ -356,7 +356,7 @@ void test_nss_getpwnam_neg(void **state)
     /* Wait until the test finishes with ENOENT */
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     /* Test that subsequent search for a nonexistent user yields
      * ENOENT and Account callback is not called, on the other hand
@@ -373,7 +373,7 @@ void test_nss_getpwnam_neg(void **state)
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
     /* Negative cache was hit this time */
-    assert_true(nss_test_ctx->ncache_hit == true);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
 static int test_nss_getpwnam_search_acct_cb(void *pvt)
@@ -791,7 +791,7 @@ void test_nss_getpwuid_neg(void **state)
     mock_input_id(nss_test_ctx, id);
     mock_account_recv_simple();
 
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID,
                           nss_test_ctx->nss_cmds);
@@ -800,7 +800,7 @@ void test_nss_getpwuid_neg(void **state)
     /* Wait until the test finishes with ENOENT */
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
-    assert_true(nss_test_ctx->ncache_hit == false);
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
 
     /* Test that subsequent search for a nonexistent id yields
      * ENOENT and Account callback is not called, on the other hand
@@ -817,7 +817,7 @@ void test_nss_getpwuid_neg(void **state)
     ret = test_ev_loop(nss_test_ctx->tctx);
     assert_int_equal(ret, ENOENT);
     /* Negative cache was hit this time */
-    assert_true(nss_test_ctx->ncache_hit == true);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
 static int test_nss_getpwuid_search_acct_cb(void *pvt)
-- 
2.1.0

-------------- next part --------------
>From 4cbd5783da8cd4173ac4c7dd33a0a8af023de8bd Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 15:41:56 +0100
Subject: [PATCH 2/4] tests: Add a getpwnam-by-UPN test

---
 src/tests/cmocka/test_nss_srv.c | 97 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index bf8bddb263f1f0b8eae1293c021fa958f329f1e0..2e95bc0f9a92e702a37bc21f029254691a706025 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -2071,6 +2071,99 @@ void test_nss_getorigbyname_multi_value_attrs(void **state)
     assert_int_equal(ret, EOK);
 }
 
+static int test_nss_getpwnam_upn_check(uint32_t status,
+                                       uint8_t *body,
+                                       size_t blen)
+{
+    struct passwd pwd;
+    errno_t ret;
+
+    assert_int_equal(status, EOK);
+
+    ret = parse_user_packet(body, blen, &pwd);
+    assert_int_equal(ret, EOK);
+
+    assert_int_equal(pwd.pw_uid, 34567);
+    assert_int_equal(pwd.pw_gid, 45678);
+    assert_string_equal(pwd.pw_name, "upnuser");
+    assert_string_equal(pwd.pw_shell, "/bin/sh");
+    assert_string_equal(pwd.pw_passwd, "*");
+    return EOK;
+}
+
+void test_nss_getpwnam_upn(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, "upnuser at upndomain.test");
+    assert_int_equal(ret, EOK);
+
+    /* Prime the cache with a valid user */
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "upnuser", 34567, 45678, "up user",
+                         "/home/upnuser", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("upnuser at upndomain.test");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWNAM);
+    mock_fill_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_getpwnam_upn_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_nss_getpwnam_upn_neg(void **state)
+{
+    errno_t ret;
+
+    mock_input_user_or_group("nosuchupnuser at upndomain.test");
+    mock_account_recv_simple();
+
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+
+    /* Test that subsequent search for a nonexistent user yields
+     * ENOENT and Account callback is not called, on the other hand
+     * the ncache functions will be called
+     */
+    nss_test_ctx->tctx->done = false;
+    nss_test_ctx->ncache_hits = 0;
+
+    mock_input_user_or_group("nosuchupnuser at upndomain.test");
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWNAM,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* Negative cache was hit this time */
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+}
+
 static int nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -2239,6 +2332,10 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_nss_getorigbyname_multi_value_attrs,
                                         nss_test_setup_extra_attr,
                                         nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn_neg,
+                                        nss_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.1.0

-------------- next part --------------
>From c473a2c656264aedf2c700f98138128ec1a2ae45 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 17:25:48 +0100
Subject: [PATCH 3/4] NSS: Handle ENOENT when doing initgroups by UPN

https://fedorahosted.org/sssd/ticket/2598

We need to return an empty result in cases an initgroups lookup by UPN
doesn't return anything. Please note testing with "id user" is not
sufficient as id calls a getpwnam first.
---
 src/responder/nss/nsssrv_cmd.c | 46 +++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index f9056590a369b8cea358d12192357bf51599e7b6..4f297c6a32800303f0480dc63b70d3558462d385 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4062,27 +4062,37 @@ static int nss_cmd_initgroups_search(struct nss_dom_ctx *dctx)
 
         if (cmdctx->name_is_upn) {
             ret = sysdb_search_user_by_upn(cmdctx, dom, name, user_attrs, &msg);
-            if (ret != EOK && ret != ENOENT) {
+            if (ret == ENOENT) {
+                dctx->res = talloc_zero(cmdctx, struct ldb_result);
+                if (dctx->res == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+                    return ENOMEM;
+                }
+
+                dctx->res->count = 0;
+                dctx->res->msgs = NULL;
+                ret = EOK;
+            } else if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn failed.\n");
                 return ret;
-            }
+            } else {
+                sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
+                if (sysdb_name == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                        "Sysdb entry does not have a name.\n");
+                    return EINVAL;
+                }
 
-            sysdb_name = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
-            if (sysdb_name == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE,
-                      "Sysdb entry does not have a name.\n");
-                return EINVAL;
-            }
-
-            ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
-            if (ret == EOK && DOM_HAS_VIEWS(dom)) {
-                for (c = 0; c < dctx->res->count; c++) {
-                    ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
-                                                        NULL, NULL);
-                    if (ret != EOK) {
-                        DEBUG(SSSDBG_OP_FAILURE,
-                              "sysdb_add_overrides_to_object failed.\n");
-                        return ret;
+                ret = sysdb_initgroups(cmdctx, dom, sysdb_name, &dctx->res);
+                if (ret == EOK && DOM_HAS_VIEWS(dom)) {
+                    for (c = 0; c < dctx->res->count; c++) {
+                        ret = sysdb_add_overrides_to_object(dom, dctx->res->msgs[c],
+                                                            NULL, NULL);
+                        if (ret != EOK) {
+                            DEBUG(SSSDBG_OP_FAILURE,
+                                "sysdb_add_overrides_to_object failed.\n");
+                            return ret;
+                        }
                     }
                 }
             }
-- 
2.1.0

-------------- next part --------------
>From 0043a367b740969eb6565893c54e53c96562caa8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Mar 2015 16:59:39 +0100
Subject: [PATCH 4/4] Add unit tests for initgroups

---
 src/tests/cmocka/test_nss_srv.c | 346 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 346 insertions(+)

diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 2e95bc0f9a92e702a37bc21f029254691a706025..480d65aec502cd60e11a7a23911c7c7afb59833b 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -39,6 +39,9 @@
 #define TEST_SUBDOM_NAME "test.subdomain"
 #define TEST_ID_PROVIDER "ldap"
 
+#define N_ELEMENTS(arr) \
+    (sizeof(arr) / sizeof(arr[0]))
+
 struct nss_test_ctx {
     struct sss_test_ctx *tctx;
     struct sss_domain_info *subdom;
@@ -205,6 +208,11 @@ static void mock_fill_user(void)
     will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
 }
 
+static void mock_fill_initgr_user(void)
+{
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+}
+
 static void mock_fill_group_with_members(unsigned members)
 {
     unsigned i;
@@ -287,6 +295,26 @@ static int parse_group_packet(uint8_t *body, size_t blen, struct group *gr, uint
     return EOK;
 }
 
+static void check_initgr_packet(uint8_t *body, size_t blen,
+                                gid_t *gids, size_t num_gids)
+{
+    size_t rp;
+    unsigned i;
+    gid_t cur_gid;
+    uint32_t num_ret;
+
+    rp = 0;
+    SAFEALIGN_COPY_UINT32(&num_ret, body, NULL);
+    assert_int_equal(num_ret, num_gids);
+
+    rp = 2 * sizeof(uint32_t); /* Len and reserved */
+
+    for (i = 0; i < num_gids; i++) {
+        SAFEALIGN_COPY_UINT32(&cur_gid, body + rp, &rp);
+        assert_int_equal(cur_gid, gids[i]);
+    }
+}
+
 /* ====================== The tests =============================== */
 
 /* Check getting cached and valid user from cache. Account callback will
@@ -2164,6 +2192,312 @@ void test_nss_getpwnam_upn_neg(void **state)
     assert_int_equal(nss_test_ctx->ncache_hits, 1);
 }
 
+static int test_nss_initgr_check(uint32_t status, uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 3211, 3212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgroups(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, "upninitgr at upndomain.test");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr", 321, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_gr1", 3211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_gr2", 3212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_gr1", "testinitgr",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_gr2", "testinitgr",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testinitgr");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_initgr_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_initgr_neg_by_name(const char *name, bool is_upn)
+{
+    errno_t ret;
+
+    mock_input_user_or_group(name);
+    mock_account_recv_simple();
+
+    assert_int_equal(nss_test_ctx->ncache_hits, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* UPN lookup will first hit negcache with the username */
+    assert_int_equal(nss_test_ctx->ncache_hits, is_upn ? 1 : 0);
+
+    /* Test that subsequent search for a nonexistent user yields
+     * ENOENT and Account callback is not called, on the other hand
+     * the ncache functions will be called
+     */
+    nss_test_ctx->tctx->done = false;
+    nss_test_ctx->ncache_hits = 0;
+
+    mock_input_user_or_group(name);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with ENOENT */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, ENOENT);
+    /* Negative cache was hit this time */
+    assert_int_equal(nss_test_ctx->ncache_hits, 1);
+}
+
+void test_nss_initgr_neg(void **state)
+{
+    test_initgr_neg_by_name("testinitgr_neg", false);
+}
+
+static int test_nss_initgr_search_acct_cb(void *pvt)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", 421, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_srch_gr1", 4211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_srch_gr2", 4212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_srch_gr1", "testinitgr_srch",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_srch_gr2", "testinitgr_srch",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+
+    return EOK;
+}
+
+static int test_nss_initgr_search_check(uint32_t status,
+                                        uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 4211, 4212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgr_search(void **state)
+{
+    errno_t ret;
+    struct ldb_result *res;
+
+    mock_input_user_or_group("testinitgr_srch");
+    mock_account_recv(0, 0, NULL, test_nss_initgr_search_acct_cb, nss_test_ctx);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+    set_cmd_cb(test_nss_initgr_search_check);
+
+    ret = sysdb_getpwnam(nss_test_ctx, nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", &res);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(res->count, 0);
+
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+
+    /* test_nss_getpwnam_search_check will check the user attributes */
+    ret = sysdb_getpwnam(nss_test_ctx, nss_test_ctx->tctx->dom,
+                         "testinitgr_srch", &res);
+    assert_int_equal(ret, EOK);
+    assert_int_equal(res->count, 1);
+}
+
+static int test_nss_initgr_update_acct_cb(void *pvt)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) + 300);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_set_user_attr(nss_test_ctx->tctx->dom,
+                              "testinitgr_update",
+                              attrs, SYSDB_MOD_REP);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_check_gr2", 5212,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_check_gr2",
+                                 "testinitgr_update",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    return EOK;
+}
+
+static int test_nss_initgr_update_check(uint32_t status, uint8_t *body, size_t blen)
+{
+    gid_t expected_gids[] = { 5211, 5212 };
+
+    assert_int_equal(status, EOK);
+    check_initgr_packet(body, blen, expected_gids, N_ELEMENTS(expected_gids));
+    return EOK;
+}
+
+void test_nss_initgr_update(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_uint32(attrs, SYSDB_INITGR_EXPIRE,
+                                 time(NULL) - 1);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testinitgr_update", 521, 654, "test initgroups",
+                         "/home/testinitgr", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group(nss_test_ctx->tctx->dom,
+                          "testinitgr_update_gr1", 5211,
+                          NULL, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_add_group_member(nss_test_ctx->tctx->dom,
+                                 "testinitgr_update_gr1", "testinitgr_update",
+                                 SYSDB_MEMBER_USER, false);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testinitgr_update");
+    mock_account_recv(0, 0, NULL, test_nss_initgr_update_acct_cb, nss_test_ctx);
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+    set_cmd_cb(test_nss_initgr_update_check);
+
+
+    /* Query for that user, call a callback when command finishes */
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+void test_nss_initgroups_upn(void **state)
+{
+    errno_t ret;
+
+    mock_input_user_or_group("upninitgr at upndomain.test");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_INITGR);
+    mock_fill_initgr_user();
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_initgr_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_INITGR,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+/* Test that searching for a nonexistant user yields ENOENT.
+ * Account callback will be called
+ */
+void test_nss_initgr_neg_upn(void **state)
+{
+    test_initgr_neg_by_name("upninitgr_neg at upndomain.test", true);
+}
+
 static int nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -2336,6 +2670,18 @@ int main(int argc, const char *argv[])
                                         nss_test_setup, nss_test_teardown),
         cmocka_unit_test_setup_teardown(test_nss_getpwnam_upn_neg,
                                         nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgroups,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_neg,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_search,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_update,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgroups_upn,
+                                        nss_test_setup, nss_test_teardown),
+        cmocka_unit_test_setup_teardown(test_nss_initgr_neg_upn,
+                                        nss_test_setup, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
2.1.0



More information about the sssd-devel mailing list