From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 22ECF1FF16F
for <inbox@lore.proxmox.com>; Thu, 13 Feb 2025 12:03:56 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 38C7CD86;
Thu, 13 Feb 2025 12:03:53 +0100 (CET)
Date: Thu, 13 Feb 2025 12:03:50 +0100 (CET)
From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Mira Limbeck <m.limbeck@proxmox.com>
Message-ID: <542246635.6628.1739444630435@webmail.proxmox.com>
In-Reply-To: <f3732723-d617-47a9-a7be-652f0893b6ed@proxmox.com>
References: <20250211054029.1269099-1-thomas@atskinner.net>
<20250211054029.1269099-4-thomas@atskinner.net>
<f3732723-d617-47a9-a7be-652f0893b6ed@proxmox.com>
MIME-Version: 1.0
X-Priority: 3
Importance: Normal
X-Mailer: Open-Xchange Mailer v7.10.6-Rev73
X-Originating-Client: open-xchange-appsuite
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.045 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH access-control v3 1/1] fix #4411: openid:
add logic for openid groups support
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
> Mira Limbeck <m.limbeck@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 <thomas@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