On Sat, May 30, 2015 at 09:25:58PM +0200, Lukas Slebodnik wrote:
> On (29/05/15 18:20), Sumit Bose wrote:
> >Hi,
> >
> >I came across this issue while testing one of Jakub's patches. You can
> >reproduce this e.g. with the AD provider when joined to a AD forest with
> >multiple domains. Now calling 'id -G' for a user from a sub-domain will
> >first not show all groups only after a second call of 'id -G'. In the
> >domain log object-not-found error messages can be found.
> >
> >bye,
> >Sumit
>
> >From 6f34fdd5426967f7feb11801d3de991cdaa9ee7b Mon Sep 17 00:00:00 2001
> >From: Sumit Bose <sb...@redhat.com>
> >Date: Fri, 29 May 2015 16:37:54 +0200
> >Subject: [PATCH] ldap: use proper sysdb name in groups_by_user_done()
> >
> >In a recent change set_initgroups_expire_attribute() was added to
> >groups_by_user_done() to make sure that the initgroups timeout is only
> >added to the user object until all groups added to the cache.
> >
> >This change (and the original code in groups_by_user_done() as well)
> >didn't took sub-domain users into account where the name in sysdb might
> >different form the original request and the domain is not the configured
> >domain. This patch tries to ensure that the right name and domain are
> >used.
> >---
> > src/providers/ldap/ldap_id.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> >diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
> >index
> >6631237fffb2ac1901b1bbbf27ce6bd1bf48a244..1473ab0e12201840a93b521a4d336e41e919effd
> > 100644
> >--- a/src/providers/ldap/ldap_id.c
> >+++ b/src/providers/ldap/ldap_id.c
> >@@ -1140,6 +1140,7 @@ static void groups_by_user_done(struct tevent_req
> >*subreq)
> > struct
> > groups_by_user_state);
> > int dp_error = DP_ERR_FATAL;
> > int ret;
> >+ char *cname;
> >
> > ret = sdap_get_initgr_recv(subreq);
> > talloc_zfree(subreq);
> >@@ -1163,16 +1164,24 @@ static void groups_by_user_done(struct tevent_req
> >*subreq)
> > return;
> > }
> >
> >+ /* state->name is still the name used for the original request. The
> >cached
> >+ * object might have a different name, e.g. a fully-qualified name. */
> >+ ret = sysdb_get_real_name(state, state->domain, state->name, &cname);
> >+ if (ret != EOK) {
> >+ cname = state->name;
> >+ DEBUG(SSSDBG_OP_FAILURE, "Failed to canonicalize name, using
> >[%s].\n",
> >+ cname);
> >+ }
> >+
> > if (ret == ENOENT && state->noexist_delete == true) {
> >- ret = sysdb_delete_user(state->ctx->be->domain, state->name, 0);
> >+ ret = sysdb_delete_user(state->domain, cname, 0);
> > if (ret != EOK && ret != ENOENT) {
> > tevent_req_error(req, ret);
> > return;
> > }
> > }
> >
> >- ret = set_initgroups_expire_attribute(state->ctx->be->domain,
> >- state->name);
> >+ ret = set_initgroups_expire_attribute(state->domain, cname);
> > if (ret != EOK) {
> > state->dp_error = DP_ERR_FATAL;
> > tevent_req_error(req, ret);
> Bug is fixed,
> but there is a compiler warning
> src/providers/ldap/ldap_id.c: In function 'groups_by_user_done':
> src/providers/ldap/ldap_id.c:1154:5: warning: passing argument 4 of
> 'sysdb_get_real_name' from incompatible pointer type [enabled by default]
> ret = sysdb_get_real_name(state, state->domain, state->name, &cname);
> ^
> In file included from src/providers/ldap/ldap_id.c:31:0:
> ./src/db/sysdb.h:372:9: note: expected 'const char **' but argument is of
> type 'char **'
> errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
> ^
> src/providers/ldap/ldap_id.c:1156:15: warning: assignment discards 'const'
> qualifier from pointer target type [enabled by default]
> cname = state->name;
> ^
>
> Just change type of cname from "char *" -> "const char *"
>
> It would be good to add ticket to commit message
> (related?) https://fedorahosted.org/sssd/ticket/2634
Thank you for the review, I added your comments, new version attached.
bye,
Sumit
>
> LS
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 48910b4fa13bdff962d91becc509dc3aa0c5493d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 29 May 2015 16:37:54 +0200
Subject: [PATCH] ldap: use proper sysdb name in groups_by_user_done()
In a recent change set_initgroups_expire_attribute() was added to
groups_by_user_done() to make sure that the initgroups timeout is only
added to the user object until all groups added to the cache.
This change (and the original code in groups_by_user_done() as well)
didn't took sub-domain users into account where the name in sysdb might
different form the original request and the domain is not the configured
domain. This patch tries to ensure that the right name and domain are
used.
Related to https://fedorahosted.org/sssd/ticket/2634
---
src/providers/ldap/ldap_id.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index
6631237fffb2ac1901b1bbbf27ce6bd1bf48a244..cee30b6267d0d9cecda911ca19bc1533100d894a
100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -1140,6 +1140,7 @@ static void groups_by_user_done(struct tevent_req *subreq)
struct
groups_by_user_state);
int dp_error = DP_ERR_FATAL;
int ret;
+ const char *cname;
ret = sdap_get_initgr_recv(subreq);
talloc_zfree(subreq);
@@ -1163,16 +1164,24 @@ static void groups_by_user_done(struct tevent_req
*subreq)
return;
}
+ /* state->name is still the name used for the original request. The cached
+ * object might have a different name, e.g. a fully-qualified name. */
+ ret = sysdb_get_real_name(state, state->domain, state->name, &cname);
+ if (ret != EOK) {
+ cname = state->name;
+ DEBUG(SSSDBG_OP_FAILURE, "Failed to canonicalize name, using [%s].\n",
+ cname);
+ }
+
if (ret == ENOENT && state->noexist_delete == true) {
- ret = sysdb_delete_user(state->ctx->be->domain, state->name, 0);
+ ret = sysdb_delete_user(state->domain, cname, 0);
if (ret != EOK && ret != ENOENT) {
tevent_req_error(req, ret);
return;
}
}
- ret = set_initgroups_expire_attribute(state->ctx->be->domain,
- state->name);
+ ret = set_initgroups_expire_attribute(state->domain, cname);
if (ret != EOK) {
state->dp_error = DP_ERR_FATAL;
tevent_req_error(req, ret);
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
