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

Lukas Slebodnik lslebodn at redhat.com
Tue Nov 4 17:41:42 UTC 2014


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

LS



More information about the sssd-devel mailing list