From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pmg-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 563541FF16F
	for <inbox@lore.proxmox.com>; Thu, 27 Feb 2025 10:52:03 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 1112D2E469;
	Thu, 27 Feb 2025 10:52:02 +0100 (CET)
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pmg-devel@lists.proxmox.com
Date: Thu, 27 Feb 2025 10:51:52 +0100
Message-Id: <20250227095152.281531-1-t.lamprecht@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.039 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: [pmg-devel] applied: [PATCH api] oidc realm: add more flexiple
 access-role assignment
X-BeenThere: pmg-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Mail Gateway development discussion
 <pmg-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pmg-devel>, 
 <mailto:pmg-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pmg-devel/>
List-Post: <mailto:pmg-devel@lists.proxmox.com>
List-Help: <mailto:pmg-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel>, 
 <mailto:pmg-devel-request@lists.proxmox.com?subject=subscribe>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pmg-devel-bounces@lists.proxmox.com
Sender: "pmg-devel" <pmg-devel-bounces@lists.proxmox.com>

deprecate the fixed role assignment property and replace it with one
that allows to select the source of the role, for now allow static,
which is equivalent with what we had, and also to query the role from
a role claim, so that the OIDC provider can return the correct role
inside the returned user information.

Tested the dynamic role assignment with keycload by setting up an
'User Attribute' Mapper for a client and added added the respective
user attribute to some test user.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

It's not much code and most is schema and parsing, the implementation
itself is pretty straigt forward, basically the only questionable thing
is naming of properties.

 src/PMG/API2/OIDC.pm | 26 ++++++++++++++++++++-
 src/PMG/Auth/OIDC.pm | 54 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/src/PMG/API2/OIDC.pm b/src/PMG/API2/OIDC.pm
index 92ff88d..20acc41 100644
--- a/src/PMG/API2/OIDC.pm
+++ b/src/PMG/API2/OIDC.pm
@@ -13,6 +13,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 
 use PMG::AccessControl;
+use PMG::Auth::OIDC;
 use PMG::Auth::Plugin;
 use PMG::RESTEnvironment;
 
@@ -204,7 +205,30 @@ __PACKAGE__->register_method ({
 		    if (defined(my $family_name = $info->{'family_name'})) {
 			$entry->{lastname} = $family_name;
 		    }
-		    $entry->{role} = $config->{'autocreate-role'} // 'audit';
+
+		    # NOTE: 'autocreate-role' is deprecated and has less priority than the more
+		    # flexible 'autocreate-role-assignment'
+		    $entry->{role} = $config->{'autocreate-role'} // 'audit'; # default
+		    if (my $role_assignment_raw = $config->{'autocreate-role-assignment'}) {
+			my $role_assignment =
+			    PMG::Auth::OIDC::parse_autocreate_role_assignment($role_assignment_raw);
+
+			if ($role_assignment->{source} eq 'fixed') {
+			    $entry->{role} = $role_assignment->{'fixed-role'};
+			} elsif ($role_assignment->{source} eq 'from-claim') {
+			    my $role_attr = $role_assignment->{'role-claim'};
+			    if (my $role = $info->{$role_attr}) {
+				$role = lc($role); # normalize to lower-case
+				die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n"
+				    if $role !~ /^(?:admin|qmanager|audit|helpdesk)$/;
+				$entry->{role} = $role;
+			    } else {
+				die "required '$role_attr' role-claim attribute not found, cannot autocreate user\n";
+			    }
+			} else {
+			    die "unkown role assignment source '$role_assignment->{source}' - implement me";
+			}
+		    }
 		    $entry->{userid} = $username;
 		    $entry->{username} = $unique_name;
 		    $entry->{realm} = $realm;
diff --git a/src/PMG/Auth/OIDC.pm b/src/PMG/Auth/OIDC.pm
index 4129d47..9082c75 100755
--- a/src/PMG/Auth/OIDC.pm
+++ b/src/PMG/Auth/OIDC.pm
@@ -4,6 +4,8 @@ use strict;
 use warnings;
 
 use PVE::Tools;
+use PVE::JSONSchema qw(parse_property_string);
+
 use PMG::Auth::Plugin;
 
 use base qw(PMG::Auth::Plugin);
@@ -12,6 +14,44 @@ sub type {
     return 'oidc';
 }
 
+my $autocreate_role_assignment_format = {
+    source => {
+	type => 'string',
+	enum => ['fixed', 'from-claim'],
+	default => 'fixed',
+	description => "How the access role for a newly auto-created user should be selected.",
+    },
+    'fixed-role' => {
+	type => 'string',
+	enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
+	default => 'audit',
+	optional => 1,
+	description => "The fixed role that should be assigned to auto-created users.",
+    },
+    'role-claim' => {
+	description => "OIDC claim used to assign the unique username.",
+	type => 'string',
+	default => 'role',
+	optional => 1,
+	pattern => qr/^[a-zA-Z0-9._:-]+$/,
+    },
+};
+
+
+sub parse_autocreate_role_assignment {
+    my ($raw) = @_;
+    return undef if !$raw or !length($raw);
+
+    my $role_assignment = parse_property_string($autocreate_role_assignment_format, $raw);
+    $role_assignment->{'fixed-role'} = 'audit'
+	if $role_assignment->{'source'} eq 'fixed' && !defined($role_assignment->{'fixed-role'});
+
+    $role_assignment->{'role-claim'} = 'role'
+	if $role_assignment->{'source'} eq 'from-clain' && !defined($role_assignment->{'role-claim'});
+
+    return $role_assignment;
+}
+
 sub properties {
     return {
 	'issuer-url' => {
@@ -39,13 +79,20 @@ sub properties {
 	    type => 'boolean',
 	    default => 0,
 	},
-	'autocreate-role' => {
-	    description => "Automatically create users with a specific role.",
+	'autocreate-role' => { # NOTE: depreacated since the beginning, just here for compat
+	    description => "Automatically create users with a specific role."
+		." NOTE: Depreacated, favor 'autocreate-role-assignment'",
 	    type => 'string',
 	    enum => ['admin', 'qmanager', 'audit', 'helpdesk'],
 	    default => 'audit',
 	    optional => 1,
 	},
+	'autocreate-role-assignment' => {
+	    description => "Defines which role should be assigned to auto-created users.",
+	    type => 'string', format => $autocreate_role_assignment_format,
+	    default => 'source=fixed,fixed-role=auditor',
+	    optional => 1,
+	},
 	'username-claim' => {
 	    description => "OpenID Connect claim used to generate the unique username.",
 	    type => 'string',
@@ -84,7 +131,8 @@ sub options {
 	'client-id' => {},
 	'client-key' => { optional => 1 },
 	autocreate => { optional => 1 },
-	'autocreate-role' => { optional => 1 },
+	'autocreate-role' => { optional => 1 }, # NOTE: depreacated in favor of 'autocreate-role-assignment'
+	'autocreate-role-assignment' => { optional => 1 },
 	'username-claim' => { optional => 1, fixed => 1 },
 	prompt => { optional => 1 },
 	scopes => { optional => 1 },
-- 
2.39.5



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