On (01/06/15 10:24), Sumit Bose wrote:
>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.
>
>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
>---
Thank you for changes
and we should backport the patch to downstream.
http://sssd-ci.duckdns.org/logs/job/16/48/summary.html
ACK
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
