[pve-devel] [PATCH access-control v2 1/1] fix #4411: openid: add logic for openid groups support

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas at atskinner.net>
---
 src/PVE/API2/OpenId.pm   | 68 ++++++++++++++++++++++++++++++++++++++++
 src/PVE/AccessControl.pm | 13 +++++---
 src/PVE/Auth/OpenId.pm   | 30 ++++++++++++++++++
 3 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..5cfe5a1 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,73 @@ __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_list = map { 
+				$_ =~ s/[^$PVE::Auth::OpenId::groupname_regex_chars]/$replace_character/gr 
+			    } $groups_list->@*;
+
+			    # list groups that exist in pve
+			    my @existing_groups_list = keys %{$usercfg->{groups}};
+
+			    my @groups_intersect;
+			    if ( $config->{'groups-autocreate'} ) {
+				# populate all groups in claim
+				@groups_intersect = @oidc_groups_list;
+			    }
+			    else {
+				# only populate groups that are in the oidc list and exist in pve
+				@groups_intersect = @{PVE::Tools::array_intersect(
+				    \@oidc_groups_list,
+				    \@existing_groups_list,
+				)};
+			    }
+
+			    # if groups should be overwritten, find and delete the ones to remove
+			    if ( $config->{'groups-overwrite'} ) {
+				# get the groups that need to be removed from the user
+				my %groups_remove_user;
+				$groups_remove_user{ $_ } = undef 
+				    for keys %{$usercfg->{users}->{$username}->{groups}};
+				delete $groups_remove_user{ $_ } for @groups_intersect;
+				
+				# ensure user is not a member of these groups
+				PVE::AccessControl::delete_user_group_single(
+				    $username, 
+				    $usercfg, 
+				    $_,
+				) for keys %groups_remove_user;
+			    }
+
+			    # get the groups that need to be added to the user
+			    my %groups_add_user;
+			    $groups_add_user{ $_ } = undef for @groups_intersect;
+			    delete $groups_add_user{ $_ } 
+			        for keys %{$usercfg->{users}->{$username}->{groups}};
+			    
+			    # ensure user is a member of these groups
+			    PVE::AccessControl::add_user_group(
+				$username,
+			    	$usercfg,
+				$_
+			    ) for keys %groups_add_user;
+
+			    cfs_write_file("user.cfg", $usercfg);
+			}, "openid group mapping failed");
+		    } else {
+			syslog('err', "groups list is not an array; groups will not be updated");
+		    }
+		} else {
+		    syslog('err', "groups claim '$groups_claim' is not found in claims");
+		}
+	    }
+
 	    my $ticket = PVE::AccessControl::assemble_ticket($username);
 	    my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
 	    my $cap = $rpcenv->compute_api_permission($username);
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 47f2d38..d643a00 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -990,12 +990,17 @@ sub delete_user_group {
     my ($username, $usercfg) = @_;
 
     foreach my $group (keys %{$usercfg->{groups}}) {
-
-	delete ($usercfg->{groups}->{$group}->{users}->{$username})
-	    if $usercfg->{groups}->{$group}->{users}->{$username};
+	delete_user_group_single($username, $usercfg, $group);
     }
 }
 
+sub delete_user_group_single {
+    my ($username, $usercfg, $group) = @_;
+
+    delete ($usercfg->{groups}->{$group}->{users}->{$username})
+	if $usercfg->{groups}->{$group}->{users}->{$username};
+}
+
 sub delete_user_acl {
     my ($username, $usercfg) = @_;
 
@@ -1293,7 +1298,7 @@ PVE::JSONSchema::register_format('pve-groupid', \&verify_groupname);
 sub verify_groupname {
     my ($groupname, $noerr) = @_;
 
-    if ($groupname !~ m/^[A-Za-z0-9\.\-_]+$/) {
+    if ($groupname !~ m/^[$PVE::Auth::OpenId::groupname_regex_chars]+$/) {
 
 	die "group name '$groupname' contains invalid characters\n" if !$noerr;
 
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..d7b5574 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -9,6 +9,8 @@ use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file
 
 use base qw(PVE::Auth::Plugin);
 
+our $groupname_regex_chars = qr/A-Za-z0-9\.\-_/;
+
 sub type {
     return 'openid';
 }
@@ -42,6 +44,30 @@ sub properties {
 	    type => 'string',
 	    optional => 1,
 	},
+	"groups-claim" => {
+	    description => "OpenID claim used to retrieve groups with.",
+	    type => 'string',
+	    optional => 1,
+	},
+	"groups-autocreate" => {
+	    description => "Automatically create groups if they do not exist.",
+	    optional => 1,
+	    type => 'boolean',
+	    default => 0,
+	},
+	"groups-overwrite" => {
+	    description => "All groups will be overwritten for the user on login.",
+	    type => 'boolean',
+	    default => 0,
+	    optional => 1,
+	},
+	"groups-replace-character" => {
+	    description => "Character used to replace any invalid characters in groups from provider.",
+	    type => 'string',
+	    pattern => qr/^[$groupname_regex_chars]$/,
+	    default => '_',
+	    optional => 1,
+	},
 	prompt => {
 	    description => "Specifies whether the Authorization Server prompts the End-User for"
 	        ." reauthentication and consent.",
@@ -73,6 +99,10 @@ sub options {
 	"client-key" => { optional => 1 },
 	autocreate => { optional => 1 },
 	"username-claim" => { optional => 1, fixed => 1 },
+	"groups-claim" => { optional => 1 },
+	"groups-autocreate" => { optional => 1 },
+	"groups-overwrite" => { optional => 1 },
+	"groups-replace-character" => { optional => 1},
 	prompt => { optional => 1 },
 	scopes => { optional => 1 },
 	"acr-values" => { optional => 1 },
-- 
2.39.5