URL: https://github.com/SSSD/sssd/pull/31
Title: #31: nss: allow UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME

sumit-bose commented:
"""
On Mon, Sep 26, 2016 at 02:13:08AM -0700, Jakub Hrozek wrote:
> jhrozek commented on this pull request.
> 
> 
> 
> >              if (name == NULL) {
>                  DEBUG(SSSDBG_OP_FAILURE, "sss_get_cased_name failed.\n");
>                  ret = ENOMEM;
>                  goto done;
>              }
>  
> +            if (cmdctx->name_is_upn) {
> +                neg_cache_name = talloc_asprintf(name, "@%s", name);
> 
> Hmm this is new, why is there the @-sign?

It is not new, it is used in nss_cmd_getpwnam_search() as well. 

commit 62df78512145db94b51c5573d4df1737197e368a
Author: Sumit Bose <sb...@redhat.com>
Date:   Fri Jul 22 16:01:38 2016 +0200

    NSS: use different neg cache name for UPN searches
    
    If Kerberos principals or email address have the same domain suffix as
    the domain itself the first user lookup by name might have already added
    the name to the negative cache and the second lookup by UPN/email will
    skip the domain because of the neg cache entry. To avoid this a special
    name with a '@' prefix is used here.
    
    Reviewed-by: Jakub Hrozek <jhro...@redhat.com>


But I agree that it would be better to make this a bit more obvious by
e.g. adding something like sss_ncache_name(const char *name, bool
is_upn).

> 
> > @@ -4573,8 +4623,19 @@ static errno_t nss_cmd_getsidby_search(struct 
> > nss_dom_ctx *dctx)
>                  req_type = SSS_DP_USER_AND_GROUP;
>              }
>  
> +            if (cmdctx->name_is_upn) {
> +                extra_flag = EXTRA_NAME_IS_UPN;
> +            } else if (DOM_HAS_VIEWS(dom) && (dctx->res->count == 0
> +                    || ldb_msg_find_attr_as_string(dctx->res->msgs[0],
> +                                                   OVERRIDE_PREFIX 
> SYSDB_NAME,
> +                                                   NULL) != NULL)) {
> 
> I started writing tests for this new functionality and I wonder if this is 
> another a bit unrelated thing this patch fixes -- looking up SID by overriden 
> name?

Currently it is more a copy-and-paste error because the name based sysdb
searches in nss_cmd_getsidby_search() do not use the *_with_views()
variant of the searches.

There was some discussion when adding the override feature what name a
by-SID search should return and we agreed that in this case always the
original name from AD should be returned. I guess as a result the whole
SID lookup related code path was skipped when adding overrides. But I
think it would be ok to allow lookup by overriden names here as well.

> 
> -- 
> You are receiving this because you authored the thread.
> Reply to this email directly or view it on GitHub:
> https://github.com/SSSD/sssd/pull/31#pullrequestreview-1498162

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/31#issuecomment-249525279
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to