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

[Thomas Skinner <thomas@atskinner.net>]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 src/PVE/API2/OpenId.pm   | 83 ++++++++++++++++++++++++++++++++++++++++
 src/PVE/AccessControl.pm |  2 +-
 src/PVE/Auth/OpenId.pm   | 25 ++++++++++++
 src/PVE/Auth/Plugin.pm   |  1 +
 4 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..baa4dbc 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,88 @@ __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");
+
+			    my $oidc_groups;
+			    for my $group (@$groups_list) {
+				if (PVE::AccessControl::verify_groupname($group, 1)) {
+				    # add realm name as suffix to group
+				    $oidc_groups->{"$group-$realm"} = 1;
+				} else {
+				    # ignore any groups in the list that have invalid characters
+				    syslog(
+					'warn', 
+					"openid group '$group' contains invalid characters"
+				    );
+				}
+			    }
+			    
+			    # get groups that exist in OIDC and PVE
+			    my $groups_intersect;
+			    for my $group (keys %$oidc_groups) {
+				$groups_intersect->{$group} = 1 if $usercfg->{groups}->{$group};
+			    }
+
+			    if ($config->{'groups-autocreate'}) {
+                                # populate all groups in claim
+                                $groups_intersect = $oidc_groups;
+				my $groups_to_create;
+				for my $group (keys %$oidc_groups) {
+                                    $groups_to_create->{$group} = 1 if !$usercfg->{groups}->{$group};
+			    	}
+				if ($groups_to_create) {
+				    # log a messages about created groups here
+				    my $groups_to_create_string = join(', ', sort keys %$groups_to_create);
+				    syslog(
+					'info', 
+					"groups created automatically from openid claim: $groups_to_create_string"
+				    );
+				}
+                            }
+
+			    # if groups should be overwritten, delete all the users groups first
+			    if ( $config->{'groups-overwrite'} ) {
+				PVE::AccessControl::delete_user_group(
+				    $username, 
+				    $usercfg, 
+				);
+				syslog(
+				    'info', 
+				    "openid overwrite groups enabled; user '$username' removed from all groups"
+			    	);
+			    }
+			    
+			    if (keys %$groups_intersect) {
+				# ensure user is a member of the groups
+				for my $group (keys %$groups_intersect) {
+				    PVE::AccessControl::add_user_group(
+					$username,
+					$usercfg,
+					$group
+				    );
+				}
+
+				my $groups_intersect_string = join(', ', sort keys %$groups_intersect);
+				syslog(
+				    'info', 
+				    "openid user '$username' added to groups: $groups_intersect_string"
+				);
+			    }
+
+			    cfs_write_file("user.cfg", $usercfg);
+			}, "openid group mapping failed");
+		    } else {
+			syslog('err', "openid groups list is not an array; groups will not be updated");
+		    }
+		} else {
+		    syslog('err', "openid 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..7493c57 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1293,7 +1293,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::Plugin::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..4c52adc 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -9,6 +9,9 @@ use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file
 
 use base qw(PVE::Auth::Plugin);
 
+# include all printable ascii characters
+my $openid_claim_regex = qr/[ -~]+/;
+
 sub type {
     return 'openid';
 }
@@ -42,6 +45,25 @@ sub properties {
 	    type => 'string',
 	    optional => 1,
 	},
+	"groups-claim" => {
+	    description => "OpenID claim used to retrieve groups with.",
+	    type => 'string',
+	    pattern => $openid_claim_regex,
+	    maxLength => 256,
+	    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,
+	},
 	prompt => {
 	    description => "Specifies whether the Authorization Server prompts the End-User for"
 	        ." reauthentication and consent.",
@@ -73,6 +95,9 @@ 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 },
 	prompt => { optional => 1 },
 	scopes => { optional => 1 },
 	"acr-values" => { optional => 1 },
diff --git a/src/PVE/Auth/Plugin.pm b/src/PVE/Auth/Plugin.pm
index 763239f..7617044 100755
--- a/src/PVE/Auth/Plugin.pm
+++ b/src/PVE/Auth/Plugin.pm
@@ -31,6 +31,7 @@ sub lock_domain_config {
 
 our $realm_regex = qr/[A-Za-z][A-Za-z0-9\.\-_]+/;
 our $user_regex = qr![^\s:/]+!;
+our $groupname_regex_chars = qr/A-Za-z0-9\.\-_/;
 
 PVE::JSONSchema::register_format('pve-realm', \&pve_verify_realm);
 sub pve_verify_realm {
-- 
2.39.5


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