On (24/08/16 11:26), Jakub Hrozek wrote: >On Wed, Aug 24, 2016 at 10:48:21AM +0200, Fabiano Fidêncio wrote: >> On Wed, Aug 24, 2016 at 10:43 AM, Lukas Slebodnik <lsleb...@redhat.com> >> wrote: >> > On (24/08/16 10:37), Fabiano Fidêncio wrote: >> >>On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik <lsleb...@redhat.com> >> >>wrote: >> >>> On (24/08/16 00:59), Fabiano Fidêncio wrote: >> >>>>When saving the user there is a comparison between the "cased alias" >> >>>>and the "lowercase password name". However, the first doesn't use fully >> >>>>qualified name while the second does, resulting in a not expected >> >>>>override of the "nameAlias" attribute of a stored user when trying to >> >>>>authenticate more than once using an alias. >> >>>> >> >>>>Resolves: >> >>>>https://fedorahosted.org/sssd/ticket/3134 >> >>>> >> >>>>CI has passed: http://sssd-ci.duckdns.org/logs/job/52/26/summary.html >> >>>> >> >>>>Best Regards, >> >>>>-- >> >>>>Fabiano Fidêncio >> >>> >> >>> >From a6a38a856a15ec9854797c1df4369288f1bec6c1 Mon Sep 17 00:00:00 2001 >> >>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> >> >>>>Date: Tue, 23 Aug 2016 23:46:59 +0200 >> >>>>Subject: [PATCH] PROXY: Use the fqname when converting to lowercase >> >>>>MIME-Version: 1.0 >> >>>>Content-Type: text/plain; charset=UTF-8 >> >>>>Content-Transfer-Encoding: 8bit >> >>>> >> >>>>When saving the user there is a comparison between the "cased alias" >> >>>>and the "lowercase password name". However, the first doesn't use fully >> >>>>qualified name while the second does, resulting in a not expected >> >>>>override of the "nameAlias" attribute of a stored user when trying to >> >>>>authenticate more than once using an alias. >> >>>> >> >>>>Resolves: >> >>>>https://fedorahosted.org/sssd/ticket/3134 >> >>>> >> >>>>Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> >> >>>>--- >> >>>> src/providers/proxy/proxy_id.c | 2 +- >> >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >>>> >> >>>>diff --git a/src/providers/proxy/proxy_id.c >> >>>>b/src/providers/proxy/proxy_id.c >> >>>>index 26f4d74..b0c8280 100644 >> >>>>--- a/src/providers/proxy/proxy_id.c >> >>>>+++ b/src/providers/proxy/proxy_id.c >> >>>>@@ -256,7 +256,7 @@ static int save_user(struct sss_domain_info *domain, >> >>>> } >> >>>> >> >>>> if (lowercase) { >> >>>>- lc_pw_name = sss_tc_utf8_str_tolower(attrs, pwd->pw_name); >> >>>>+ lc_pw_name = sss_tc_utf8_str_tolower(attrs, real_name); >> >>>> if (lc_pw_name == NULL) { >> >>>> DEBUG(SSSDBG_OP_FAILURE, "Cannot convert name to >> >>>> lowercase.\n"); >> >>>> ret = ENOMEM; >> >>> I thought it would be a one-liner. >> >>> Nice catch and good job. >> >>> >> >>> I checked the code and we do the same in save_group. >> >>> >> >>> But I noticed that we do not use lowercase in save_group >> >>> but directly !dom->case_sensitive. >> >>> >> >>> I think you can do sa small refactoring and a little bit >> >>> unify functions save_user and save_group (of cource in separate patch. >> >>> >> >>> I would prefer if parameter "bool lowercase" could be removed >> >>> from function save_user. We can still use it as a local variable. >> >>> And we can use local variable "bool lowercase" in save_group >> >>> to improve readability inside save_group. >> >> >> >>Okay, I'll do the refactoring. >> >>May I consider this patch ACKed and ready for RHEL? >> >> >> > Sure >> > >> >>The refactoring patches will come later :-) >> >> >> > I do not assume that it will took a long time to do a refactoring. >> > That's the reason why I would prefer to do it sooner >> > and as part of this ticket :-) >> >> It always can be "Related to" :-) >> I just believe the fix can be pushed (and backported) as soon as >> possible, while the refactoring can be delayed by a few days if needed >> (mainly because I've returned the beaker machines and it will take me >> a day or so to get another ones ... and I want to be sure I won't >> break anything with that) > >Yes, I would also prefer to push this patch now if it fixes the bug and >continue on the refactoring (without delay, it seems to be small and >would improve the proxy provider code). > >> >> > >> > But If you are busy we can do it later >> > which usually means almost never(deferred). >> >> That's not the case. > >+1 to fixing downstream now (and 'hot-patching' upstream also with this >patch) and continuing on improving upstream.
As you wish. ACK master: * 5691b2d668541585d2a8ae3ddb834f29d828036e LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
