On Mon, Dec 16, 2013 at 10:09:58PM +0100, Sumit Bose wrote:
> On Mon, Dec 16, 2013 at 07:05:16PM +0100, Jakub Hrozek wrote:
> > On Mon, Dec 16, 2013 at 07:03:11PM +0100, Jakub Hrozek wrote:
> > > On Mon, Dec 16, 2013 at 12:04:44PM +0100, Sumit Bose wrote:
> > > > On Sat, Dec 14, 2013 at 10:18:32PM +0100, Jakub Hrozek wrote:
> > > > > Hi,
> > > > > 
> > > > > I found this bug when testing the GC patches. Previously, when SSSD 
> > > > > was
> > > > > started, but subdomains list was up-to-date, the ad_ctx was not
> > > > > initialized for the subdomain.
> > > > > 
> > > > > I was also thinking whether we should re-initialize the domain-realm
> > > > > mappings after sssd startup, the way we re-initialize kdcinfo files. I
> > > > > don't think it's strictly necessary because if someone deletes a file 
> > > > > in
> > > > > /var/lib/sss/pubconf, he should keep the broken pieces, but perhaps we
> > > > > should be at least aware..
> > > > 
> > > > Maybe we can do this during the init phase of the responder? We
> 
> ah, sorry, I had the typo here                          ^^^^^^^^ first.
> I meant to call it during the initialization of the subdomain part of
> the AD and IPA providers.
> 
> bye,
> Sumit

That makes perfect sense :-)

See the attached patch, I moved the code to a separate function.
>From ecbace46b35f6e2ee3657a72f95f48bc5f8da157 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Fri, 13 Dec 2013 19:11:47 +0100
Subject: [PATCH] AD: Always refresh LDAP subdomain list

Previously, if no changes were done to the list of subdomains, the SSSD
didn't update its list of sdap_domain mappings for the new subdomain.
This resulted in errors as no id_ctx was present for the subdomain
during lookup.

This patch moves the block of code performed during update to a function
of its own and calls it during provider initialization as well.
---
 src/providers/ad/ad_subdomains.c | 49 ++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 
100fb13e99f7bf4b3946b1f5c5f9c626674bfb46..85872be88fa6b44d817819d7240dbd8cb4ca0c85
 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -414,6 +414,31 @@ done:
     return ret;
 }
 
+static errno_t ad_subdom_reinit(struct ad_subdomains_ctx *ctx)
+{
+    errno_t ret;
+
+    ret = sysdb_update_subdomains(ctx->be_ctx->domain);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+        return ret;
+    }
+
+    ret = sss_write_domain_mappings(ctx->be_ctx->domain, false);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE, ("sss_krb5_write_mappings failed.\n"));
+        /* Just continue */
+    }
+
+    ret = ads_store_sdap_subdom(ctx, ctx->be_ctx->domain);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("ads_store_sdap_subdom failed.\n"));
+        return ret;
+    }
+
+    return ret;
+}
+
 static void ad_subdomains_get_conn_done(struct tevent_req *req);
 static void ad_subdomains_master_dom_done(struct tevent_req *req);
 static errno_t ad_subdomains_get_slave(struct ad_subdomains_req_ctx *ctx);
@@ -619,25 +644,15 @@ static void ad_subdomains_get_slave_domain_done(struct 
tevent_req *req)
         goto done;
     }
 
+    DEBUG(SSSDBG_TRACE_LIBS, ("There are %schanges\n",
+                    refresh_has_changes ? "" : "no "));
+
     if (refresh_has_changes) {
-        ret = sysdb_update_subdomains(ctx->sd_ctx->be_ctx->domain);
+        ret = ad_subdom_reinit(ctx->sd_ctx);
         if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+            DEBUG(SSSDBG_OP_FAILURE, ("Could not reinitialize subdomains\n"));
             goto done;
         }
-
-        ret = ads_store_sdap_subdom(ctx->sd_ctx, ctx->sd_ctx->be_ctx->domain);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("ads_store_sdap_subdom failed.\n"));
-            goto done;
-        }
-
-        ret = sss_write_domain_mappings(ctx->sd_ctx->be_ctx->domain, false);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE,
-                  ("sss_krb5_write_mappings failed.\n"));
-            /* Just continue */
-        }
     }
 
     ret = EOK;
@@ -783,9 +798,9 @@ int ad_subdom_init(struct be_ctx *be_ctx,
         return EFAULT;
     }
 
-    ret = sysdb_update_subdomains(be_ctx->domain);
+    ret = ad_subdom_reinit(ctx);
     if (ret != EOK) {
-        DEBUG(SSSDBG_MINOR_FAILURE, ("Could not load the list of subdomains. "
+        DEBUG(SSSDBG_MINOR_FAILURE, ("Could not reinitialize subdomains. "
               "Users from trusted domains might not be resolved correctly\n"));
         /* Ignore this error and try to discover the subdomains later */
     }
-- 
1.8.4.2

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

Reply via email to