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 22A181FF162
	for <inbox@lore.proxmox.com>; Sat,  8 Feb 2025 07:40:44 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id CD4E918AE5;
	Sat,  8 Feb 2025 07:40:40 +0100 (CET)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1738996832; x=1739601632;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id
 :reply-to;
 bh=tbvibQNPsbj89gJ2QU6DkCbWf+0GjBAucSA+M8nFVns=;
 b=Cu4C8xq7U6G18NV+YomlV8C5JXA+rW6HmVpJEqkgyDroI4ktC1cpyjwY7Wcslz5ShW
 3PEpxq2eAxcdmdpnnmuUIOxANmnAxVzeg+P5SNct30dlQNEhEzhj7DiTYFN2fNyHqfTt
 WmW9D0VV+Kmqo2SfvzXQAb5hT3vuPlJrdGbot3uEta10QEbWBLOZ1Yd1uy2qFTwct7iS
 1n4lMmKbG8kQCJUVhDGWe4zYadg4nQwZJGtKcOTGtbap7kzbXg2xQ8FbQTV1jbWj5B1H
 rGkVbiL7Pj/eMnm5rb2MPXq7MBj3CH1UbUdqu5x9IlAh8GM5bRYQAeKFhF8FI4LBLKRA
 6vLg==
X-Gm-Message-State: AOJu0YyiKPVamhDwI8ryVhT9duXcFHAi7JhrHWZfj9lH8JKO0afyebl6
 lsBZIkHe/F87x0v6J9PcUinkCfvf+yffRDmmxwtihB4mx+y6XS7mmajJucwO
X-Gm-Gg: ASbGncurTEGxhk8Jdvngh0AuzFa5BDwM93tegoUyjGXID4Tk24PeQ+vtX87aZ+yJMy3
 hbLW4k/GA+c/hf2Uv+ymWag8CSw3seh+iPl36d3u9296cwaHlblCXqodJlsbB+wdY6FhaR9+7z/
 RJhm/G5WOtXuHSRStVH+A+72BZGaSRnEcKNzk5o5+ZNDvtXlepd48JoVsmYP2xYddKzviGXGMbV
 JMv8hagzyYp7sWQWrDXFxRQA+Bo4B2wpuTpeamMO7/89VccJtYBX4nIPuZx19RUkpC2dpYP5C8d
 aLKl9HUrvKgErMr7OscO+nS+yW6xYd9N5KMjUr6PAEXDEmIuyBgLig==
X-Google-Smtp-Source: AGHT+IFa/2VFPZCChpaZ0c7h8gnWz4wBu/ER2a0qvBd34qWqMu7ZXFiHeABGxm7ai6z4clI5NEOChQ==
X-Received: by 2002:a17:907:1ca8:b0:ab6:4fa6:71e2 with SMTP id
 a640c23a62f3a-ab789ae2976mr586932266b.22.1738996831317; 
 Fri, 07 Feb 2025 22:40:31 -0800 (PST)
X-Received: by 2002:a05:6000:186d:b0:385:e1eb:a7af with SMTP id
 ffacd0b85a97d-38dc9491e85mr4527770f8f.48.1738996830691; Fri, 07 Feb 2025
 22:40:30 -0800 (PST)
MIME-Version: 1.0
References: <20241224202429.3072813-1-thomas@atskinner.net>
 <20241224202429.3072813-4-thomas@atskinner.net>
 <1737710597.ns84scjebn.astroid@yuna.none>
In-Reply-To: <1737710597.ns84scjebn.astroid@yuna.none>
From: Thomas Skinner <thomas@atskinner.net>
Date: Sat, 8 Feb 2025 00:40:04 -0600
X-Gmail-Original-Message-ID: <CALn9RMdajoXYjMOpS_vrRZk=vkTDKNMj7goMZx-u+aL8e9XDNw@mail.gmail.com>
X-Gm-Features: AWEUYZnHRXthP1Ki-tEtyK0C4f0SFeZogI_wBGYRjUFKPEL3mZS6fL3iaYFWoVo
Message-ID: <CALn9RMdajoXYjMOpS_vrRZk=vkTDKNMj7goMZx-u+aL8e9XDNw@mail.gmail.com>
To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.073 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
 FREEMAIL_FORGED_FROMDOMAIN 0.001 2nd level domains in From and EnvelopeFrom
 freemail headers are different
 FREEMAIL_FROM 0.001 Sender email is commonly abused enduser mail provider
 HEADER_FROM_DIFFERENT_DOMAINS 0.07 From and EnvelopeFrom 2nd level mail
 domains are different
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_DNSWL_NONE     -0.0001 Sender listed at https://www.dnswl.org/,
 no trust RCVD_IN_MSPIKE_H2       0.001 Average reputation (+2)
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 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 v2 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>
Cc: 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>

> do we want to mangle the group names to include the OIDC-realm name,
> like we do for LDAP/AD syncing? that way it is more clear that those
> groups originated from OIDC.. downside is that you can't use a group
> shared between OIDC and other realms..

More on this: it looks like in LDAP/AD sync, the group is suffixed
with `-$realm`, which does meet the requirements of the existing regex
character requirements for groups. We could do the same thing with the
OIDC groups. I will add that suffix in as the option to be consistent.

-----

On a similar but separate note, I think that it may be more clear of
where groups come from to have them optionally suffixed with something
like `@$realm` like is already done for users. The verify function for
groups could be adjusted to validate on a regex that makes the suffix
optional and validates it as `group_regex@realm_regex`. The downside
of this change is that it would break any existing groups or the
module would need to be adjusted to continue to support or migrate the
groups with the old suffix. An upside of this change is eliminating
inadvertent group collisions. For example:

Consider we have 2 realms: AD/LDAP "ad-admins" and OIDC "oidc". Realm
"ad-admins" suffixes groups automatically and realm "oidc" is
configured to not suffix. Realm "ad-admins" contains a group
"privileged" and a realm "oidc" claim sends a group
"privileged-ad-admins".

With the existing configuration, the group "privileged" from realm
"ad-admins" becomes "privileged-ad-admins" in PVE. The user with the
group "privileged-ad-admins" in the OIDC claim is then granted access
to this group on login. This could lead to potential erroneous group
membership because the group names could overlap.

If the suffix was changed to include a character that would not exist
in the group name (e.g. "@"), then it would be impossible to have this
overlap. Additionally, this suffixing option could be extended to any
other realm type that could introduce groups into PVE. I can see
scenarios where having the option to enable/disable group suffixes for
all realms would be useful. If the admin controls all the
authentication services (or at least can ensure no inadvertent
collisions would occur), the suffix is not needed. Non-suffixed group
names could also simplify ACL delegation. If the admin cannot
guarantee inadvertent collisions in group names, then suffixes that do
not include a valid group character would be necessary to prevent
collisions.

If PVE is interested in moving towards this, I'd be happy to start
authoring a patch to support it. If nothing else, I can file a bug and
we can continue discussion there.

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel