[SSSD] [PATCH] nss: return ldap_user_extra_attrs attributes in origbyname request

Sumit Bose sbose at redhat.com
Tue Nov 4 19:49:19 UTC 2014


On Tue, Nov 04, 2014 at 07:15:05PM +0100, Jakub Hrozek wrote:
> On Tue, Nov 04, 2014 at 07:07:11PM +0100, Jakub Hrozek wrote:
> > On Tue, Nov 04, 2014 at 07:02:34PM +0100, Lukas Slebodnik wrote:
> > > On (04/11/14 18:41), Lukas Slebodnik wrote:
> > > >On (04/11/14 18:09), Sumit Bose wrote:
> > > >>On Tue, Nov 04, 2014 at 12:17:43PM +0100, Jakub Hrozek wrote:
> > > >>> On Wed, Oct 29, 2014 at 01:01:43PM +0100, Sumit Bose wrote:
> > > >>> > I agree, I allowed user_attributes for the nss responder as well with a
> > > >>> > fallback to the InfoPipe value so the admin gets it for free if InfoPipe
> > > >>> > is already configured but does not have to configure InfoPipe if not
> > > >>> > needed. See man page change for details.
> > > >>> > 
> > > >>> > Thank you for the review, new version attached.
> > > >>> > 
> > > >>> > bye,
> > > >>> > Sumit
> > > >>> 
> > > >>> I'm sorry about the delay in review.
> > > >>> 
> > > >>> This version looks almost good, but I have one more request -- can we
> > > >>> move ifp_parse_attr_list_ex into responder_common.c and drop the ifp
> > > >>> prefix?
> > > >>
> > > >>I kind of expected this :-) I was a bit reluctant because I was afraid
> > > >>to break something in InfoPipe. I created a new file responder_utils.c
> > > >>for this because responder_common.c looks already quite big.
> > > >>
> > > >>New version attached.
> > > >>
> > > >>bye,
> > > >>Sumit
> > > >>
> > > >>> 
> > > >>> What that would prevent in particular is:
> > > >>> > diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
> > > >>> > index 1bbeaa1..601ffc2 100644
> > > >>> > --- a/src/responder/nss/nsssrv.c
> > > >>> > +++ b/src/responder/nss/nsssrv.c
> > > >>> > @@ -48,6 +48,7 @@
> > > >>> >  #include "monitor/monitor_interfaces.h"
> > > >>> >  #include "sbus/sbus_client.h"
> > > >>> >  #include "util/util_sss_idmap.h"
> > > >>> > +#include "responder/ifp/ifp_private.h"
> > > >>> >  
> > > >>> 
> > > >>> I know we're currently only using a single function, but it's not very
> > > >>> nice to use a private interface of another responder.
> > > >>> 
> > > >>> I'm fine with the rest of the code.
> > > >
> > > >
> > > >>From 52842484474caa2e91ab39c2ef7edc538175aced Mon Sep 17 00:00:00 2001
> > > >>From: Sumit Bose <sbose at redhat.com>
> > > >>Date: Tue, 28 Oct 2014 19:42:47 +0100
> > > >>Subject: [PATCH 3/4] nss: parse user_attributes option
> > > >>
> > > >>---
> > > >> src/man/sssd.conf.5.xml    | 26 ++++++++++++++++++++++++++
> > > >> src/responder/nss/nsssrv.c | 17 +++++++++++++++++
> > > >> src/responder/nss/nsssrv.h |  2 ++
> > > >> 3 files changed, 45 insertions(+)
> > > >>
> > > >//snip
> > > >
> > > >>+++ b/src/responder/nss/nsssrv.c
> > > >>@@ -214,6 +214,7 @@ static int nss_get_config(struct nss_ctx *nctx,
> > > >>                           struct confdb_ctx *cdb)
> > > >> {
> > > >>     int ret;
> > > >>+    char *tmp_str;
> > > >> 
> > > >>     ret = confdb_get_int(cdb, CONFDB_NSS_CONF_ENTRY,
> > > >>                          CONFDB_NSS_ENUM_CACHE_TIMEOUT, 120,
> > > >>@@ -298,6 +299,22 @@ static int nss_get_config(struct nss_ctx *nctx,
> > > >>                             &nctx->homedir_substr);
> > > >>     if (ret != EOK) goto done;
> > > >> 
> > > >>+
> > > >>+    ret = confdb_get_string(cdb, nctx, CONFDB_NSS_CONF_ENTRY,
> > > >>+                            CONFDB_IFP_USER_ATTR_LIST, NULL, &tmp_str);
> > > >>+    if (ret != EOK) goto done;
> > > >>+
> > > >>+    if (tmp_str == NULL) {
> > > >>+        ret = confdb_get_string(cdb, nctx, CONFDB_IFP_CONF_ENTRY,
> > > >>+                                CONFDB_IFP_USER_ATTR_LIST, NULL, &tmp_str);
> > > >>+        if (ret != EOK) goto done;
> > > >>+    }
> > > >>+
> > > >>+    if (tmp_str != NULL) {
> > > >>+        nctx->extra_attributes = parse_attr_list_ex(nctx, tmp_str, NULL);
> > > >>+        if (ret != EOK) goto done;
> > > >              ^^^^^^^^^^
> > > >            It would be better to test "nctx->extra_attributes" rather than ret

d'oh, fixed

> > > >
> > > 
> > > I sent mail very early :-)
> > > 
> > > These patches depends on another patch set.
> > > 
> > >   CC       src/responder/nss/nsssrv_cmd.o
> > > src/responder/nss/nsssrv_cmd.c: In function ‘nss_cmd_getsidby_search’:
> > > src/responder/nss/nsssrv_cmd.c:4259:13: error: implicit declaration of function ‘add_strings_lists’ [-Werror=implicit-function-declaration]
> > >              ret = add_strings_lists(cmdctx, default_attrs,
> > >              ^
> > > cc1: all warnings being treated as errors
> > > 
> > > LS
> > 
> > Yes, we talked about that with Sumit on IRC. I'll reorder the patches
> > before pushing.
> 
> btw I don't have any more comments, I can even fix the check before
> pushing..
> 
> The code works fine:
> $ dbus-send --print-reply --system --dest=org.freedesktop.sssd.infopipe
>         /org/freedesktop/sssd/infopipe org.freedesktop.sssd.infopipe.GetUserAttr
>         string:psuser at ad.example.com array:string:name,gecos,phone
> method return sender=:1.27 -> dest=:1.29 reply_serial=2
>    array [
>       dict entry(
>          string "name"
>          variant             array [
>                string "psuser at AD.EXAMPLE.COM"
>             ]
>       )
>       dict entry(
>          string "gecos"
>          variant             array [
>                string "ps user"
>             ]
>       )
>       dict entry(
>          string "phone"
>          variant             array [
>                string "444-333"
>             ]
>       )
>    ]
> 
> I had to add phone to the allowed attrs to the nss section on the IPA
> server itself (now I see the convenience of using ldap_extra_attrs right
> away..) and to the ifp section on the client.

Thank you Lukas and Jakub.

I added the add_strings_lists() patch to this set to get the order
right.

bye,
Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
-------------- next part --------------
From 43e00ef75e2a425486aa7b9cb308565ce79c51ce Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 27 Oct 2014 16:53:44 +0100
Subject: [PATCH 1/5] Add add_strings_lists() utility function

---
 src/tests/cmocka/test_utils.c | 111 ++++++++++++++++++++++++++++++++++++++++++
 src/util/util.c               |  65 +++++++++++++++++++++++++
 src/util/util.h               |  18 +++++++
 3 files changed, 194 insertions(+)

diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index 9d6cbf3..d978137 100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -875,6 +875,114 @@ void test_expand_homedir_template(void **state)
     talloc_free(tmp_ctx);
 }
 
+void setup_add_strings_lists(void **state)
+{
+    assert_true(leak_check_setup());
+
+    check_leaks_push(global_talloc_context);
+}
+
+void teardown_add_strings_lists(void **state)
+{
+    assert_true(check_leaks_pop(global_talloc_context) == true);
+    assert_true(leak_check_teardown());
+}
+
+void test_add_strings_lists(void **state)
+{
+    const char *l1[] = {"a", "b", "c", NULL};
+    const char *l2[] = {"1", "2", "3", NULL};
+    char **res;
+    int ret;
+    size_t c;
+    size_t d;
+
+    ret = add_strings_lists(global_talloc_context, NULL, NULL, true, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    assert_null(res[0]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, NULL, NULL, false, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    assert_null(res[0]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, l1, NULL, false, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'false', pointers must be equal */
+        assert_int_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+    }
+    assert_null(res[c]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, l1, NULL, true, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'true', pointers must be different, but strings
+         * must be equal */
+        assert_int_not_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+        assert_string_equal(l1[c], res[c]);
+    }
+    assert_null(res[c]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, NULL, l1, false, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'false', pointers must be equal */
+        assert_int_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+    }
+    assert_null(res[c]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, NULL, l1, true, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'true', pointers must be different, but strings
+         * must be equal */
+        assert_int_not_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+        assert_string_equal(l1[c], res[c]);
+    }
+    assert_null(res[c]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, l1, l2, false, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'false', pointers must be equal */
+        assert_int_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+    }
+    for (d = 0; l2[d] != NULL; d++) {
+        assert_int_equal(memcmp(&l2[d], &res[c+d], sizeof(char *)), 0);
+    }
+    assert_null(res[c+d]);
+    talloc_free(res);
+
+    ret = add_strings_lists(global_talloc_context, l1, l2, true, &res);
+    assert_int_equal(ret, EOK);
+    assert_non_null(res);
+    for (c = 0; l1[c] != NULL; c++) {
+        /* 'copy_strings' is 'true', pointers must be different, but strings
+         * must be equal */
+        assert_int_not_equal(memcmp(&l1[c], &res[c], sizeof(char *)), 0);
+        assert_string_equal(l1[c], res[c]);
+    }
+    for (d = 0; l2[d] != NULL; d++) {
+        assert_int_not_equal(memcmp(&l2[d], &res[c+d], sizeof(char *)), 0);
+        assert_string_equal(l2[d], res[c+d]);
+    }
+    assert_null(res[c+d]);
+    talloc_free(res);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -919,6 +1027,9 @@ int main(int argc, const char *argv[])
         unit_test(test_textual_public_key),
         unit_test(test_replace_whitespaces),
         unit_test(test_reverse_replace_whitespaces),
+        unit_test_setup_teardown(test_add_strings_lists,
+                                 setup_add_strings_lists,
+                                 teardown_add_strings_lists),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
diff --git a/src/util/util.c b/src/util/util.c
index d78d37d..2acb860 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -815,3 +815,68 @@ const char * const * get_known_services(void)
 
     return svc;
 }
+
+errno_t add_strings_lists(TALLOC_CTX *mem_ctx, const char **l1, const char **l2,
+                          bool copy_strings, char ***_new_list)
+{
+    size_t c;
+    size_t l1_count = 0;
+    size_t l2_count = 0;
+    size_t new_count = 0;
+    char **new;
+    int ret;
+
+    if (l1 != NULL) {
+        for (l1_count = 0; l1[l1_count] != NULL; l1_count++);
+    }
+
+    if (l2 != NULL) {
+        for (l2_count = 0; l2[l2_count] != NULL; l2_count++);
+    }
+
+    new_count = l1_count + l2_count;
+
+    new = talloc_array(mem_ctx, char *, new_count + 1);
+    if (new == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+        return ENOMEM;
+    }
+    new [new_count] = NULL;
+
+    if (copy_strings) {
+        for(c = 0; c < l1_count; c++) {
+            new[c] = talloc_strdup(new, l1[c]);
+            if (new[c] == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        }
+        for(c = 0; c < l2_count; c++) {
+            new[l1_count + c] = talloc_strdup(new, l2[c]);
+            if (new[l1_count + c] == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        }
+    } else {
+        if (l1 != NULL) {
+            memcpy(new, l1, sizeof(char *) * l1_count);
+        }
+
+        if (l2 != NULL) {
+            memcpy(&new[l1_count], l2, sizeof(char *) * l2_count);
+        }
+    }
+
+    *_new_list = new;
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_free(new);
+    }
+
+    return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 69074c9..ffc8a87 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -426,6 +426,24 @@ errno_t sss_hash_create_ex(TALLOC_CTX *mem_ctx,
                            hash_delete_callback *delete_callback,
                            void *delete_private_data);
 
+/**
+ * @brief Add two list of strings
+ *
+ * Create a new NULL-termintated list of strings by adding two lists together.
+ *
+ * @param[in] mem_ctx      Talloc memory context for the new list.
+ * @param[in] l1           First NULL-termintated list of strings.
+ * @param[in] l2           Second NULL-termintated list of strings.
+ * @param[in] copy_strings If set to 'true' the list items will be copied
+ *                         otherwise only the pointers to the items are
+ *                         copied.
+ * @param[out] new_list    New NULL-terminated list of strings. Must be freed
+ *                         with talloc_free() by the caller. If copy_strings
+ *                         is 'true' the new elements will be freed as well.
+ */
+errno_t add_strings_lists(TALLOC_CTX *mem_ctx, const char **l1, const char **l2,
+                          bool copy_strings, char ***_new_list);
+
 /* Copy a NULL-terminated string list
  * Returns NULL on out of memory error or invalid input
  */
-- 
1.8.3.1

-------------- next part --------------
From 868c748b7ecbfefc49e4dea1e6a7b7ba5bd3070e Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Oct 2014 15:41:04 +0200
Subject: [PATCH 2/5] IPA: inherit ldap_user_extra_attrs to AD subdomains

Currently the component of the IPA provider which reads the AD user and
group attributes in ipa-server-mode uses  default settings for the LDAP
related attributes. As a result even if ldap_user_extra_attrs is defined
in sssd.conf no extra attributes are read from AD.

With the patch the value if ldap_user_extra_attrs is inherited to the AD
subdomains to allow them to read extra attributes as well.

Related to https://fedorahosted.org/sssd/ticket/2464
---
 src/providers/ipa/ipa_subdomains.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index c61c1c6..9281aab 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -109,6 +109,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
     char *ad_domain;
     struct sdap_domain *sdom;
     errno_t ret;
+    const char *extra_attrs;
 
     ad_options = ad_create_default_options(id_ctx, id_ctx->server_mode->realm,
                                            id_ctx->server_mode->hostname);
@@ -135,6 +136,36 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
         return ret;
     }
 
+    extra_attrs = dp_opt_get_string(id_ctx->sdap_id_ctx->opts->basic,
+                            SDAP_USER_EXTRA_ATTRS);
+    if (extra_attrs != NULL) {
+        DEBUG(SSSDBG_TRACE_ALL,
+              "Setting extra attrs for subdomain [%s] to [%s].\n", ad_domain,
+                                                                   extra_attrs);
+
+        ret = dp_opt_set_string(ad_options->id->basic, SDAP_USER_EXTRA_ATTRS,
+                                extra_attrs);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "dp_opt_get_string failed.\n");
+            talloc_free(ad_options);
+            return ret;
+        }
+
+        ret = sdap_extend_map_with_list(ad_options->id, ad_options->id,
+                                        SDAP_USER_EXTRA_ATTRS,
+                                        ad_options->id->user_map,
+                                        SDAP_OPTS_USER,
+                                        &ad_options->id->user_map,
+                                        &ad_options->id->user_map_cnt);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "sdap_extend_map_with_list failed.\n");
+            talloc_free(ad_options);
+            return ret;
+        }
+    } else {
+        DEBUG(SSSDBG_TRACE_ALL, "No extra attrs set.\n");
+    }
+
     gc_service_name = talloc_asprintf(ad_options, "%s%s", "gc_", subdom->name);
     if (gc_service_name == NULL) {
         talloc_free(ad_options);
-- 
1.8.3.1

-------------- next part --------------
From 7df4b1f8fc1cba15d27ed2d31c18e030b587e878 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 28 Oct 2014 19:40:02 +0100
Subject: [PATCH 3/5] Add parse_attr_list_ex() helper function

---
 Makefile.am                            |   5 +-
 src/responder/common/responder.h       |   2 +
 src/responder/common/responder_utils.c | 151 +++++++++++++++++++++++++++++++++
 src/responder/ifp/ifp_private.h        |   3 +
 src/responder/ifp/ifpsrv_util.c        | 117 +------------------------
 src/tests/cmocka/test_ifp.c            |  51 +++++++++++
 6 files changed, 212 insertions(+), 117 deletions(-)
 create mode 100644 src/responder/common/responder_utils.c

diff --git a/Makefile.am b/Makefile.am
index 61bf5cf..ac8ad15 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -409,6 +409,7 @@ SSSD_RESPONDER_OBJ = \
     src/responder/common/responder_dp.c \
     src/responder/common/responder_packet.c \
     src/responder/common/responder_get_domains.c \
+    src/responder/common/responder_utils.c \
     src/monitor/monitor_iface_generated.c \
     src/monitor/monitor_iface_generated.h \
     src/providers/data_provider_iface_generated.c \
@@ -2025,7 +2026,9 @@ ifp_tests_SOURCES = \
     src/tests/cmocka/test_ifp.c \
     src/responder/ifp/ifpsrv_cmd.c \
     src/responder/ifp/ifp_iface_generated.c \
-    src/responder/ifp/ifpsrv_util.c
+    src/responder/ifp/ifpsrv_util.c \
+    src/responder/common/responder_utils.c \
+    $(NULL)
 ifp_tests_CFLAGS = \
     $(AM_CFLAGS)
 ifp_tests_LDADD = \
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index e3c0f22..02a215c 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -329,4 +329,6 @@ sss_parse_inp_send(TALLOC_CTX *mem_ctx, struct resp_ctx *rctx,
 errno_t sss_parse_inp_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
                            char **_name, char **_domname);
 
+const char **parse_attr_list_ex(TALLOC_CTX *mem_ctx, const char *conf_str,
+                                const char **defaults);
 #endif /* __SSS_RESPONDER_H__ */
diff --git a/src/responder/common/responder_utils.c b/src/responder/common/responder_utils.c
new file mode 100644
index 0000000..815b61b
--- /dev/null
+++ b/src/responder/common/responder_utils.c
@@ -0,0 +1,151 @@
+
+/*
+   SSSD
+
+   Common Responder utility functions
+
+   Copyright (C) Sumit Bose <sbose at redhat.com> 2014
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+
+#include "util/util.h"
+
+static inline bool
+attr_in_list(const char **list, size_t nlist, const char *str)
+{
+    size_t i;
+
+    for (i = 0; i < nlist; i++) {
+        if (strcasecmp(list[i], str) == 0) {
+            break;
+        }
+    }
+
+    return (i < nlist) ? true : false;
+}
+
+const char **parse_attr_list_ex(TALLOC_CTX *mem_ctx, const char *conf_str,
+                                const char **defaults)
+{
+    TALLOC_CTX *tmp_ctx;
+    errno_t ret;
+    const char **list = NULL;
+    const char **res = NULL;
+    int list_size;
+    char **conf_list = NULL;
+    int conf_list_size = 0;
+    const char **allow = NULL;
+    const char **deny = NULL;
+    int ai = 0, di = 0, li = 0;
+    int i;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return NULL;
+    }
+
+    if (conf_str) {
+        ret = split_on_separator(tmp_ctx, conf_str, ',', true, true,
+                                 &conf_list, &conf_list_size);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Cannot parse attribute ACL list  %s: %d\n", conf_str, ret);
+            goto done;
+        }
+
+        allow = talloc_zero_array(tmp_ctx, const char *, conf_list_size);
+        deny = talloc_zero_array(tmp_ctx, const char *, conf_list_size);
+        if (allow == NULL || deny == NULL) {
+            goto done;
+        }
+    }
+
+    for (i = 0; i < conf_list_size; i++) {
+        switch (conf_list[i][0]) {
+            case '+':
+                allow[ai] = conf_list[i] + 1;
+                ai++;
+                continue;
+            case '-':
+                deny[di] = conf_list[i] + 1;
+                di++;
+                continue;
+            default:
+                DEBUG(SSSDBG_CRIT_FAILURE, "ACL values must start with "
+                      "either '+' (allow) or '-' (deny), got '%s'\n",
+                      conf_list[i]);
+                goto done;
+        }
+    }
+
+    /* Assume the output will have to hold defaults and all the configured,
+     * values, resize later
+     */
+    list_size = 0;
+    if (defaults != NULL) {
+        while (defaults[list_size]) {
+            list_size++;
+        }
+    }
+    list_size += conf_list_size;
+
+    list = talloc_zero_array(tmp_ctx, const char *, list_size + 1);
+    if (list == NULL) {
+        goto done;
+    }
+
+    /* Start by copying explicitly allowed attributes */
+    for (i = 0; i < ai; i++) {
+        /* if the attribute is explicitly denied, skip it */
+        if (attr_in_list(deny, di, allow[i])) {
+            continue;
+        }
+
+        list[li] = talloc_strdup(list, allow[i]);
+        if (list[li] == NULL) {
+            goto done;
+        }
+        li++;
+
+        DEBUG(SSSDBG_TRACE_INTERNAL,
+              "Added allowed attr %s to whitelist\n", allow[i]);
+    }
+
+    /* Add defaults */
+    if (defaults != NULL) {
+        for (i = 0; defaults[i]; i++) {
+            /* if the attribute is explicitly denied, skip it */
+            if (attr_in_list(deny, di, defaults[i])) {
+                continue;
+            }
+
+            list[li] = talloc_strdup(list, defaults[i]);
+            if (list[li] == NULL) {
+                goto done;
+            }
+            li++;
+
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  "Added default attr %s to whitelist\n", defaults[i]);
+        }
+    }
+
+    res = talloc_steal(mem_ctx, list);
+done:
+    talloc_free(tmp_ctx);
+    return res;
+}
diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
index bf2015a..fb1639c 100644
--- a/src/responder/ifp/ifp_private.h
+++ b/src/responder/ifp/ifp_private.h
@@ -82,5 +82,8 @@ char *_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
 errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
                                struct ldb_message_element *el);
 const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str);
+const char **
+ifp_parse_attr_list_ex(TALLOC_CTX *mem_ctx, const char *conf_str,
+                       const char **defaults);
 bool ifp_attr_allowed(const char *whitelist[], const char *attr);
 #endif /* _IFPSRV_PRIVATE_H_ */
diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c
index c0ab686..909bc54 100644
--- a/src/responder/ifp/ifpsrv_util.c
+++ b/src/responder/ifp/ifpsrv_util.c
@@ -356,127 +356,12 @@ errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
     return EOK;
 }
 
-static inline bool
-attr_in_list(const char **list, size_t nlist, const char *str)
-{
-    size_t i;
-
-    for (i = 0; i < nlist; i++) {
-        if (strcasecmp(list[i], str) == 0) {
-            break;
-        }
-    }
-
-    return (i < nlist) ? true : false;
-}
-
 const char **
 ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str)
 {
-    TALLOC_CTX *tmp_ctx;
-    errno_t ret;
-    const char **list = NULL;
-    const char **res = NULL;
-    int list_size;
-    char **conf_list = NULL;
-    int conf_list_size = 0;
-    const char **allow = NULL;
-    const char **deny = NULL;
-    int ai = 0, di = 0, li = 0;
-    int i;
     const char *defaults[] = IFP_DEFAULT_ATTRS;
 
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        return NULL;
-    }
-
-    if (conf_str) {
-        ret = split_on_separator(tmp_ctx, conf_str, ',', true, true,
-                                 &conf_list, &conf_list_size);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE,
-                  "Cannot parse attribute ACL list  %s: %d\n", conf_str, ret);
-            goto done;
-        }
-
-        allow = talloc_zero_array(tmp_ctx, const char *, conf_list_size);
-        deny = talloc_zero_array(tmp_ctx, const char *, conf_list_size);
-        if (allow == NULL || deny == NULL) {
-            goto done;
-        }
-    }
-
-    for (i = 0; i < conf_list_size; i++) {
-        switch (conf_list[i][0]) {
-            case '+':
-                allow[ai] = conf_list[i] + 1;
-                ai++;
-                continue;
-            case '-':
-                deny[di] = conf_list[i] + 1;
-                di++;
-                continue;
-            default:
-                DEBUG(SSSDBG_CRIT_FAILURE, "ACL values must start with "
-                      "either '+' (allow) or '-' (deny), got '%s'\n",
-                      conf_list[i]);
-                goto done;
-        }
-    }
-
-    /* Assume the output will have to hold defauls and all the configured,
-     * values, resize later
-     */
-    list_size = 0;
-    while (defaults[list_size]) {
-        list_size++;
-    }
-    list_size += conf_list_size;
-
-    list = talloc_zero_array(tmp_ctx, const char *, list_size + 1);
-    if (list == NULL) {
-        goto done;
-    }
-
-    /* Start by copying explicitly allowed attributes */
-    for (i = 0; i < ai; i++) {
-        /* if the attribute is explicitly denied, skip it */
-        if (attr_in_list(deny, di, allow[i])) {
-            continue;
-        }
-
-        list[li] = talloc_strdup(list, allow[i]);
-        if (list[li] == NULL) {
-            goto done;
-        }
-        li++;
-
-        DEBUG(SSSDBG_TRACE_INTERNAL,
-              "Added allowed attr %s to whitelist\n", allow[i]);
-    }
-
-    /* Add defaults */
-    for (i = 0; defaults[i]; i++) {
-        /* if the attribute is explicitly denied, skip it */
-        if (attr_in_list(deny, di, defaults[i])) {
-            continue;
-        }
-
-        list[li] = talloc_strdup(list, defaults[i]);
-        if (list[li] == NULL) {
-            goto done;
-        }
-        li++;
-
-        DEBUG(SSSDBG_TRACE_INTERNAL,
-              "Added default attr %s to whitelist\n", defaults[i]);
-    }
-
-    res = talloc_steal(mem_ctx, list);
-done:
-    talloc_free(tmp_ctx);
-    return res;
+    return parse_attr_list_ex(mem_ctx, conf_str, defaults);
 }
 
 bool
diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c
index b0f6e09..d6e4170 100644
--- a/src/tests/cmocka/test_ifp.c
+++ b/src/tests/cmocka/test_ifp.c
@@ -246,6 +246,29 @@ static void attr_parse_test(const char *expected[], const char *input)
     talloc_free(test_ctx);
 }
 
+static void attr_parse_test_ex(const char *expected[], const char *input,
+                               const char **defaults)
+{
+    const char **res;
+    TALLOC_CTX *test_ctx;
+
+    test_ctx = talloc_new(NULL);
+    assert_non_null(test_ctx);
+
+    res = parse_attr_list_ex(test_ctx, input, defaults);
+
+    if (expected) {
+        /* Positive test */
+        assert_non_null(res);
+        assert_string_list_equal(res, expected);
+    } else {
+        /* Negative test */
+        assert_null(res);
+    }
+
+    talloc_free(test_ctx);
+}
+
 void test_attr_acl(void **state)
 {
     /* Test defaults */
@@ -296,6 +319,33 @@ void test_attr_acl(void **state)
     attr_parse_test(NULL,  "missing_plus_or_minus");
 }
 
+void test_attr_acl_ex(void **state)
+{
+    /* Test defaults */
+    const char *exp_defaults[] = { "abc", "123", "xyz", NULL };
+    attr_parse_test_ex(exp_defaults, NULL, exp_defaults);
+
+    /* Test adding some attributes to the defaults */
+    const char *exp_add[] = { "telephoneNumber", "streetAddress",
+                              "abc", "123", "xyz",
+                              NULL };
+    attr_parse_test_ex(exp_add, "+telephoneNumber, +streetAddress",
+                       exp_defaults);
+
+    /* Test removing some attributes to the defaults */
+    const char *exp_rm[] = { "123", NULL };
+    attr_parse_test_ex(exp_rm, "-abc, -xyz", exp_defaults);
+
+    /* Test adding with empty defaults */
+    const char *exp_add_empty[] = { "telephoneNumber", "streetAddress",
+                                    NULL };
+    attr_parse_test_ex(exp_add_empty, "+telephoneNumber, +streetAddress", NULL);
+
+    /* Test removing with empty defaults */
+    const char *rm_all[] = { NULL };
+    attr_parse_test_ex(rm_all, "-telephoneNumber, -streetAddress", NULL);
+}
+
 void test_attr_allowed(void **state)
 {
     const char *whitelist[] = { "name", "gecos", NULL };
@@ -452,6 +502,7 @@ int main(int argc, const char *argv[])
         unit_test(test_path_prefix),
         unit_test(test_el_to_dict),
         unit_test(test_attr_acl),
+        unit_test(test_attr_acl_ex),
         unit_test(test_attr_allowed),
         unit_test(test_path_escape_unescape),
         unit_test_setup_teardown(test_reply_path,
-- 
1.8.3.1

-------------- next part --------------
From bf1cbb06a3585da0896dd378b565764bb780d19d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 28 Oct 2014 19:42:47 +0100
Subject: [PATCH 4/5] nss: parse user_attributes option

---
 src/man/sssd.conf.5.xml    | 26 ++++++++++++++++++++++++++
 src/responder/nss/nsssrv.c | 17 +++++++++++++++++
 src/responder/nss/nsssrv.h |  2 ++
 3 files changed, 45 insertions(+)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index e2cb0b8..fbaca66 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -711,6 +711,32 @@ fallback_homedir = /home/%u
                         </para>
                     </listitem>
                 </varlistentry>
+                <varlistentry>
+                    <term>user_attributes (string)</term>
+                    <listitem>
+                        <para>
+                            Some of the additional NSS responder requests can
+                            return more attributes than just the POSIX ones
+                            defined by the NSS interface. The list of attributes
+                            is controlled by this option. It is handle the same
+                            way as the <quote>user_attributes</quote> option of
+                            the InfoPipe responder (see
+                            <citerefentry>
+                                <refentrytitle>sssd-ifp</refentrytitle>
+                                <manvolnum>5</manvolnum>
+                            </citerefentry>
+                            for details) but with no default values.
+                        </para>
+                        <para>
+                            To make configuration more easy the NSS responder
+                            will check the InfoPipe option if it is not set for
+                            the NSS responder.
+                        </para>
+                        <para>
+                            Default: not set, fallback to InfoPipe option
+                        </para>
+                    </listitem>
+                </varlistentry>
             </variablelist>
         </refsect2>
         <refsect2 id='PAM'>
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index 1bbeaa1..baea9d5 100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -214,6 +214,7 @@ static int nss_get_config(struct nss_ctx *nctx,
                           struct confdb_ctx *cdb)
 {
     int ret;
+    char *tmp_str;
 
     ret = confdb_get_int(cdb, CONFDB_NSS_CONF_ENTRY,
                          CONFDB_NSS_ENUM_CACHE_TIMEOUT, 120,
@@ -298,6 +299,22 @@ static int nss_get_config(struct nss_ctx *nctx,
                             &nctx->homedir_substr);
     if (ret != EOK) goto done;
 
+
+    ret = confdb_get_string(cdb, nctx, CONFDB_NSS_CONF_ENTRY,
+                            CONFDB_IFP_USER_ATTR_LIST, NULL, &tmp_str);
+    if (ret != EOK) goto done;
+
+    if (tmp_str == NULL) {
+        ret = confdb_get_string(cdb, nctx, CONFDB_IFP_CONF_ENTRY,
+                                CONFDB_IFP_USER_ATTR_LIST, NULL, &tmp_str);
+        if (ret != EOK) goto done;
+    }
+
+    if (tmp_str != NULL) {
+        nctx->extra_attributes = parse_attr_list_ex(nctx, tmp_str, NULL);
+        if (ret != EOK) goto done;
+    }
+
     ret = 0;
 done:
     return ret;
diff --git a/src/responder/nss/nsssrv.h b/src/responder/nss/nsssrv.h
index a5b946b..784eba2 100644
--- a/src/responder/nss/nsssrv.h
+++ b/src/responder/nss/nsssrv.h
@@ -75,6 +75,8 @@ struct nss_ctx {
 
     struct sss_idmap_ctx *idmap_ctx;
     struct sss_names_ctx *global_names;
+
+    const char **extra_attributes;
 };
 
 struct nss_packet;
-- 
1.8.3.1

-------------- next part --------------
From 9457c1d6df9ac1939c5381ee1b54d44a617d58cf Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Oct 2014 11:30:33 +0200
Subject: [PATCH 5/5] nss: return user_attributes in origbyname request

To allow IPA clients to offer special attributes of AD users form
trusted domain the extdom plugin on the IPA server must send them to the
clients. The extdom plugin already uses sss_nss_getorigbyname() to get
attributes like the SID and the user principal name. This patch adds the
attributes given by the NSS/IFP user_attributes option to the list of
attributes returned by sss_nss_getorigbyname().

Fixes https://fedorahosted.org/sssd/ticket/2464
---
 src/responder/nss/nsssrv_cmd.c  | 120 +++++++++++++++++++++++++++---------
 src/tests/cmocka/test_nss_srv.c | 133 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 223 insertions(+), 30 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 9fca644..28275ac 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4119,18 +4119,19 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
     struct nss_ctx *nctx;
     int ret;
     int err;
-    const char *attrs[] = {SYSDB_NAME, SYSDB_OBJECTCLASS, SYSDB_SID_STR,
-                           ORIGINALAD_PREFIX SYSDB_NAME,
-                           ORIGINALAD_PREFIX SYSDB_UIDNUM,
-                           ORIGINALAD_PREFIX SYSDB_GIDNUM,
-                           ORIGINALAD_PREFIX SYSDB_GECOS,
-                           ORIGINALAD_PREFIX SYSDB_HOMEDIR,
-                           ORIGINALAD_PREFIX SYSDB_SHELL,
-                           SYSDB_UPN,
-                           SYSDB_DEFAULT_OVERRIDE_NAME,
-                           SYSDB_AD_ACCOUNT_EXPIRES,
-                           SYSDB_AD_USER_ACCOUNT_CONTROL,
-                           SYSDB_DEFAULT_ATTRS, NULL};
+    const char *default_attrs[] = {SYSDB_NAME, SYSDB_OBJECTCLASS, SYSDB_SID_STR,
+                                   ORIGINALAD_PREFIX SYSDB_NAME,
+                                   ORIGINALAD_PREFIX SYSDB_UIDNUM,
+                                   ORIGINALAD_PREFIX SYSDB_GIDNUM,
+                                   ORIGINALAD_PREFIX SYSDB_GECOS,
+                                   ORIGINALAD_PREFIX SYSDB_HOMEDIR,
+                                   ORIGINALAD_PREFIX SYSDB_SHELL,
+                                   SYSDB_UPN,
+                                   SYSDB_DEFAULT_OVERRIDE_NAME,
+                                   SYSDB_AD_ACCOUNT_EXPIRES,
+                                   SYSDB_AD_USER_ACCOUNT_CONTROL,
+                                   SYSDB_DEFAULT_ATTRS, NULL};
+    const char **attrs;
     bool user_found = false;
     bool group_found = false;
     struct ldb_message *msg = NULL;
@@ -4252,6 +4253,18 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
             goto done;
         }
 
+        attrs = default_attrs;
+        if (cmdctx->cmd == SSS_NSS_GETORIGBYNAME
+                && nctx->extra_attributes != NULL) {
+            ret = add_strings_lists(cmdctx, default_attrs,
+                                    nctx->extra_attributes, false,
+                                    discard_const(&attrs));
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, "add_strings_lists failed.\n");
+                goto done;
+            }
+        }
+
         if (cmdctx->cmd == SSS_NSS_GETSIDBYID) {
             ret = sysdb_search_user_by_uid(cmdctx, dom, cmdctx->id, attrs,
                                            &msg);
@@ -4564,16 +4577,21 @@ static errno_t fill_sid(struct sss_packet *packet,
 }
 
 static errno_t fill_orig(struct sss_packet *packet,
+                         struct resp_ctx *rctx,
                          enum sss_id_type id_type,
                          struct ldb_message *msg)
 {
     int ret;
+    TALLOC_CTX *tmp_ctx;
     const char *tmp_str;
     uint8_t *body;
     size_t blen;
     size_t pctr = 0;
     size_t c;
     size_t sum;
+    size_t found;
+    size_t extra_attrs_count = 0;
+    const char **extra_attrs_list = NULL;
     const char *orig_attr_list[] = {SYSDB_SID_STR,
                                     ORIGINALAD_PREFIX SYSDB_NAME,
                                     ORIGINALAD_PREFIX SYSDB_UIDNUM,
@@ -4586,42 +4604,83 @@ static errno_t fill_orig(struct sss_packet *packet,
                                     SYSDB_AD_ACCOUNT_EXPIRES,
                                     SYSDB_AD_USER_ACCOUNT_CONTROL,
                                     NULL};
-    struct sized_string keys[sizeof(orig_attr_list)];
-    struct sized_string vals[sizeof(orig_attr_list)];
+    struct sized_string *keys;
+    struct sized_string *vals;
+    struct nss_ctx *nctx;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
+        return ENOMEM;
+    }
+
+    nctx = talloc_get_type(rctx->pvt_ctx, struct nss_ctx);
+    if (nctx->extra_attributes != NULL) {
+        extra_attrs_list = nctx->extra_attributes;
+            for(extra_attrs_count = 0;
+                extra_attrs_list[extra_attrs_count] != NULL;
+                extra_attrs_count++);
+    }
+
+    keys = talloc_array(tmp_ctx, struct sized_string,
+                        sizeof(orig_attr_list) + extra_attrs_count);
+    vals = talloc_array(tmp_ctx, struct sized_string,
+                        sizeof(orig_attr_list) + extra_attrs_count);
+    if (keys == NULL || vals == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
 
     sum = 0;
+    found = 0;
     for (c = 0; orig_attr_list[c] != NULL; c++) {
         tmp_str = ldb_msg_find_attr_as_string(msg, orig_attr_list[c], NULL);
         if (tmp_str != NULL) {
-            to_sized_string(&keys[c], orig_attr_list[c]);
-            sum += keys[c].len;
-            to_sized_string(&vals[c], tmp_str);
-            sum += vals[c].len;
-        } else {
-            vals[c].len = 0;
+            to_sized_string(&keys[found], orig_attr_list[c]);
+            sum += keys[found].len;
+            to_sized_string(&vals[found], tmp_str);
+            sum += vals[found].len;
+
+            found++;
+        }
+    }
+
+    for (c = 0; c < extra_attrs_count; c++) {
+        tmp_str = ldb_msg_find_attr_as_string(msg, extra_attrs_list[c], NULL);
+        if (tmp_str != NULL) {
+            to_sized_string(&keys[found], extra_attrs_list[c]);
+            sum += keys[found].len;
+            to_sized_string(&vals[found], tmp_str);
+            sum += vals[found].len;
+
+            found++;
         }
     }
 
     ret = sss_packet_grow(packet, sum +  3 * sizeof(uint32_t));
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "sss_packet_grow failed.\n");
-        return ret;
+        goto done;
     }
 
     sss_packet_get_body(packet, &body, &blen);
     SAFEALIGN_SETMEM_UINT32(body, 1, &pctr); /* Num results */
     SAFEALIGN_SETMEM_UINT32(body + pctr, 0, &pctr); /* reserved */
     SAFEALIGN_COPY_UINT32(body + pctr, &id_type, &pctr);
-    for (c = 0; orig_attr_list[c] != NULL; c++) {
-        if (vals[c].len != 0) {
-            memcpy(&body[pctr], keys[c].str, keys[c].len);
-            pctr+= keys[c].len;
-            memcpy(&body[pctr], vals[c].str, vals[c].len);
-            pctr+= vals[c].len;
-        }
+    for (c = 0; c < found; c++) {
+        memcpy(&body[pctr], keys[c].str, keys[c].len);
+        pctr+= keys[c].len;
+        memcpy(&body[pctr], vals[c].str, vals[c].len);
+        pctr+= vals[c].len;
     }
 
-    return EOK;
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+
+    return ret;
 }
 
 static errno_t fill_name(struct sss_packet *packet,
@@ -4789,7 +4848,8 @@ static errno_t nss_cmd_getbysid_send_reply(struct nss_dom_ctx *dctx)
         ret = fill_sid(cctx->creq->out, id_type, dctx->res->msgs[0]);
         break;
     case SSS_NSS_GETORIGBYNAME:
-        ret = fill_orig(cctx->creq->out, id_type, dctx->res->msgs[0]);
+        ret = fill_orig(cctx->creq->out, cctx->rctx, id_type,
+                        dctx->res->msgs[0]);
         break;
     default:
         DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported request type.\n");
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 65298cd..c318d94 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -52,6 +52,8 @@ struct nss_test_ctx {
     bool ncache_hit;
 };
 
+const char *global_extra_attrs[] = {"phone", "mobile", NULL};
+
 struct nss_test_ctx *nss_test_ctx;
 
 /* Mock NSS structure */
@@ -1000,6 +1002,7 @@ void test_nss_setup(struct sss_test_conf_param params[],
     nss_test_ctx->rctx = mock_rctx(nss_test_ctx, nss_test_ctx->tctx->ev,
                                    nss_test_ctx->tctx->dom, nss_test_ctx->nctx);
     assert_non_null(nss_test_ctx->rctx);
+    nss_test_ctx->rctx->cdb = nss_test_ctx->tctx->confdb;
     nss_test_ctx->nctx->rctx = nss_test_ctx->rctx;
 
     /* Create client context */
@@ -1827,6 +1830,122 @@ void test_nss_getorigbyname(void **state)
     assert_int_equal(ret, EOK);
 }
 
+static int test_nss_getorigbyname_extra_check(uint32_t status, uint8_t *body,
+                                              size_t blen)
+{
+    const char *s;
+    enum sss_id_type id_type;
+    size_t rp = 2 * sizeof(uint32_t);
+
+    assert_int_equal(status, EOK);
+
+    SAFEALIGN_COPY_UINT32(&id_type, body+rp, &rp);
+    assert_int_equal(id_type, SSS_ID_TYPE_UID);
+
+    /* Sequence of null terminated strings */
+    s = (char *) body+rp;
+    assert_string_equal(s, SYSDB_SID_STR);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "S-1-2-3-4");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, ORIGINALAD_PREFIX SYSDB_NAME);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "orig_name");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, ORIGINALAD_PREFIX SYSDB_UIDNUM);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "1234");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "phone");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "+12-34 56 78");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "mobile");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "+98-76 54 32");
+    rp += strlen(s) + 1;
+    assert_int_equal(rp, blen);
+
+    return EOK;
+}
+
+void test_nss_getorigbyname_extra_attrs(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_SID_STR, "S-1-2-3-4");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, ORIGINALAD_PREFIX SYSDB_NAME,
+                                 "orig_name");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_uint32(attrs, ORIGINALAD_PREFIX SYSDB_UIDNUM, 1234);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "phone", "+12-34 56 78");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "mobile", "+98-76 54 32");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "not_extra", "abc");
+    assert_int_equal(ret, EOK);
+
+    /* Prime the cache with a valid user */
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testuserorigextra", 2345, 6789,
+                         "test user orig extra",
+                         "/home/testuserorigextra", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testuserorigextra");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETORIGBYNAME);
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_getorigbyname_extra_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETORIGBYNAME,
+                          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 nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -1848,6 +1967,18 @@ void nss_fqdn_test_setup(void **state)
     test_nss_setup(params, state);
 }
 
+void nss_test_setup_extra_attr(void **state)
+{
+    struct sss_test_conf_param params[] = {
+        { "enumerate", "false" },
+        { NULL, NULL },             /* Sentinel */
+    };
+
+    test_nss_setup(params, state);
+
+    nss_test_ctx->nctx->extra_attributes = global_extra_attrs;
+}
+
 void nss_subdom_test_setup(void **state)
 {
     const char *const testdom[4] = { TEST_SUBDOM_NAME, "TEST.SUB", "test", "S-3" };
@@ -1963,6 +2094,8 @@ int main(int argc, const char *argv[])
                                  nss_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getorigbyname,
                                  nss_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getorigbyname_extra_attrs,
+                                 nss_test_setup_extra_attr, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1



More information about the sssd-devel mailing list