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
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?= fidencio@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 fidencio@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.
LS
On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik lslebodn@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?= fidencio@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 fidencio@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?
The refactoring patches will come later :-)
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On (24/08/16 10:37), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik lslebodn@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?= fidencio@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 fidencio@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 :-)
But If you are busy we can do it later which usually means almost never(deferred).
LS
On Wed, Aug 24, 2016 at 10:43 AM, Lukas Slebodnik lslebodn@redhat.com wrote:
On (24/08/16 10:37), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik lslebodn@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?= fidencio@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 fidencio@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)
But If you are busy we can do it later which usually means almost never(deferred).
That's not the case.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
Best Regards, -- Fabiano Fidêncio
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 lslebodn@redhat.com wrote:
On (24/08/16 10:37), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik lslebodn@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?= fidencio@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 fidencio@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.
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 lslebodn@redhat.com wrote:
On (24/08/16 10:37), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 10:33 AM, Lukas Slebodnik lslebodn@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?= fidencio@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 fidencio@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@lists.fedorahosted.org
-
Fabiano Fidêncio -
Jakub Hrozek -
Lukas Slebodnik