* [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
@ 2023-12-20 14:37 Filip Schauer
2023-12-21 9:42 ` Lukas Wagner
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Filip Schauer @ 2023-12-20 14:37 UTC (permalink / raw)
To: pve-devel
Decode non-ASCII character when syncing user attributes, since those
will be encoded later on. Without this fix the attributes where encoded
twice, resulting in cases such as 'ü' turning into 'ü'.
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
src/PVE/Auth/LDAP.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
index b958f2b..5e7a30c 100755
--- a/src/PVE/Auth/LDAP.pm
+++ b/src/PVE/Auth/LDAP.pm
@@ -301,7 +301,7 @@ sub get_users {
foreach my $attr (keys %$user_attributes) {
if (my $ours = $ldap_attribute_map->{$attr}) {
- $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
+ $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
}
}
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
2023-12-20 14:37 [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes Filip Schauer
@ 2023-12-21 9:42 ` Lukas Wagner
2023-12-21 9:46 ` Christoph Heiss
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2023-12-21 9:42 UTC (permalink / raw)
To: Proxmox VE development discussion, Filip Schauer
Hi, thanks for tackling this!
On 12/20/23 15:37, Filip Schauer wrote:
> Decode non-ASCII character when syncing user attributes, since those
> will be encoded later on. Without this fix the attributes where encoded
> twice, resulting in cases such as 'ü' turning into 'ü'.
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/Auth/LDAP.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index b958f2b..5e7a30c 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -301,7 +301,7 @@ sub get_users {
>
> foreach my $attr (keys %$user_attributes) {
> if (my $ours = $ldap_attribute_map->{$attr}) {
> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> + $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
> }
> }
>
Gave this a quick test.
Set up a glauth LDAP server, added some unicode symbols to the 'sn' LDAP
attribute, configured sync_attributes lastname=sn in domains.cfg and
tested the sync.
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
--
- Lukas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
2023-12-20 14:37 [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes Filip Schauer
2023-12-21 9:42 ` Lukas Wagner
@ 2023-12-21 9:46 ` Christoph Heiss
2023-12-21 10:03 ` Lukas Wagner
2024-01-08 9:26 ` Wolfgang Bumiller
3 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2023-12-21 9:46 UTC (permalink / raw)
To: Filip Schauer; +Cc: Proxmox VE development discussion
Tested it using Windows Server 2022 and Samba 4.19.2 on Linux, with
both LDAP and AD realms.
Fixes the problem after a re-sync, LGTM.
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
On Wed, Dec 20, 2023 at 03:37:03PM +0100, Filip Schauer wrote:
> Decode non-ASCII character when syncing user attributes, since those
> will be encoded later on. Without this fix the attributes where encoded
> twice, resulting in cases such as 'ü' turning into 'ü'.
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/Auth/LDAP.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index b958f2b..5e7a30c 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -301,7 +301,7 @@ sub get_users {
>
> foreach my $attr (keys %$user_attributes) {
> if (my $ours = $ldap_attribute_map->{$attr}) {
> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> + $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
> }
> }
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
2023-12-20 14:37 [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes Filip Schauer
2023-12-21 9:42 ` Lukas Wagner
2023-12-21 9:46 ` Christoph Heiss
@ 2023-12-21 10:03 ` Lukas Wagner
2024-01-08 9:26 ` Wolfgang Bumiller
3 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2023-12-21 10:03 UTC (permalink / raw)
To: Proxmox VE development discussion, Filip Schauer
On 12/20/23 15:37, Filip Schauer wrote:
> Decode non-ASCII character when syncing user attributes, since those
> will be encoded later on. Without this fix the attributes where encoded
> twice, resulting in cases such as 'ü' turning into 'ü'.
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/Auth/LDAP.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index b958f2b..5e7a30c 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -301,7 +301,7 @@ sub get_users {
>
> foreach my $attr (keys %$user_attributes) {
> if (my $ours = $ldap_attribute_map->{$attr}) {
> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> + $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
> }
> }
>
For the record, the reporter of the issue also confirmed that the fix
works for them:
https://bugzilla.proxmox.com/show_bug.cgi?id=5136
--
- Lukas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
2023-12-20 14:37 [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes Filip Schauer
` (2 preceding siblings ...)
2023-12-21 10:03 ` Lukas Wagner
@ 2024-01-08 9:26 ` Wolfgang Bumiller
2024-01-09 13:36 ` Filip Schauer
3 siblings, 1 reply; 6+ messages in thread
From: Wolfgang Bumiller @ 2024-01-08 9:26 UTC (permalink / raw)
To: Filip Schauer; +Cc: pve-devel
On Wed, Dec 20, 2023 at 03:37:03PM +0100, Filip Schauer wrote:
> Decode non-ASCII character when syncing user attributes, since those
decode *how*?
> will be encoded later on. Without this fix the attributes where encoded
> twice, resulting in cases such as 'ü' turning into 'ü'.
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/Auth/LDAP.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index b958f2b..5e7a30c 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -301,7 +301,7 @@ sub get_users {
>
> foreach my $attr (keys %$user_attributes) {
> if (my $ours = $ldap_attribute_map->{$attr}) {
> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
> + $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
This does 2 things: URI unescaping and utf-8 decoding.
Does the former make sense?
Given that 'decode_text' is a way too generic name in a module with yet
another way too generic name "tools", I'd argue against its use in
general and would prefer to call the *actual* decode function right
there so you can see what's going on.
> }
> }
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
2024-01-08 9:26 ` Wolfgang Bumiller
@ 2024-01-09 13:36 ` Filip Schauer
0 siblings, 0 replies; 6+ messages in thread
From: Filip Schauer @ 2024-01-09 13:36 UTC (permalink / raw)
To: Wolfgang Bumiller; +Cc: pve-devel
Patch v2 is available:
https://lists.proxmox.com/pipermail/pve-devel/2024-January/061273.html
On 08/01/2024 10:26, Wolfgang Bumiller wrote:
> On Wed, Dec 20, 2023 at 03:37:03PM +0100, Filip Schauer wrote:
>> Decode non-ASCII character when syncing user attributes, since those
> decode *how*?
>
>> will be encoded later on. Without this fix the attributes where encoded
>> twice, resulting in cases such as 'ü' turning into 'ü'.
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> src/PVE/Auth/LDAP.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
>> index b958f2b..5e7a30c 100755
>> --- a/src/PVE/Auth/LDAP.pm
>> +++ b/src/PVE/Auth/LDAP.pm
>> @@ -301,7 +301,7 @@ sub get_users {
>>
>> foreach my $attr (keys %$user_attributes) {
>> if (my $ours = $ldap_attribute_map->{$attr}) {
>> - $ret->{$username}->{$ours} = $user_attributes->{$attr}->[0];
>> + $ret->{$username}->{$ours} = PVE::Tools::decode_text($user_attributes->{$attr}->[0]);
> This does 2 things: URI unescaping and utf-8 decoding.
> Does the former make sense?
>
> Given that 'decode_text' is a way too generic name in a module with yet
> another way too generic name "tools", I'd argue against its use in
> general and would prefer to call the *actual* decode function right
> there so you can see what's going on.
>
>> }
>> }
>>
>> --
>> 2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-09 13:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-20 14:37 [pve-devel] [PATCH access-control] fix #5136: ldap: Decode non-ASCII characters in attributes Filip Schauer
2023-12-21 9:42 ` Lukas Wagner
2023-12-21 9:46 ` Christoph Heiss
2023-12-21 10:03 ` Lukas Wagner
2024-01-08 9:26 ` Wolfgang Bumiller
2024-01-09 13:36 ` Filip Schauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox