Re: [SSSD] [PATCH] cache_req: support UPN

[Jakub Hrozek]

On Fri, Oct 09, 2015 at 02:14:12PM +0200, Pavel Březina wrote:
> On 10/09/2015 01:34 PM, Sumit Bose wrote:
> >On Thu, Oct 08, 2015 at 08:51:41PM +0200, Jakub Hrozek wrote:
> >>On Wed, Oct 07, 2015 at 04:28:02PM +0200, Sumit Bose wrote:
> >>> From 3d5352977ca07f8a1a6d3bbaa57d11b54fd705d0 Mon Sep 17 00:00:00 2001
> >>>From: Sumit Bose 
> >>>Date: Wed, 7 Oct 2015 15:22:34 +0200
> >>>Subject: [PATCH 3/3] nss: fix UPN lookups for sub-domain users
> >>
> >>This one should go to sssd-1-12, right? Can you send a rebased version,
> >>please?
> >
> >please find it attached.
> >
> >bye,
> >Sumit
> 
> Ack.

CI: http://sssd-ci.duckdns.org/logs/job/30/06/summary.html

* sssd-1-12: e412f49be66e6248fbfa61cdd1516c3444deaff8
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Sumit Bose]

On Thu, Oct 08, 2015 at 08:51:41PM +0200, Jakub Hrozek wrote:
> On Wed, Oct 07, 2015 at 04:28:02PM +0200, Sumit Bose wrote:
> > From 3d5352977ca07f8a1a6d3bbaa57d11b54fd705d0 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose 
> > Date: Wed, 7 Oct 2015 15:22:34 +0200
> > Subject: [PATCH 3/3] nss: fix UPN lookups for sub-domain users
> 
> This one should go to sssd-1-12, right? Can you send a rebased version,
> please?

please find it attached.

bye,
Sumit

> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 4a3c728624bc310b15690ae697d273e0a67289b3 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 7 Oct 2015 15:22:34 +0200
Subject: [PATCH] nss: fix UPN lookups for sub-domain users

Resolves https://fedorahosted.org/sssd/ticket/2827
---
 src/db/sysdb_ops.c |  3 +--
 src/responder/nss/nsssrv_cmd.c | 12 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 
ea786d59158eb8a82952c7e457ea83286abbf2c4..34f1832950939203375bbdbb843700bd2d1833f2
 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -494,8 +494,7 @@ int sysdb_search_user_by_upn(TALLOC_CTX *mem_ctx,
 return ENOMEM;
 }
 
-basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb,
-SYSDB_TMPL_USER_BASE, domain->name);
+basedn = sysdb_base_dn(domain->sysdb, tmp_ctx);
 if (basedn == NULL) {
 ret = ENOMEM;
 goto done;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 
12134ce65f5aa7fa9748fad1462a7f647c70df98..4285473de81d2cef32043ff59a3a0f8111dbf044
 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -849,7 +849,11 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx 
*dctx)
name, dom->name);
 /* if a multidomain search, try with next */
 if (cmdctx->check_next) {
-dom = get_next_domain(dom, false);
+if (cmdctx->name_is_upn) {
+dom = get_next_domain(dom, true);
+} else {
+dom = get_next_domain(dom, false);
+}
 continue;
 }
 /* There are no further domains or this was a
@@ -924,7 +928,11 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx 
*dctx)
 
 /* if a multidomain search, try with next */
 if (cmdctx->check_next) {
-dom = get_next_domain(dom, false);
+if (cmdctx->name_is_upn) {
+dom = get_next_domain(dom, true);
+} else {
+dom = get_next_domain(dom, false);
+}
 if (dom) continue;
 }
 
-- 
2.1.0

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Pavel Březina]

On 10/09/2015 01:34 PM, Sumit Bose wrote:

On Thu, Oct 08, 2015 at 08:51:41PM +0200, Jakub Hrozek wrote:

On Wed, Oct 07, 2015 at 04:28:02PM +0200, Sumit Bose wrote:

 From 3d5352977ca07f8a1a6d3bbaa57d11b54fd705d0 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 7 Oct 2015 15:22:34 +0200
Subject: [PATCH 3/3] nss: fix UPN lookups for sub-domain users


This one should go to sssd-1-12, right? Can you send a rebased version,
please?


please find it attached.

bye,
Sumit


Ack.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Pavel Březina]

On 10/07/2015 04:28 PM, Sumit Bose wrote:

On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:

On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:

On 10/06/2015 03:16 PM, Pavel Březina wrote:

On 10/01/2015 05:06 PM, Sumit Bose wrote:

On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:

On 09/17/2015 10:32 AM, Petr Cech wrote:

Hi Pavel!

There is some code between my last end and this continuation. I was
read
it and did't find anything wrong.


Hi Petr,
thank you for you review. New patches are attached. All comments from
the
previous mail should be addressed.




[PATCH 2/3] cache_req: add support for UPN


diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index
5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
  return filter;
  }

+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
+   struct sss_domain_info *domain,
+   const char *upn,
+   struct ldb_result **_res)
+{
+TALLOC_CTX *tmp_ctx;
+struct ldb_message *msg;
+struct ldb_result *res;
+static const char *attrs[] = SYSDB_PW_ATTRS;
+errno_t ret;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+return ENOMEM;
+}
+
+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, );
+if (ret != EOK && ret != ENOENT) {
+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
failed.\n");
+goto done;
+}


The call stack here would be

sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()


and the returned results, starting from ldb_search()

ldb_result -> ldb_message (array) -> ldb_message -> ldb_result

I think it would be easier and safe some allocations if it would be

sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .

Do you think this change makes sense?


Hi, thanks for the review. Done.

Although I created a new function sysdb_search_user_by_upn_res since you
can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
api).




const char *orig_name;


Instead of adding a new member which again will hold the originally
provided name I would suggest to refactor the change added by
e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
needed) by introducing a member called e.g. parsed_name which keeps the
name returned by sss_parse_inp_recv() and return it in cache_req_recv().

In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
orig_name because no additional member was needed. But now I think the
code would be more clear if orig_name will be kept unmodified.


Done in separate patch so the changes are more visible. It can be
squashed to the second patch.


Sumit found that test won't pass. New patchset is attached.




Thank you, the tests pass now. I've seen a compiler warning which is
seen with the first patch I attached.

During testing I found that UPN lookups for sub-domain users do not
work even with the original nss responder code. I attached fixes for
your patches and the nss responder since the sysdb change depends on you
patches.


and now with patches ...


Ack. Thank you!

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Jakub Hrozek]

On Thu, Oct 08, 2015 at 08:34:17PM +0200, Jakub Hrozek wrote:
> On Thu, Oct 08, 2015 at 01:21:29PM +0200, Pavel Březina wrote:
> > On 10/07/2015 04:28 PM, Sumit Bose wrote:
> > >On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:
> > >>On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
> > >>>On 10/06/2015 03:16 PM, Pavel Březina wrote:
> > On 10/01/2015 05:06 PM, Sumit Bose wrote:
> > >On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
> > >>On 09/17/2015 10:32 AM, Petr Cech wrote:
> > >>>Hi Pavel!
> > >>>
> > >>>There is some code between my last end and this continuation. I was
> > >>>read
> > >>>it and did't find anything wrong.
> > >>
> > >>Hi Petr,
> > >>thank you for you review. New patches are attached. All comments from
> > >>the
> > >>previous mail should be addressed.
> > >>
> > >
> > >
> > >[PATCH 2/3] cache_req: add support for UPN
> > >
> > >>diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> > >>index
> > >>5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
> > >>100644
> > >>--- a/src/db/sysdb_search.c
> > >>+++ b/src/db/sysdb_search.c
> > >>@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
> > >>  return filter;
> > >>  }
> > >>
> > >>+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
> > >>+   struct sss_domain_info *domain,
> > >>+   const char *upn,
> > >>+   struct ldb_result **_res)
> > >>+{
> > >>+TALLOC_CTX *tmp_ctx;
> > >>+struct ldb_message *msg;
> > >>+struct ldb_result *res;
> > >>+static const char *attrs[] = SYSDB_PW_ATTRS;
> > >>+errno_t ret;
> > >>+
> > >>+tmp_ctx = talloc_new(NULL);
> > >>+if (tmp_ctx == NULL) {
> > >>+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> > >>+return ENOMEM;
> > >>+}
> > >>+
> > >>+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, 
> > >>);
> > >>+if (ret != EOK && ret != ENOENT) {
> > >>+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
> > >>failed.\n");
> > >>+goto done;
> > >>+}
> > >
> > >The call stack here would be
> > >
> > >sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
> > >
> > >
> > >and the returned results, starting from ldb_search()
> > >
> > >ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
> > >
> > >I think it would be easier and safe some allocations if it would be
> > >
> > >sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
> > >
> > >Do you think this change makes sense?
> > 
> > Hi, thanks for the review. Done.
> > 
> > Although I created a new function sysdb_search_user_by_upn_res since you
> > can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
> > api).
> > 
> > >
> > const char *orig_name;
> > >
> > >Instead of adding a new member which again will hold the originally
> > >provided name I would suggest to refactor the change added by
> > >e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name 
> > >if
> > >needed) by introducing a member called e.g. parsed_name which keeps the
> > >name returned by sss_parse_inp_recv() and return it in 
> > >cache_req_recv().
> > >
> > >In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> > >orig_name because no additional member was needed. But now I think the
> > >code would be more clear if orig_name will be kept unmodified.
> > 
> > Done in separate patch so the changes are more visible. It can be
> > squashed to the second patch.
> > >>>
> > >>>Sumit found that test won't pass. New patchset is attached.
> > >>>
> > >>>
> > >>
> > >>Thank you, the tests pass now. I've seen a compiler warning which is
> > >>seen with the first patch I attached.
> > >>
> > >>During testing I found that UPN lookups for sub-domain users do not
> > >>work even with the original nss responder code. I attached fixes for
> > >>your patches and the nss responder since the sysdb change depends on you
> > >>patches.
> > >
> > >and now with patches ...
> > 
> > Ack. Thank you!
> 
> I pushed both sets to master:
> * 8ded8b2f4a57d1833fd230307218d8b07a571785
> * 374268c5eda35e8bbc2fef30752299199439cffe
> * 391b81f2a78a812a87530e0c50c70d59150f49eb
> * 2fce47f2dadd10d2a2c8bf9f03ab7094bc6c6b3a
> * 3688374991afb34bbaf2b7843683fc13dd77879d
> * 28ebfa4373d1e7ce45b5d70a3619df1c074a661e
> * d8125f0e0d38c6939887a0849a44859d6c498c57
> and sssd-1-13:
> * b1b0abe6223348083be552531f195518b0df3ba5
> * f2c3994c6ffd6e715621d7d7ee580db3cd3e85a9
> * 055248cf20c1ad9dfba06903789e71b88752b810
>   

Re: [SSSD] [PATCH] cache_req: support UPN

[Jakub Hrozek]

On Wed, Oct 07, 2015 at 04:28:02PM +0200, Sumit Bose wrote:
> From 3d5352977ca07f8a1a6d3bbaa57d11b54fd705d0 Mon Sep 17 00:00:00 2001
> From: Sumit Bose 
> Date: Wed, 7 Oct 2015 15:22:34 +0200
> Subject: [PATCH 3/3] nss: fix UPN lookups for sub-domain users

This one should go to sssd-1-12, right? Can you send a rebased version,
please?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Jakub Hrozek]

On Thu, Oct 08, 2015 at 01:21:29PM +0200, Pavel Březina wrote:
> On 10/07/2015 04:28 PM, Sumit Bose wrote:
> >On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:
> >>On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
> >>>On 10/06/2015 03:16 PM, Pavel Březina wrote:
> On 10/01/2015 05:06 PM, Sumit Bose wrote:
> >On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
> >>On 09/17/2015 10:32 AM, Petr Cech wrote:
> >>>Hi Pavel!
> >>>
> >>>There is some code between my last end and this continuation. I was
> >>>read
> >>>it and did't find anything wrong.
> >>
> >>Hi Petr,
> >>thank you for you review. New patches are attached. All comments from
> >>the
> >>previous mail should be addressed.
> >>
> >
> >
> >[PATCH 2/3] cache_req: add support for UPN
> >
> >>diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> >>index
> >>5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
> >>100644
> >>--- a/src/db/sysdb_search.c
> >>+++ b/src/db/sysdb_search.c
> >>@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
> >>  return filter;
> >>  }
> >>
> >>+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
> >>+   struct sss_domain_info *domain,
> >>+   const char *upn,
> >>+   struct ldb_result **_res)
> >>+{
> >>+TALLOC_CTX *tmp_ctx;
> >>+struct ldb_message *msg;
> >>+struct ldb_result *res;
> >>+static const char *attrs[] = SYSDB_PW_ATTRS;
> >>+errno_t ret;
> >>+
> >>+tmp_ctx = talloc_new(NULL);
> >>+if (tmp_ctx == NULL) {
> >>+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> >>+return ENOMEM;
> >>+}
> >>+
> >>+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, );
> >>+if (ret != EOK && ret != ENOENT) {
> >>+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
> >>failed.\n");
> >>+goto done;
> >>+}
> >
> >The call stack here would be
> >
> >sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
> >
> >
> >and the returned results, starting from ldb_search()
> >
> >ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
> >
> >I think it would be easier and safe some allocations if it would be
> >
> >sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
> >
> >Do you think this change makes sense?
> 
> Hi, thanks for the review. Done.
> 
> Although I created a new function sysdb_search_user_by_upn_res since you
> can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
> api).
> 
> >
> const char *orig_name;
> >
> >Instead of adding a new member which again will hold the originally
> >provided name I would suggest to refactor the change added by
> >e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
> >needed) by introducing a member called e.g. parsed_name which keeps the
> >name returned by sss_parse_inp_recv() and return it in cache_req_recv().
> >
> >In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> >orig_name because no additional member was needed. But now I think the
> >code would be more clear if orig_name will be kept unmodified.
> 
> Done in separate patch so the changes are more visible. It can be
> squashed to the second patch.
> >>>
> >>>Sumit found that test won't pass. New patchset is attached.
> >>>
> >>>
> >>
> >>Thank you, the tests pass now. I've seen a compiler warning which is
> >>seen with the first patch I attached.
> >>
> >>During testing I found that UPN lookups for sub-domain users do not
> >>work even with the original nss responder code. I attached fixes for
> >>your patches and the nss responder since the sysdb change depends on you
> >>patches.
> >
> >and now with patches ...
> 
> Ack. Thank you!

I pushed both sets to master:
* 8ded8b2f4a57d1833fd230307218d8b07a571785
* 374268c5eda35e8bbc2fef30752299199439cffe
* 391b81f2a78a812a87530e0c50c70d59150f49eb
* 2fce47f2dadd10d2a2c8bf9f03ab7094bc6c6b3a
* 3688374991afb34bbaf2b7843683fc13dd77879d
* 28ebfa4373d1e7ce45b5d70a3619df1c074a661e
* d8125f0e0d38c6939887a0849a44859d6c498c57
and sssd-1-13:
* b1b0abe6223348083be552531f195518b0df3ba5
* f2c3994c6ffd6e715621d7d7ee580db3cd3e85a9
* 055248cf20c1ad9dfba06903789e71b88752b810
* 6bb2a01333bca8a338cb0be6e8d513040b227cb8
* 44415c5a8ad054555a5e0cc016eec8a125c927ab
* f04ead531a4e941de1a5bcb695326ca11a3dd145
* ae3896a12fb4818aac11ccc5d3a57d2cdf61cd43

I hope it's OK to push these all to sssd-1-13 since Sumit's patches
depend on Pavel's and in 1-13 the cache_req is mostly used for UPN.

Re: [SSSD] [PATCH] cache_req: support UPN

[Sumit Bose]

On Wed, Oct 07, 2015 at 04:22:31PM +0200, Sumit Bose wrote:
> On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
> > On 10/06/2015 03:16 PM, Pavel Březina wrote:
> > >On 10/01/2015 05:06 PM, Sumit Bose wrote:
> > >>On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
> > >>>On 09/17/2015 10:32 AM, Petr Cech wrote:
> > Hi Pavel!
> > 
> > There is some code between my last end and this continuation. I was
> > read
> > it and did't find anything wrong.
> > >>>
> > >>>Hi Petr,
> > >>>thank you for you review. New patches are attached. All comments from
> > >>>the
> > >>>previous mail should be addressed.
> > >>>
> > >>
> > >>
> > >>[PATCH 2/3] cache_req: add support for UPN
> > >>
> > >>>diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> > >>>index
> > >>>5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
> > >>>100644
> > >>>--- a/src/db/sysdb_search.c
> > >>>+++ b/src/db/sysdb_search.c
> > >>>@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
> > >>>  return filter;
> > >>>  }
> > >>>
> > >>>+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
> > >>>+   struct sss_domain_info *domain,
> > >>>+   const char *upn,
> > >>>+   struct ldb_result **_res)
> > >>>+{
> > >>>+TALLOC_CTX *tmp_ctx;
> > >>>+struct ldb_message *msg;
> > >>>+struct ldb_result *res;
> > >>>+static const char *attrs[] = SYSDB_PW_ATTRS;
> > >>>+errno_t ret;
> > >>>+
> > >>>+tmp_ctx = talloc_new(NULL);
> > >>>+if (tmp_ctx == NULL) {
> > >>>+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> > >>>+return ENOMEM;
> > >>>+}
> > >>>+
> > >>>+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, );
> > >>>+if (ret != EOK && ret != ENOENT) {
> > >>>+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
> > >>>failed.\n");
> > >>>+goto done;
> > >>>+}
> > >>
> > >>The call stack here would be
> > >>
> > >>sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
> > >>
> > >>
> > >>and the returned results, starting from ldb_search()
> > >>
> > >>ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
> > >>
> > >>I think it would be easier and safe some allocations if it would be
> > >>
> > >>sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
> > >>
> > >>Do you think this change makes sense?
> > >
> > >Hi, thanks for the review. Done.
> > >
> > >Although I created a new function sysdb_search_user_by_upn_res since you
> > >can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
> > >api).
> > >
> > >>
> > >const char *orig_name;
> > >>
> > >>Instead of adding a new member which again will hold the originally
> > >>provided name I would suggest to refactor the change added by
> > >>e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
> > >>needed) by introducing a member called e.g. parsed_name which keeps the
> > >>name returned by sss_parse_inp_recv() and return it in cache_req_recv().
> > >>
> > >>In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> > >>orig_name because no additional member was needed. But now I think the
> > >>code would be more clear if orig_name will be kept unmodified.
> > >
> > >Done in separate patch so the changes are more visible. It can be
> > >squashed to the second patch.
> > 
> > Sumit found that test won't pass. New patchset is attached.
> > 
> > 
> 
> Thank you, the tests pass now. I've seen a compiler warning which is
> seen with the first patch I attached.
> 
> During testing I found that UPN lookups for sub-domain users do not
> work even with the original nss responder code. I attached fixes for
> your patches and the nss responder since the sysdb change depends on you
> patches.

and now with patches ...

> 
> bye,
> Sumit
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 617c1a39fab16d12f9a346073eaee4a8bee4ac08 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 7 Oct 2015 16:11:56 +0200
Subject: [PATCH 1/3] fix ldb_search usage

---
 src/db/sysdb_ops.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 
5c086cb0a25b452a38ca717b2459c76fdb3f22ff..fafa0207cd068b22151f805a217e2cd55b357d17
 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -483,7 +483,6 @@ int sysdb_search_user_by_upn_res(TALLOC_CTX *mem_ctx,
 TALLOC_CTX *tmp_ctx;
 struct ldb_result *res;
 struct ldb_dn *base_dn;
-char *filter;
 int ret;
 const char *def_attrs[] = { SYSDB_NAME, SYSDB_UPN, SYSDB_CANONICAL_UPN,
 NULL };
@@ -500,15 +499,9 @@ int sysdb_search_user_by_upn_res(TALLOC_CTX *mem_ctx,
 goto done;
 }
 
-filter = 

Re: [SSSD] [PATCH] cache_req: support UPN

[Sumit Bose]

On Wed, Oct 07, 2015 at 02:42:18PM +0200, Pavel Březina wrote:
> On 10/06/2015 03:16 PM, Pavel Březina wrote:
> >On 10/01/2015 05:06 PM, Sumit Bose wrote:
> >>On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:
> >>>On 09/17/2015 10:32 AM, Petr Cech wrote:
> Hi Pavel!
> 
> There is some code between my last end and this continuation. I was
> read
> it and did't find anything wrong.
> >>>
> >>>Hi Petr,
> >>>thank you for you review. New patches are attached. All comments from
> >>>the
> >>>previous mail should be addressed.
> >>>
> >>
> >>
> >>[PATCH 2/3] cache_req: add support for UPN
> >>
> >>>diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
> >>>index
> >>>5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
> >>>100644
> >>>--- a/src/db/sysdb_search.c
> >>>+++ b/src/db/sysdb_search.c
> >>>@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
> >>>  return filter;
> >>>  }
> >>>
> >>>+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
> >>>+   struct sss_domain_info *domain,
> >>>+   const char *upn,
> >>>+   struct ldb_result **_res)
> >>>+{
> >>>+TALLOC_CTX *tmp_ctx;
> >>>+struct ldb_message *msg;
> >>>+struct ldb_result *res;
> >>>+static const char *attrs[] = SYSDB_PW_ATTRS;
> >>>+errno_t ret;
> >>>+
> >>>+tmp_ctx = talloc_new(NULL);
> >>>+if (tmp_ctx == NULL) {
> >>>+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> >>>+return ENOMEM;
> >>>+}
> >>>+
> >>>+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, );
> >>>+if (ret != EOK && ret != ENOENT) {
> >>>+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn()
> >>>failed.\n");
> >>>+goto done;
> >>>+}
> >>
> >>The call stack here would be
> >>
> >>sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()
> >>
> >>
> >>and the returned results, starting from ldb_search()
> >>
> >>ldb_result -> ldb_message (array) -> ldb_message -> ldb_result
> >>
> >>I think it would be easier and safe some allocations if it would be
> >>
> >>sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .
> >>
> >>Do you think this change makes sense?
> >
> >Hi, thanks for the review. Done.
> >
> >Although I created a new function sysdb_search_user_by_upn_res since you
> >can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with
> >api).
> >
> >>
> >const char *orig_name;
> >>
> >>Instead of adding a new member which again will hold the originally
> >>provided name I would suggest to refactor the change added by
> >>e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
> >>needed) by introducing a member called e.g. parsed_name which keeps the
> >>name returned by sss_parse_inp_recv() and return it in cache_req_recv().
> >>
> >>In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
> >>orig_name because no additional member was needed. But now I think the
> >>code would be more clear if orig_name will be kept unmodified.
> >
> >Done in separate patch so the changes are more visible. It can be
> >squashed to the second patch.
> 
> Sumit found that test won't pass. New patchset is attached.
> 
> 

Thank you, the tests pass now. I've seen a compiler warning which is
seen with the first patch I attached.

During testing I found that UPN lookups for sub-domain users do not
work even with the original nss responder code. I attached fixes for
your patches and the nss responder since the sysdb change depends on you
patches.

bye,
Sumit
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Pavel Březina]

On 10/01/2015 05:06 PM, Sumit Bose wrote:

On Thu, Sep 17, 2015 at 11:06:30AM +0200, Pavel Březina wrote:

On 09/17/2015 10:32 AM, Petr Cech wrote:

Hi Pavel!

There is some code between my last end and this continuation. I was read
it and did't find anything wrong.


Hi Petr,
thank you for you review. New patches are attached. All comments from the
previous mail should be addressed.




[PATCH 2/3] cache_req: add support for UPN


diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index 
5f33b225a31e1c1add7f776215d500fb09e127ca..a72a8bac594dc10ff9aa337399f27d9936830a8e
 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -295,6 +295,59 @@ static char *enum_filter(TALLOC_CTX *mem_ctx,
  return filter;
  }

+int sysdb_getpwupn(TALLOC_CTX *mem_ctx,
+   struct sss_domain_info *domain,
+   const char *upn,
+   struct ldb_result **_res)
+{
+TALLOC_CTX *tmp_ctx;
+struct ldb_message *msg;
+struct ldb_result *res;
+static const char *attrs[] = SYSDB_PW_ATTRS;
+errno_t ret;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
+return ENOMEM;
+}
+
+ret = sysdb_search_user_by_upn(tmp_ctx, domain, upn, attrs, );
+if (ret != EOK && ret != ENOENT) {
+DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_user_by_upn() failed.\n");
+goto done;
+}


The call stack here would be

sysdb_getpwupn()->sysdb_search_user_by_upn()->sysdb_search_entry()->ldb_search()

and the returned results, starting from ldb_search()

ldb_result -> ldb_message (array) -> ldb_message -> ldb_result

I think it would be easier and safe some allocations if it would be

sysdb_search_user_by_upn()->sysdb_getpwupn()->ldb_search() .

Do you think this change makes sense?


Hi, thanks for the review. Done.

Although I created a new function sysdb_search_user_by_upn_res since you 
can't pass attrs into sysdb_getpwnupn (unless we are inconsistent with api).





   const char *orig_name;


Instead of adding a new member which again will hold the originally
provided name I would suggest to refactor the change added by
e87b2a6e94c1066b3044fe683825ff5b4f8716c2 (cache_req: parse input name if
needed) by introducing a member called e.g. parsed_name which keeps the
name returned by sss_parse_inp_recv() and return it in cache_req_recv().

In e87b2a6e94c1066b3044fe683825ff5b4f8716c2 it made sense to overwrite
orig_name because no additional member was needed. But now I think the
code would be more clear if orig_name will be kept unmodified.


Done in separate patch so the changes are more visible. It can be 
squashed to the second patch.





bye,
Sumit
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



>From a4cb7fb90fb21d435c00a4951deedddf5bdb259c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Thu, 7 May 2015 13:01:44 +0200
Subject: [PATCH 1/4] cache_req: provide extra flag for oob request

---
 src/responder/common/responder_cache_req.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/responder/common/responder_cache_req.c b/src/responder/common/responder_cache_req.c
index d0a90d2c94b7f990a46af488a08dd386854384e0..fba5001476481040ed962dbbd9b01cc16fe0ba74 100644
--- a/src/responder/common/responder_cache_req.c
+++ b/src/responder/common/responder_cache_req.c
@@ -615,6 +615,11 @@ static errno_t cache_req_cache_check(struct tevent_req *req)
 if (state->input->type == CACHE_REQ_USER_BY_CERT) {
 search_str = state->input->cert;
 }
+
+if (DOM_HAS_VIEWS(state->input->domain)) {
+extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
+}
+
 switch (ret) {
 case EOK:
 DEBUG(SSSDBG_TRACE_FUNC, "Cached entry is valid, returning...\n");
@@ -629,7 +634,7 @@ static errno_t cache_req_cache_check(struct tevent_req *req)
  state->input->domain, true,
  state->input->dp_type,
  search_str,
- state->input->id, NULL);
+ state->input->id, extra_flag);
 if (subreq == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory sending out-of-band "
"data provider request\n");
@@ -643,10 +648,6 @@ static errno_t cache_req_cache_check(struct tevent_req *req)
 /* Cache miss or the cache is expired. We need to get the updated
  * information before returning it. */
 
-if (DOM_HAS_VIEWS(state->input->domain)) {
-extra_flag = EXTRA_INPUT_MAYBE_WITH_VIEW;
-}
-
 subreq = sss_dp_get_account_send(state, state->rctx,
  

Re: [SSSD] [PATCH] cache_req: support UPN

[Pavel Březina]

On 09/17/2015 11:06 AM, Pavel Březina wrote:

On 09/17/2015 10:32 AM, Petr Cech wrote:

Hi Pavel!

There is some code between my last end and this continuation. I was read
it and did't find anything wrong.


Hi Petr,
thank you for you review. New patches are attached. All comments from
the previous mail should be addressed.


+#define run_cache_req(ctx, send_fn, done_fn, dom, crp, lookup, expret)
do { \
+TALLOC_CTX
*req_mem_ctx;\
+struct tevent_req
*req; \
+errno_t
ret;\
+
\
+req_mem_ctx =
talloc_new(global_talloc_context);\
+
check_leaks_push(req_mem_ctx);
\
+
\
+req = send_fn(req_mem_ctx, ctx->tctx->ev,
ctx->rctx,\
+  ctx->ncache, 10,
crp, \
+  (dom == NULL ? NULL : dom->name),
lookup);\
+
assert_non_null(req);
\
+tevent_req_set_callback(req, done_fn,
ctx); \
+
\
+ret =
test_ev_loop(ctx->tctx);  \
+assert_int_equal(ret,
expret);  \
+
assert_true(check_leaks_pop(req_mem_ctx));
\
+
\
+
talloc_free(req_mem_ctx);
\
+} while (0)

This definition should be a function. I found that you use it like


I wanted to avoid writing types for send_fn and done_fn. I'll do it if
you want me to, but I think this is good enough for test.


# return run_cache_req(...) but it doesn't provide value.


It was there originally then I realized I don't have to return a value.
I forgot to switch errno_t with void in function definition though, thanks!

However, I did not find line "return run_cache_req...", so if it is
there somewhere, please tell me.


+
+static void run_user_by_name(struct cache_req_test_ctx *test_ctx,
+ struct sss_domain_info *domain,
+ int cache_refresh_percent,
+ errno_t exp_ret)
+{
+run_cache_req(test_ctx, cache_req_user_by_name_send,
+  cache_req_user_by_name_test_done, domain,
+  cache_refresh_percent, TEST_USER_NAME, exp_ret);
+}
+
+static errno_t run_user_by_upn(struct cache_req_test_ctx *test_ctx,
+   struct sss_domain_info *domain,
+   int cache_refresh_percent,
+   errno_t exp_ret)
+{
+run_cache_req(test_ctx, cache_req_user_by_name_send,
+  cache_req_user_by_name_test_done, domain,
+  cache_refresh_percent, TEST_UPN, exp_ret);
+}

This function returns errno_t but run_cache_req is
without return statement.

I tried with your patches
# make responder_cache_req-tests
and thre is a result:

src/tests/cmocka/test_responder_cache_req.c: In function
‘run_user_by_upn’:
src/tests/cmocka/test_responder_cache_req.c:199:1: warning: no return
statement in function returning non-void [-Wreturn-type]
  }
  ^
src/tests/cmocka/test_responder_cache_req.c: In function
‘run_user_by_id’:
src/tests/cmocka/test_responder_cache_req.c:209:1: warning: no return
statement in function returning non-void [-Wreturn-type]
  }
  ^
src/tests/cmocka/test_responder_cache_req.c: In function
‘run_group_by_name’:
src/tests/cmocka/test_responder_cache_req.c:261:1: warning: no return
statement in function returning non-void [-Wreturn-type]
  }
  ^
src/tests/cmocka/test_responder_cache_req.c: In function
‘run_group_by_id’:
src/tests/cmocka/test_responder_cache_req.c:271:1: warning: no return
statement in function returning non-void [-Wreturn-type]
  }


See above.



That's all for me. I didn't test the functionality. Please, ask someone
other who has skills with AD (and running instance of it) to test it.

Regards

Petr


Bump.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Petr Cech]

On 09/17/2015 11:06 AM, Pavel Březina wrote:


I wanted to avoid writing types for send_fn and done_fn. I'll do it if
you want me to, but I think this is good enough for test.


Well.

# return run_cache_req(...) but it doesn't provide value.


It was there originally then I realized I don't have to return a value.
I forgot to switch errno_t with void in function definition though, thanks!

However, I did not find line "return run_cache_req...", so if it is
there somewhere, please tell me.
That's my fault. There isn't any return run_cache_req(...) but I think 
that should be.


Thanks for addressing all comments (I didn't check it.).

Petr
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Petr Cech]

Hi Pavel!

There is some code between my last end and this continuation. I was read 
it and did't find anything wrong.


On 09/16/2015 04:26 PM, Petr Cech wrote:

0003-cache_req-tests-reduce-code-duplication.patch


  From e41f96a47f2b0f8d3e07e34af83e9a516d29df34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
Date: Mon, 14 Sep 2015 11:06:45 +0200
Subject: [PATCH 3/3] cache_req tests: reduce code duplication

---
   src/tests/cmocka/test_responder_cache_req.c | 1624
+++
   1 file changed, 394 insertions(+), 1230 deletions(-)

diff --git a/src/tests/cmocka/test_responder_cache_req.c
b/src/tests/cmocka/test_responder_cache_req.c
index
7db87ccc816ea0e30e707ec8c2fa4666441892a8..2af481494319d0d01d29d0c243020c5adcb06d3a
100644
--- a/src/tests/cmocka/test_responder_cache_req.c
+++ b/src/tests/cmocka/test_responder_cache_req.c
@@ -52,6 +52,27 @@
   test_multi_domain_setup, \
   test_multi_domain_teardown)

+#define run_cache_req(ctx, send_fn, done_fn, dom, crp, lookup, expret)
do { \
+TALLOC_CTX
*req_mem_ctx;\
+struct tevent_req
*req; \
+errno_t
ret;\
+
\
+req_mem_ctx =
talloc_new(global_talloc_context);\
+
check_leaks_push(req_mem_ctx);  \
+
\
+req = send_fn(req_mem_ctx, ctx->tctx->ev,
ctx->rctx,\
+  ctx->ncache, 10,
crp, \
+  (dom == NULL ? NULL : dom->name),
lookup);\
+
assert_non_null(req);   \
+tevent_req_set_callback(req, done_fn,
ctx); \
+
\
+ret =
test_ev_loop(ctx->tctx);  \
+assert_int_equal(ret,
expret);  \
+
assert_true(check_leaks_pop(req_mem_ctx));  \
+
\
+
talloc_free(req_mem_ctx);   \
+} while (0)

This definition should be a function. I found that you use it like
# return run_cache_req(...) but it doesn't provide value.


+
   struct cache_req_test_ctx {
   struct sss_test_ctx *tctx;
   struct resp_ctx *rctx;
@@ -80,46 +101,6 @@ struct cli_protocol_version
*register_cli_protocol_version(void)
   return version;
   }

-struct tevent_req *
-__wrap_sss_dp_get_account_send(TALLOC_CTX *mem_ctx,
-   struct resp_ctx *rctx,
-   struct sss_domain_info *dom,
-   bool fast_reply,
-   enum sss_dp_acct_type type,
-   const char *opt_name,
-   uint32_t opt_id,
-   const char *extra)
-{
-struct sysdb_attrs *attrs = NULL;
-struct cache_req_test_ctx *ctx = NULL;
-errno_t ret;
-
-ctx = sss_mock_ptr_type(struct cache_req_test_ctx*);
-ctx->dp_called = true;
-
-if (ctx->create_user) {
-attrs = sysdb_new_attrs(ctx);
-assert_non_null(attrs);
-
-ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, TEST_UPN);
-assert_int_equal(ret, EOK);
-
-ret = sysdb_store_user(ctx->tctx->dom, TEST_USER_NAME, "pwd",
-   TEST_USER_ID, 1000, NULL, NULL, NULL,
-   "cn=test-user,dc=test", attrs, NULL,
-   1000, time(NULL));
-assert_int_equal(ret, EOK);
-}
-
-if (ctx->create_group) {
-ret = sysdb_store_group(ctx->tctx->dom, TEST_GROUP_NAME,
-TEST_GROUP_ID, NULL, 1000, time(NULL));
-assert_int_equal(ret, EOK);
-}
-
-return test_req_succeed_send(mem_ctx, rctx->ev);
-}
-
   static void cache_req_user_by_name_test_done(struct tevent_req *req)
   {
   struct cache_req_test_ctx *ctx = NULL;
@@ -176,6 +157,173 @@ static void cache_req_group_by_id_test_done(struct
tevent_req *req)
   ctx->tctx->done = true;
   }

+static void prepare_user(TALLOC_CTX *mem_ctx,
+ struct sss_domain_info *domain,
+ uint64_t timeout,
+ time_t time)
+{
+struct sysdb_attrs *attrs;
+errno_t ret;
+
+attrs = sysdb_new_attrs(mem_ctx);
+assert_non_null(attrs);
+
+ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, TEST_UPN);
+assert_int_equal(ret, EOK);
+
+ret = sysdb_store_user(domain, TEST_USER_NAME, "pwd",
+   TEST_USER_ID, TEST_GROUP_ID, NULL, NULL, NULL,
+   "cn=test-user,dc=test", attrs, NULL,
+   timeout, time);
+assert_int_equal(ret, EOK);
+}
+
+static void 

Re: [SSSD] [PATCH] cache_req: support UPN

[Petr Cech]

On 09/14/2015 01:34 PM, Pavel Březina wrote:

On 09/14/2015 01:32 PM, Pavel Březina wrote:

0001:
Use extra flag also in OOB request.

0002:
Provide support for UPN. This add an improvement from NSS code, but I'm
not sure if it is desired or not.

If you have [domain/AD.PB] in sssd.conf and UPN "u...@ad.pb" then NSS
responder will not find this user, cache_req will. Is this nss behavior
intentional or a bug?

0003:
I got really sick of the way new test are written in cache_req when
writing new tests so I kinda rewrote it.


I think this completes the cache_req interface. If you find anything
missing, please let me no so I can add it.


Hi,

I compiled it.
CI tests over all 3 patches:
http://sssd-ci.duckdns.org/logs/job/26/73/summary.html

I was interested in the third patch, since it affects the tests, with 
which I have worked. This is something what I will inspect more detail.


I cannot say ack, because there is large logic.
I would like to ask someone more experienced
to take care of this review.

Petr

PS: I installed AD on my laptop and I try to set up
it correctly.

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Jakub Hrozek]

On Wed, Sep 16, 2015 at 03:50:37PM +0200, Petr Cech wrote:
> On 09/14/2015 01:34 PM, Pavel Březina wrote:
> >On 09/14/2015 01:32 PM, Pavel Březina wrote:
> >>0001:
> >>Use extra flag also in OOB request.
> >>
> >>0002:
> >>Provide support for UPN. This add an improvement from NSS code, but I'm
> >>not sure if it is desired or not.
> >>
> >>If you have [domain/AD.PB] in sssd.conf and UPN "u...@ad.pb" then NSS
> >>responder will not find this user, cache_req will. Is this nss behavior
> >>intentional or a bug?
> >>
> >>0003:
> >>I got really sick of the way new test are written in cache_req when
> >>writing new tests so I kinda rewrote it.
> >
> >I think this completes the cache_req interface. If you find anything
> >missing, please let me no so I can add it.
> >
> Hi,
> 
> I compiled it.
> CI tests over all 3 patches:
> http://sssd-ci.duckdns.org/logs/job/26/73/summary.html
> 
> I was interested in the third patch, since it affects the tests, with which
> I have worked. This is something what I will inspect more detail.
> 
> I cannot say ack, because there is large logic.
> I would like to ask someone more experienced
> to take care of this review.

Thank you, please don't hesitate to just note anything strange you
notice in the patchset -- you don't have to review it all!
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] cache_req: support UPN

[Pavel Březina]

On 09/14/2015 01:32 PM, Pavel Březina wrote:

0001:
Use extra flag also in OOB request.

0002:
Provide support for UPN. This add an improvement from NSS code, but I'm
not sure if it is desired or not.

If you have [domain/AD.PB] in sssd.conf and UPN "u...@ad.pb" then NSS
responder will not find this user, cache_req will. Is this nss behavior
intentional or a bug?

0003:
I got really sick of the way new test are written in cache_req when
writing new tests so I kinda rewrote it.


I think this completes the cache_req interface. If you find anything 
missing, please let me no so I can add it.


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel