* [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive
@ 2020-09-03 8:36 Wolfgang Link
2020-09-07 8:20 ` Dominik Csapak
0 siblings, 1 reply; 4+ messages in thread
From: Wolfgang Link @ 2020-09-03 8:36 UTC (permalink / raw)
To: pve-devel
This is an optional for LDAP and AD realm.
The default behavior is case-sensitive.
Signed-off-by: Wolfgang Link <w.link@proxmox.com>
---
v1 -> v2: * naming of paramenter
* use grep instead of a loop, to avoid login errors
with ambiguous usernames
PVE/API2/AccessControl.pm | 23 +++++++++++++++++++++++
PVE/Auth/AD.pm | 1 +
PVE/Auth/LDAP.pm | 7 +++++++
3 files changed, 31 insertions(+)
diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
index fd27786..3155d67 100644
--- a/PVE/API2/AccessControl.pm
+++ b/PVE/API2/AccessControl.pm
@@ -226,6 +226,28 @@ __PACKAGE__->register_method ({
returns => { type => "null" },
code => sub { return undef; }});
+sub lookup_username {
+ my ($username) = @_;
+
+ $username =~ /@(.+)/;
+
+ my $realm = $1;
+ my $domain_cfg = cfs_read_file("domains.cfg");
+ my $casesensitive = $domain_cfg->{ids}->{$realm}->{'case-sensitive'} // 1;
+ my $usercfg = cfs_read_file('user.cfg');
+
+ if (!$casesensitive) {
+ my @matches = grep { lc $username eq lc $_ } (keys %{$usercfg->{users}});
+
+ die "ambiguous case insensitive match of username '$username', cannot safely grant access!\n"
+ if scalar @matches > 1;
+
+ return $matches[0]
+ }
+
+ return $username;
+};
+
__PACKAGE__->register_method ({
name => 'create_ticket',
path => 'ticket',
@@ -292,6 +314,7 @@ __PACKAGE__->register_method ({
my $username = $param->{username};
$username .= "\@$param->{realm}" if $param->{realm};
+ $username = lookup_username($username);
my $rpcenv = PVE::RPCEnvironment::get();
my $res;
diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
index 4d64c20..88b2098 100755
--- a/PVE/Auth/AD.pm
+++ b/PVE/Auth/AD.pm
@@ -94,6 +94,7 @@ sub options {
group_classes => { optional => 1 },
'sync-defaults-options' => { optional => 1 },
mode => { optional => 1 },
+ 'case-sensitive' => { optional => 1 },
};
}
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 09b2202..97d0778 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -129,6 +129,12 @@ sub properties {
optional => 1,
default => 'ldap',
},
+ 'case-sensitive' => {
+ description => "username is case-sensitive",
+ type => 'boolean',
+ optional => 1,
+ default => 1,
+ }
};
}
@@ -159,6 +165,7 @@ sub options {
group_classes => { optional => 1 },
'sync-defaults-options' => { optional => 1 },
mode => { optional => 1 },
+ 'case-sensitive' => { optional => 1 },
};
}
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive
2020-09-03 8:36 [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive Wolfgang Link
@ 2020-09-07 8:20 ` Dominik Csapak
2020-09-07 8:42 ` Wolfgang Link
0 siblings, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2020-09-07 8:20 UTC (permalink / raw)
To: pve-devel
one comment inline
On 9/3/20 10:36 AM, Wolfgang Link wrote:
> This is an optional for LDAP and AD realm.
> The default behavior is case-sensitive.
>
> Signed-off-by: Wolfgang Link <w.link@proxmox.com>
> ---
> v1 -> v2: * naming of paramenter
> * use grep instead of a loop, to avoid login errors
> with ambiguous usernames
>
> PVE/API2/AccessControl.pm | 23 +++++++++++++++++++++++
> PVE/Auth/AD.pm | 1 +
> PVE/Auth/LDAP.pm | 7 +++++++
> 3 files changed, 31 insertions(+)
>
> diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
> index fd27786..3155d67 100644
> --- a/PVE/API2/AccessControl.pm
> +++ b/PVE/API2/AccessControl.pm
> @@ -226,6 +226,28 @@ __PACKAGE__->register_method ({
> returns => { type => "null" },
> code => sub { return undef; }});
>
> +sub lookup_username {
> + my ($username) = @_;
> +
> + $username =~ /@(.+)/;
i do not know if you saw my last mail, but we have to do a
better regex here, since the username can contain an '@'
so foo@bar@pve is a valid username (:-S)
and the realm here would parse to 'bar@pve'
so a better regex would be
/@([^@]+)$/
> +
> + my $realm = $1;
> + my $domain_cfg = cfs_read_file("domains.cfg");
> + my $casesensitive = $domain_cfg->{ids}->{$realm}->{'case-sensitive'} // 1;
> + my $usercfg = cfs_read_file('user.cfg');
> +
> + if (!$casesensitive) {
> + my @matches = grep { lc $username eq lc $_ } (keys %{$usercfg->{users}});
> +
> + die "ambiguous case insensitive match of username '$username', cannot safely grant access!\n"
> + if scalar @matches > 1;
> +
> + return $matches[0]
> + }
> +
> + return $username;
> +};
> +
> __PACKAGE__->register_method ({
> name => 'create_ticket',
> path => 'ticket',
> @@ -292,6 +314,7 @@ __PACKAGE__->register_method ({
> my $username = $param->{username};
> $username .= "\@$param->{realm}" if $param->{realm};
>
> + $username = lookup_username($username);
> my $rpcenv = PVE::RPCEnvironment::get();
>
> my $res;
> diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
> index 4d64c20..88b2098 100755
> --- a/PVE/Auth/AD.pm
> +++ b/PVE/Auth/AD.pm
> @@ -94,6 +94,7 @@ sub options {
> group_classes => { optional => 1 },
> 'sync-defaults-options' => { optional => 1 },
> mode => { optional => 1 },
> + 'case-sensitive' => { optional => 1 },
> };
> }
>
> diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
> index 09b2202..97d0778 100755
> --- a/PVE/Auth/LDAP.pm
> +++ b/PVE/Auth/LDAP.pm
> @@ -129,6 +129,12 @@ sub properties {
> optional => 1,
> default => 'ldap',
> },
> + 'case-sensitive' => {
> + description => "username is case-sensitive",
> + type => 'boolean',
> + optional => 1,
> + default => 1,
> + }
> };
> }
>
> @@ -159,6 +165,7 @@ sub options {
> group_classes => { optional => 1 },
> 'sync-defaults-options' => { optional => 1 },
> mode => { optional => 1 },
> + 'case-sensitive' => { optional => 1 },
> };
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive
2020-09-07 8:20 ` Dominik Csapak
@ 2020-09-07 8:42 ` Wolfgang Link
2020-09-07 9:29 ` Thomas Lamprecht
0 siblings, 1 reply; 4+ messages in thread
From: Wolfgang Link @ 2020-09-07 8:42 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
No I missed your mail.
Will fix it and resend it.
> On 09/07/2020 10:20 AM Dominik Csapak <d.csapak@proxmox.com> wrote:
>
>
> one comment inline
>
> On 9/3/20 10:36 AM, Wolfgang Link wrote:
> > This is an optional for LDAP and AD realm.
> > The default behavior is case-sensitive.
> >
> > Signed-off-by: Wolfgang Link <w.link@proxmox.com>
> > ---
> > v1 -> v2: * naming of paramenter
> > * use grep instead of a loop, to avoid login errors
> > with ambiguous usernames
> >
> > PVE/API2/AccessControl.pm | 23 +++++++++++++++++++++++
> > PVE/Auth/AD.pm | 1 +
> > PVE/Auth/LDAP.pm | 7 +++++++
> > 3 files changed, 31 insertions(+)
> >
> > diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
> > index fd27786..3155d67 100644
> > --- a/PVE/API2/AccessControl.pm
> > +++ b/PVE/API2/AccessControl.pm
> > @@ -226,6 +226,28 @@ __PACKAGE__->register_method ({
> > returns => { type => "null" },
> > code => sub { return undef; }});
> >
> > +sub lookup_username {
> > + my ($username) = @_;
> > +
> > + $username =~ /@(.+)/;
>
> i do not know if you saw my last mail, but we have to do a
> better regex here, since the username can contain an '@'
>
> so foo@bar@pve is a valid username (:-S)
> and the realm here would parse to 'bar@pve'
>
> so a better regex would be
> /@([^@]+)$/
>
> > +
> > + my $realm = $1;
> > + my $domain_cfg = cfs_read_file("domains.cfg");
> > + my $casesensitive = $domain_cfg->{ids}->{$realm}->{'case-sensitive'} // 1;
> > + my $usercfg = cfs_read_file('user.cfg');
> > +
> > + if (!$casesensitive) {
> > + my @matches = grep { lc $username eq lc $_ } (keys %{$usercfg->{users}});
> > +
> > + die "ambiguous case insensitive match of username '$username', cannot safely grant access!\n"
> > + if scalar @matches > 1;
> > +
> > + return $matches[0]
> > + }
> > +
> > + return $username;
> > +};
> > +
> > __PACKAGE__->register_method ({
> > name => 'create_ticket',
> > path => 'ticket',
> > @@ -292,6 +314,7 @@ __PACKAGE__->register_method ({
> > my $username = $param->{username};
> > $username .= "\@$param->{realm}" if $param->{realm};
> >
> > + $username = lookup_username($username);
> > my $rpcenv = PVE::RPCEnvironment::get();
> >
> > my $res;
> > diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
> > index 4d64c20..88b2098 100755
> > --- a/PVE/Auth/AD.pm
> > +++ b/PVE/Auth/AD.pm
> > @@ -94,6 +94,7 @@ sub options {
> > group_classes => { optional => 1 },
> > 'sync-defaults-options' => { optional => 1 },
> > mode => { optional => 1 },
> > + 'case-sensitive' => { optional => 1 },
> > };
> > }
> >
> > diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
> > index 09b2202..97d0778 100755
> > --- a/PVE/Auth/LDAP.pm
> > +++ b/PVE/Auth/LDAP.pm
> > @@ -129,6 +129,12 @@ sub properties {
> > optional => 1,
> > default => 'ldap',
> > },
> > + 'case-sensitive' => {
> > + description => "username is case-sensitive",
> > + type => 'boolean',
> > + optional => 1,
> > + default => 1,
> > + }
> > };
> > }
> >
> > @@ -159,6 +165,7 @@ sub options {
> > group_classes => { optional => 1 },
> > 'sync-defaults-options' => { optional => 1 },
> > mode => { optional => 1 },
> > + 'case-sensitive' => { optional => 1 },
> > };
> > }
> >
> >
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive
2020-09-07 8:42 ` Wolfgang Link
@ 2020-09-07 9:29 ` Thomas Lamprecht
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2020-09-07 9:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Wolfgang Link, Dominik Csapak
On 07.09.20 10:42, Wolfgang Link wrote:
> No I missed your mail.
> Will fix it and resend it.
Please also include my proposed change from then:
On 28.08.20 14:39, Thomas Lamprecht wrote:
> And we then actually want to use this method also in the API call for adding
> new users, to ensure an admin do not accidentally adds the case-sensitive same
> user multiple times.
Also, "lookup_username" does not seem to fit the API module much, I'd rather
put it in the more general PVE::AccessControl module.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-07 9:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 8:36 [pve-devel] [Patch v2 access-control] fix #2947 login name for the LDAP/AD realm can be case-insensitive Wolfgang Link
2020-09-07 8:20 ` Dominik Csapak
2020-09-07 8:42 ` Wolfgang Link
2020-09-07 9:29 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox