* [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 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