[SSSD] [PATCH] SUBDOMAINS: Reuse cached results if DP is offline

Sumit Bose sbose at redhat.com
Thu Dec 5 13:17:58 UTC 2013


On Thu, Dec 05, 2013 at 02:09:05PM +0100, Jakub Hrozek wrote:
> On Wed, Dec 04, 2013 at 05:25:48PM +0100, Sumit Bose wrote:
> > On Tue, Dec 03, 2013 at 03:49:56PM +0100, Jakub Hrozek wrote:
> > > On Fri, Nov 29, 2013 at 12:11:03PM +0100, Jakub Hrozek wrote:
> > > > On Fri, Nov 29, 2013 at 12:06:37PM +0100, Sumit Bose wrote:
> > > > > On Thu, Nov 28, 2013 at 04:02:21PM +0100, Jakub Hrozek wrote:
> > > > > > Please see the simple attached patch. To reproduce, start sssd in
> > > > > > offline mode and attempt to authenticate as subdomain user.
> > > > > 
> > > > > The patch is working as expected. I wonder if it would be better to call
> > > > > sysdb_update_subdomains() during the initialization of the subdomain
> > > > > target?
> > > > 
> > > > I think you're right, that would protect against any other error as
> > > > well. I'll prepare a new version, thanks for the review.
> > > 
> > > Attached is a new patch. I also filed a ticked, because I suspect we'll
> > > want to have the same fix in downstream distributions as well.
> > 
> > thanks, offline auth is still working with the new version. I just have
> > one comment in-line.
> > 
> > > From 2df203315bc31bab5763d2971928ea94057d1c82 Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek <jhrozek at redhat.com>
> > > Date: Tue, 3 Dec 2013 15:25:25 +0100
> > > Subject: [PATCH] SUBDOMAINS: Reuse cached results if DP is offline
> > > 
> > > If Data Provider was unable to refresh the subdomain list, the
> > > sss_domain_info->subdomains list was NULL. Which meant that no DP
> > > request matched any known domain and hence offline authentication was
> > > not working correctly.
> > > 
> > > Resolves:
> > > https://fedorahosted.org/sssd/ticket/2168
> > > ---
> > >  src/providers/ad/ad_subdomains.c   | 6 ++++++
> > >  src/providers/ipa/ipa_subdomains.c | 6 ++++++
> > >  2 files changed, 12 insertions(+)
> > > 
> > > diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
> > > index 73190faa1e7e995123f12e2200ab00fb40f3b256..282875ad38f3d0d3f4b6a3026fa2461f9af6da1f 100644
> > > --- a/src/providers/ad/ad_subdomains.c
> > > +++ b/src/providers/ad/ad_subdomains.c
> > > @@ -652,5 +652,11 @@ int ad_subdom_init(struct be_ctx *be_ctx,
> > >          return EFAULT;
> > >      }
> > >  
> > > +    ret = sysdb_update_subdomains(be_ctx->domain);
> > > +    if (ret != EOK) {
> > > +        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
> > > +        return ret;
> > 
> > Since this is only needed if sssd starts offline I wonder if we should
> > fail here or, or if it would be better to just print a more detailed log
> > message at a more important log level explaining what might happen
> > and continue?
> > 
> > > +    }
> > > +
> > >      return EOK;
> > >  }
> > > diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
> > > index 4f7627eddb9c54d68e45be876157057f3c30b422..cc0b5394bb8620abb6da148753c46d38a2eab3fe 100644
> > > --- a/src/providers/ipa/ipa_subdomains.c
> > > +++ b/src/providers/ipa/ipa_subdomains.c
> > > @@ -1291,6 +1291,12 @@ int ipa_subdom_init(struct be_ctx *be_ctx,
> > >          DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to add subdom offline callback"));
> > >      }
> > >  
> > > +    ret = sysdb_update_subdomains(be_ctx->domain);
> > > +    if (ret != EOK) {
> > > +        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
> > > +        return ret;
> > 
> > same here
> > 
> > bye,
> > Sumit
> 
> Good idea, see attached patch. I also removed one FIXME since we do what
> the FIXME describes now.

> From 733083e27cb81c4f41c38b8c80b9fc59da174f1a Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhrozek at redhat.com>
> Date: Tue, 3 Dec 2013 15:25:25 +0100

        ^^^^

looks like you attached the old one.

bye,
Sumit

> Subject: [PATCH] SUBDOMAINS: Reuse cached results if DP is offline
> 
> If Data Provider was unable to refresh the subdomain list, the
> sss_domain_info->subdomains list was NULL. Which meant that no DP
> request matched any known domain and hence offline authentication was
> not working correctly.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2168
> ---
>  src/providers/ad/ad_subdomains.c   | 6 ++++++
>  src/providers/ipa/ipa_subdomains.c | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
> index 73190faa1e7e995123f12e2200ab00fb40f3b256..282875ad38f3d0d3f4b6a3026fa2461f9af6da1f 100644
> --- a/src/providers/ad/ad_subdomains.c
> +++ b/src/providers/ad/ad_subdomains.c
> @@ -652,5 +652,11 @@ int ad_subdom_init(struct be_ctx *be_ctx,
>          return EFAULT;
>      }
>  
> +    ret = sysdb_update_subdomains(be_ctx->domain);
> +    if (ret != EOK) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
> +        return ret;
> +    }
> +
>      return EOK;
>  }
> diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
> index 4f7627eddb9c54d68e45be876157057f3c30b422..cc0b5394bb8620abb6da148753c46d38a2eab3fe 100644
> --- a/src/providers/ipa/ipa_subdomains.c
> +++ b/src/providers/ipa/ipa_subdomains.c
> @@ -1291,6 +1291,12 @@ int ipa_subdom_init(struct be_ctx *be_ctx,
>          DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to add subdom offline callback"));
>      }
>  
> +    ret = sysdb_update_subdomains(be_ctx->domain);
> +    if (ret != EOK) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
> +        return ret;
> +    }
> +
>      return EOK;
>  }
>  
> -- 
> 1.8.4.2
> 

> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list