all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Filip Schauer <f.schauer@proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 access-control] fix #5136: ldap: Decode non-ASCII characters in attributes
Date: Wed, 28 Feb 2024 16:00:48 +0100	[thread overview]
Message-ID: <bddd9ead-d8b2-4a88-b8ca-c2137dc69015@proxmox.com> (raw)
In-Reply-To: <6eb978ee-e8ea-4b86-9b9c-b04517893f4e@proxmox.com>

Am 28.02.24 um 15:41 schrieb Thomas Lamprecht:
> Am 09/01/2024 um 14:35 schrieb Filip Schauer:
>> UTF8 decode non-ASCII characters when syncing user attributes, since
>> those will be encoded later on. Without this fix the attributes were
>> encoded twice, resulting in cases such as 'ü' turning into 'ü'.
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> Changes since v1:
>> * Do not try to URI unescape the user attributes, since we do that later
>>   in PVE::AccessControl::parse_user_config anyways.
>>
>>  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..06177db 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} = Encode::decode('utf8', $user_attributes->{$attr}->[0]);

Note: missing use Encode; at the beginning of the file.

>>  	    }
>>  	}
>>  
> 
> this would need a rebase, oh, and would be great if the original testers
> could reconfirm the v2 approach of doing utf-8 decoding only.
> 

Gave it a quick test and fixes issues with special characters for me.
Don't forget to also use the latest master of pve-cluster, otherwise
writing the user config will still do the wrong thing [0]! Both are
needed to fix the issue here. I'm just wondering if we are guaranteed
that the LDAP server sends UTF-8 encoded data?

[0]:
https://git.proxmox.com/?p=pve-cluster.git;a=commit;h=2e276ccd9beb2004ddd72396b2a9b72a288771d8




  reply	other threads:[~2024-02-28 15:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-09 13:35 Filip Schauer
2024-01-09 13:38 ` Fiona Ebner
2024-01-09 13:51   ` Filip Schauer
2024-02-28 14:41 ` Thomas Lamprecht
2024-02-28 15:00   ` Fiona Ebner [this message]
2024-02-28 18:58     ` Stoiko Ivanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bddd9ead-d8b2-4a88-b8ca-c2137dc69015@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=f.schauer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal