[pmg-devel] [PATCH] utils: cleanup username/userid regex and verify

[Gabriel Goller]

Thanks for the review!

On Wed Feb 14, 2024 at 12:55 PM CET, Stoiko Ivanov wrote:
> > diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> > index 12b3ed5..8f7d438 100644
> > --- a/src/PMG/Utils.pm
> > +++ b/src/PMG/Utils.pm
> > @@ -72,13 +72,12 @@ PVE::JSONSchema::register_standard_option('pmg-endtime', {
> >      optional => 1,
> >  });
> >  
> > -PVE::JSONSchema::register_format('pmg-userid', \&verify_username);
> why deregister the format here? (verify_username does a bit more than a
> regex match - and reusing the same verification we use in the auth-code
> also in the parts where the API comes in helps in not getting even more
> matches-almost-the-same-regexes matching auth-data) - Currently I'd rather
> aim to reduce those and if possible unify PMG::UserConfig::verify_entry
> with verify_username here as far as possible - see also:
> https://lists.proxmox.com/pipermail/pmg-devel/2023-March/002381.html
> and Fabian's follow-up to it.

Right, yeah. I readded the register_format call...
Hmm how would you unify verify_entry with verify_username though? It 
seems to me that verify_entry just splits the username from the userid 
(if needed) then checks if the username is in the userid (which we 
could also check in verify_username) and then calls verify_username?

> >  sub verify_username {
> >      my ($username, $noerr) = @_;
> >  
> >      $username = '' if !$username;
> >      my $len = length($username);
> > -    if ($len < 3) {
> > +    if ($len < 1) {
> this "username" here is actually the one with the realm...
> e.g. root at pam vs. root - so limiting the length to 1 is too little
> restrictive - probably at least renaming the variable name to user_id
> might help in reducing confusion..

Missed this :(
How about I use a min length of 5 here? 
shortest realm (pam/pmg) + @ + shortest username = 5