Hi, these two patches fixes and issue which was reported on https://www.redhat.com/archives/freeipa-users/2016-February/msg00148.html and is now tracked in https://fedorahosted.org/sssd/ticket/2960 .
The first patch makes sure the view name is read even if there is an error with the master domain lookup. The second patch makes sure that SSSD will pick up the new override data immediately after the upgrade without additional changes. This change is needed because currently SSSD assumes that the cache was empty if no view name was read from the cache and does not cleanup the override data. But if a version of SSSD with the issue fixed by the first patch was run before the cache will contain the override data for the default view which most be removed to make sure SSSD will read the new override data. To reproduce use a FreeIPA where ipa-adtrust-install was not run, add a new idview and assign it to an IPA client. Without the patch the IPA client will continue to show the default IPA user data and no override value. bye, Sumit
From f6bc534c6b82445e5e29a1e80768657c06d0557b Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 18 Feb 2016 13:03:44 +0100 Subject: [PATCH 1/2] IPA: lookup idview name even if there is no master domain record Currently the IPA subdomain provider returns with a error if there is no master domain record found. Since this record contains data which is only needed to create a trust with AD, like e.g. the IPA domain SID, this record is only created by ipa-adtrust-install. But the idview name is read after the master domain record. To make the idview feature work with a plain FreeIPA setup without running ipa-adtrust-install the missing master domain record should be handled gracefully and the following lookup should run as well. Resolves https://fedorahosted.org/sssd/ticket/2960 --- src/providers/ipa/ipa_subdomains.c | 80 +++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index 8bbbad0ab6d12179d34402ab8ebff2a1b782760c..20b1c9cbff874e9f02f5db1cd0fcf6f9cfd746fc 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -1219,6 +1219,9 @@ static void ipa_subdomains_handler_master_done(struct tevent_req *req) size_t reply_count = 0; struct sysdb_attrs **reply = NULL; struct ipa_subdomains_req_ctx *ctx; + const char *flat = NULL; + const char *id = NULL; + const char *realm = NULL; ctx = tevent_req_callback_data(req, struct ipa_subdomains_req_ctx); @@ -1230,10 +1233,6 @@ static void ipa_subdomains_handler_master_done(struct tevent_req *req) } if (reply_count) { - const char *flat = NULL; - const char *id = NULL; - const char *realm; - ret = sysdb_attrs_get_string(reply[0], IPA_FLATNAME, &flat); if (ret != EOK) { goto done; @@ -1244,31 +1243,9 @@ static void ipa_subdomains_handler_master_done(struct tevent_req *req) goto done; } - realm = dp_opt_get_string(ctx->sd_ctx->id_ctx->ipa_options->basic, - IPA_KRB5_REALM); - if (realm == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "No Kerberos realm for IPA?\n"); - ret = EINVAL; - goto done; - } - - ret = sysdb_master_domain_add_info(ctx->sd_ctx->be_ctx->domain, - realm, flat, id, NULL); - if (ret != EOK) { - goto done; - } - /* There is only one master record. Don't bother checking other IPA * search bases; move to checking subdomains instead */ - ret = ipa_subdomains_handler_get_start(ctx, - ctx->sd_ctx->search_bases, - IPA_SUBDOMAINS_SLAVE); - if (ret == EAGAIN) { - return; - } - - /* Either no search bases or an error. End the request in both cases */ } else { ret = ipa_subdomains_handler_get_cont(ctx, IPA_SUBDOMAINS_MASTER); if (ret == EAGAIN) { @@ -1277,17 +1254,48 @@ static void ipa_subdomains_handler_master_done(struct tevent_req *req) goto done; } - /* Right now we know there has been an error - * and we don't have the master domain record - */ - DEBUG(SSSDBG_CRIT_FAILURE, "Master domain record not found!\n"); - - if (!ctx->sd_ctx->configured_explicit) { - ctx->sd_ctx->disabled_until = time(NULL) + - IPA_SUBDOMAIN_DISABLED_PERIOD; + /* All search paths are searched and no master domain record was + * found. + * + * A default IPA installation will not have a master domain record, + * this is only created by ipa-adtrust-install. Nevertheless we should + * continue to read other data like the idview on IPA clients. */ + + DEBUG(SSSDBG_TRACE_INTERNAL, "Master domain record not found!\n"); + + } + + realm = dp_opt_get_string(ctx->sd_ctx->id_ctx->ipa_options->basic, + IPA_KRB5_REALM); + if (realm == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "No Kerberos realm for IPA?\n"); + ret = EINVAL; + goto done; + } + + ret = sysdb_master_domain_add_info(ctx->sd_ctx->be_ctx->domain, + realm, flat, id, NULL); + if (ret != EOK) { + goto done; + } + + ret = ipa_subdomains_handler_get_start(ctx, + ctx->sd_ctx->search_bases, + IPA_SUBDOMAINS_SLAVE); + if (ret == EAGAIN) { + return; + } else if (ret == EOK) { + /* If there are no search bases defined for subdomains try to get the + * idview before ending the request */ + if (ctx->sd_ctx->id_ctx->server_mode == NULL) { + /* Only get view on clients, on servers it is always 'default' */ + ret = ipa_get_view_name(ctx); + if (ret == EAGAIN) { + return; + } else if (ret != EOK) { + goto done; + } } - - ret = EIO; } done: -- 2.1.0
From 294c817e15d4e8ad3ab24d8a0875fec9fe0434a8 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Mon, 22 Feb 2016 16:08:13 +0100 Subject: [PATCH 2/2] IPA: invalidate override data if original view is missing If the idview name cannot be read from cache this either means that the cache was empty or the name wasn't written because of an error. In the case of an error SSSD would assume that the default view was used. If the new view is different from the default view the override data must be invalidated. Since the sysdb call to invalidate the override data would work with an empty cache as well and do nothing it is safe to call it on both cases. Related to https://fedorahosted.org/sssd/ticket/2960 --- src/providers/ipa/ipa_subdomains.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index 20b1c9cbff874e9f02f5db1cd0fcf6f9cfd746fc..bcc6347fa0c53939bb60cff2e903e1db6e2bb6b6 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -898,9 +898,19 @@ static void ipa_get_view_name_done(struct tevent_req *req) } else { if (ctx->sd_ctx->id_ctx->view_name == NULL || strcmp(ctx->sd_ctx->id_ctx->view_name, view_name) != 0) { - /* View name changed */ + /* View name changed. If there was a non-default non-local view + * was used the tree in cache containing the override values is + * removed. In all cases sysdb_invalidate_overrides() is called to + * remove the override attribute from the cached user objects. + * + * Typically ctx->sd_ctx->id_ctx->view_name == NULL means that the + * cache was empty but there was a bug in with caused that the + * view name was not written to the cache at all. In this case the + * cache must be invalidated if the new view is not the + * default-view as well. */ - if (ctx->sd_ctx->id_ctx->view_name != NULL) { + if (ctx->sd_ctx->id_ctx->view_name != NULL + || !is_default_view(view_name)) { ret = sysdb_transaction_start( ctx->sd_ctx->be_ctx->domain->sysdb); if (ret != EOK) { -- 2.1.0
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
