URL:
https://github.com/SSSD/sssd/pull/31
Title: #31: nss: allow UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
jhrozek commented:
"""
On Mon, Sep 26, 2016 at 02:43:03AM -0700, sumit-bose wrote:
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?
(Summing up a discussion from IRC)
It is not new, it is used in nss_cmd_getpwnam_search() as well.
commit 62df78512145db94b51c5573d4df1737197e368a
Author: Sumit Bose <sbose(a)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 <jhrozek(a)redhat.com>
Oops, I forgot it was even me who reviewed this change :)
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).
Yes, but I think it would be more systematic to do this change in the
cache_req code. I filed
https://fedorahosted.org/sssd/ticket/3204 to
track this.
>
> > @@ -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.
OK, as discussed on IRC, it would be better to remove this change,
resubmit this patch so only related changes are included and file a
ticket to track this separate bug.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/31#issuecomment-250107905