* [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again
@ 2025-02-27 9:49 Stoiko Ivanov
2025-02-27 9:51 ` Stoiko Ivanov
2025-02-27 9:55 ` Thomas Lamprecht
0 siblings, 2 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-02-27 9:49 UTC (permalink / raw)
To: pmg-devel
verify_username is used in many places to split into realms (the part
after the last '@') and usernames (everthing before).
The commit disallowing '@' in usernames broke quarantine login
(users login with `localpart@domainname.com@quarantine`)
Fixes: 9665bbc ("utils: user schema: explicitly forbid @ in user-names")
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
tested minimally with my local setup (and with Dominik's GUI patches)
src/PMG/API2/Users.pm | 2 ++
src/PMG/Utils.pm | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/PMG/API2/Users.pm b/src/PMG/API2/Users.pm
index 132783d..9cbcfd1 100644
--- a/src/PMG/API2/Users.pm
+++ b/src/PMG/API2/Users.pm
@@ -126,6 +126,8 @@ __PACKAGE__->register_method ({
my ($userid, $username, $realm) = PMG::Utils::verify_username($entry->{userid});
die "invalid realm '$realm' in userid\n" if !PMG::Auth::Plugin::is_valid_realm($realm);
+ die "'@' forbidden in username\n" if $username =~/@/;;
+
if ($entry->{realm}) {
die "realm parameter does not fit userid ('$entry->{realm}' != '$realm')\n"
if $entry->{realm} ne $realm;
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 70e8317..3e7adbb 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -49,7 +49,7 @@ postgres_admin_cmd
try_decode_utf8
);
-my $user_regex = qr![^\s:@/]+!;
+my $user_regex = qr![^\s:/]+!;
PVE::JSONSchema::register_standard_option('pmg-starttime', {
description => "Only consider entries newer than 'starttime' (unix epoch). Default is 'now - 1day'.",
--
2.39.5
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again
2025-02-27 9:49 [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again Stoiko Ivanov
@ 2025-02-27 9:51 ` Stoiko Ivanov
2025-02-27 9:55 ` Thomas Lamprecht
1 sibling, 0 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-02-27 9:51 UTC (permalink / raw)
To: pmg-devel
On Thu, 27 Feb 2025 10:49:51 +0100
Stoiko Ivanov <s.ivanov@proxmox.com> wrote:
> verify_username is used in many places to split into realms (the part
> after the last '@') and usernames (everthing before).
>
> The commit disallowing '@' in usernames broke quarantine login
> (users login with `localpart@domainname.com@quarantine`)
>
> Fixes: 9665bbc ("utils: user schema: explicitly forbid @ in user-names")
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> tested minimally with my local setup (and with Dominik's GUI patches)
>
> src/PMG/API2/Users.pm | 2 ++
> src/PMG/Utils.pm | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/PMG/API2/Users.pm b/src/PMG/API2/Users.pm
> index 132783d..9cbcfd1 100644
> --- a/src/PMG/API2/Users.pm
> +++ b/src/PMG/API2/Users.pm
> @@ -126,6 +126,8 @@ __PACKAGE__->register_method ({
> my ($userid, $username, $realm) = PMG::Utils::verify_username($entry->{userid});
> die "invalid realm '$realm' in userid\n" if !PMG::Auth::Plugin::is_valid_realm($realm);
>
> + die "'@' forbidden in username\n" if $username =~/@/;;
Dominik pointed the ;; out - will send a v2
> +
> if ($entry->{realm}) {
> die "realm parameter does not fit userid ('$entry->{realm}' != '$realm')\n"
> if $entry->{realm} ne $realm;
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 70e8317..3e7adbb 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -49,7 +49,7 @@ postgres_admin_cmd
> try_decode_utf8
> );
>
> -my $user_regex = qr![^\s:@/]+!;
> +my $user_regex = qr![^\s:/]+!;
>
> PVE::JSONSchema::register_standard_option('pmg-starttime', {
> description => "Only consider entries newer than 'starttime' (unix epoch). Default is 'now - 1day'.",
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again
2025-02-27 9:49 [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again Stoiko Ivanov
2025-02-27 9:51 ` Stoiko Ivanov
@ 2025-02-27 9:55 ` Thomas Lamprecht
2025-02-27 10:06 ` Stoiko Ivanov
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2025-02-27 9:55 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
Am 27.02.25 um 10:49 schrieb Stoiko Ivanov:
> verify_username is used in many places to split into realms (the part
> after the last '@') and usernames (everthing before).
>
> The commit disallowing '@' in usernames broke quarantine login
> (users login with `localpart@domainname.com@quarantine`)
>
we actually allow @ in PVE/PBS and just use the last occurring @ as separator
for the realm, maybe better to go that route here to for consistency, or
what do we win?
> Fixes: 9665bbc ("utils: user schema: explicitly forbid @ in user-names")
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> tested minimally with my local setup (and with Dominik's GUI patches)
>
> src/PMG/API2/Users.pm | 2 ++
> src/PMG/Utils.pm | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/PMG/API2/Users.pm b/src/PMG/API2/Users.pm
> index 132783d..9cbcfd1 100644
> --- a/src/PMG/API2/Users.pm
> +++ b/src/PMG/API2/Users.pm
> @@ -126,6 +126,8 @@ __PACKAGE__->register_method ({
> my ($userid, $username, $realm) = PMG::Utils::verify_username($entry->{userid});
> die "invalid realm '$realm' in userid\n" if !PMG::Auth::Plugin::is_valid_realm($realm);
>
> + die "'@' forbidden in username\n" if $username =~/@/;;
> +
> if ($entry->{realm}) {
> die "realm parameter does not fit userid ('$entry->{realm}' != '$realm')\n"
> if $entry->{realm} ne $realm;
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 70e8317..3e7adbb 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -49,7 +49,7 @@ postgres_admin_cmd
> try_decode_utf8
> );
>
> -my $user_regex = qr![^\s:@/]+!;
> +my $user_regex = qr![^\s:/]+!;
>
> PVE::JSONSchema::register_standard_option('pmg-starttime', {
> description => "Only consider entries newer than 'starttime' (unix epoch). Default is 'now - 1day'.",
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again
2025-02-27 9:55 ` Thomas Lamprecht
@ 2025-02-27 10:06 ` Stoiko Ivanov
0 siblings, 0 replies; 4+ messages in thread
From: Stoiko Ivanov @ 2025-02-27 10:06 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: pmg-devel
On Thu, 27 Feb 2025 10:55:45 +0100
Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
> Am 27.02.25 um 10:49 schrieb Stoiko Ivanov:
> > verify_username is used in many places to split into realms (the part
> > after the last '@') and usernames (everthing before).
> >
> > The commit disallowing '@' in usernames broke quarantine login
> > (users login with `localpart@domainname.com@quarantine`)
> >
>
> we actually allow @ in PVE/PBS and just use the last occurring @ as separator
> for the realm, maybe better to go that route here to for consistency, or
> what do we win?
In the mid- to long-term I think that makes sense (see also fabian's
comment on e-mail being used as login/username is not too uncommon)
OTOH - It's always easy to allow something which was forbidden (or as in
case with 'foo@bar' as username in PMG - broken) before, but the other
direction (disallowing stuff) is harder.
The patch (`9665bbc`) should've made what you can create and what is
actually read-back from user.conf consisten (but broke quarantine logins)
>
> > Fixes: 9665bbc ("utils: user schema: explicitly forbid @ in user-names")
> > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> > ---
> > tested minimally with my local setup (and with Dominik's GUI patches)
> >
> > src/PMG/API2/Users.pm | 2 ++
> > src/PMG/Utils.pm | 2 +-
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/PMG/API2/Users.pm b/src/PMG/API2/Users.pm
> > index 132783d..9cbcfd1 100644
> > --- a/src/PMG/API2/Users.pm
> > +++ b/src/PMG/API2/Users.pm
> > @@ -126,6 +126,8 @@ __PACKAGE__->register_method ({
> > my ($userid, $username, $realm) = PMG::Utils::verify_username($entry->{userid});
> > die "invalid realm '$realm' in userid\n" if !PMG::Auth::Plugin::is_valid_realm($realm);
> >
> > + die "'@' forbidden in username\n" if $username =~/@/;;
> > +
> > if ($entry->{realm}) {
> > die "realm parameter does not fit userid ('$entry->{realm}' != '$realm')\n"
> > if $entry->{realm} ne $realm;
> > diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> > index 70e8317..3e7adbb 100644
> > --- a/src/PMG/Utils.pm
> > +++ b/src/PMG/Utils.pm
> > @@ -49,7 +49,7 @@ postgres_admin_cmd
> > try_decode_utf8
> > );
> >
> > -my $user_regex = qr![^\s:@/]+!;
> > +my $user_regex = qr![^\s:/]+!;
> >
> > PVE::JSONSchema::register_standard_option('pmg-starttime', {
> > description => "Only consider entries newer than 'starttime' (unix epoch). Default is 'now - 1day'.",
>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-02-27 10:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-27 9:49 [pmg-devel] [PATCH pmg-api] utils: verify_username: allow quarantine logins again Stoiko Ivanov
2025-02-27 9:51 ` Stoiko Ivanov
2025-02-27 9:55 ` Thomas Lamprecht
2025-02-27 10:06 ` Stoiko Ivanov
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.