> Mira Limbeck <m.limb...@proxmox.com> hat am 12.02.2025 15:51 CET geschrieben:
>
>
> On 2/11/25 06:40, Thomas Skinner wrote:
> > Signed-off-by: Thomas Skinner <tho...@atskinner.net>
> > ---
> > src/PVE/API2/OpenId.pm | 79 ++++++++++++++++++++++++++++++++++++++++
> > src/PVE/AccessControl.pm | 2 +-
> > src/PVE/Auth/OpenId.pm | 33 +++++++++++++++++
> > src/PVE/Auth/Plugin.pm | 1 +
> > 4 files changed, 114 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
> > index 77410e6..818175e 100644
> > --- a/src/PVE/API2/OpenId.pm
> > +++ b/src/PVE/API2/OpenId.pm
> > @@ -13,6 +13,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file);
> > use PVE::AccessControl;
> > use PVE::JSONSchema qw(get_standard_option);
> > use PVE::Auth::Plugin;
> > +use PVE::Auth::OpenId;
> >
> > use PVE::RESTHandler;
> >
> > @@ -220,6 +221,84 @@ __PACKAGE__->register_method ({
> > $rpcenv->check_user_enabled($username);
> > }
> >
> > + if (defined(my $groups_claim = $config->{'groups-claim'})) {
> > + if (defined(my $groups_list = $info->{$groups_claim})) {
> > + if (ref($groups_list) eq 'ARRAY') {
> > + PVE::AccessControl::lock_user_config(sub {
> > + my $usercfg = cfs_read_file("user.cfg");
> > +
> > + # replace any invalid characters with
> > + my $replace_character =
> > $config->{'groups-replace-character'} // '_';
> > + my $oidc_groups = { map {
> > + $_ =~
> > s/[^$PVE::Auth::Plugin::groupname_regex_chars]/$replace_character/gr => 1
> > + } $groups_list->@* };
> maybe we could log any of those replacements here? doing this silently
> may lead to confusion when groups don't match
a similar issue is filed for LDAP/AD sync as well - and I now wonder based on
the discussion there - do we really want to make this configurable? how do we
want to handle conflicts? while it's a bit less critical for two or more OIDC
groups to be mapped to the same PVE-side group (compared to the same happening
with users ;)), if it's possible to avoid it that would still be great..
groups currently allow .-_ as special characters, so we could designate one of
them as escape character and then have a unique mapping for each character that
isn't allowed on the PVE side (including that escape character ;))
e.g., an OIDC group called "foo bar" could be encoded as "foo_32_bar" (where 32
is hex for ASCII-" "). correspondingly, a group called "foo_bar" would need to
be encoded as "foo_5F_bar". (the second '_' could of course be left off if
desired).
unfortunately, adding an entirely new escape character is not really possible
unless we want to wait for 9.0, as that would then break parsing of user.cfg in
a mixed cluster which can have really dangerous side-effects..
or we could live with such a potentially lossy mapping, but then I am not sure
whether a single, hard-coded, documented value wouldn't be better? the main
issue with that is if you allow (unprivileged) creation and joining of groups
on the OIDC side, as then if there already is a group called "System
Administrators" that got mapped to "System_Adminstrators" on the PVE side, a
user could create and join "System!Administrators" on the OIDC side and get
mapped to the existing, probably privileged "System_Administrators" group..
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
