[pve-devel] [PATCH SERIES access-control/docs/manager/proxmox-openid v5] fix #4411: add support for openid groups

[pve-devel] [PATCH SERIES access-control/docs/manager/proxmox-openid v5] fix #4411: add support for openid groups

[Thomas Skinner]

Changes since v4:
- remove invalid section from docs

access-control:

Thomas Skinner (1):
  fix #4411: openid: add logic for openid groups support

 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(-)

 
docs:

Thomas Skinner (1):
  fix #4411: openid: add docs for openid groups support

 pveum.adoc | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

 
manager:

Thomas Skinner (1):
  fix #4411: openid: add ui config for openid groups support

 www/manager6/dc/AuthEditOpenId.js | 34 ++++++++++++++++++++++++++++---


proxmox-openid:

Thomas Skinner (1):
  fix #4411: openid: add library code for generic id token claim support

 proxmox-openid/src/lib.rs | 55 +++++++++++++++++++++++++++++++++------
-- 
2.39.5


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

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

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 pveum.adoc | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/pveum.adoc b/pveum.adoc
index 81565ab..3b76aca 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -456,6 +456,15 @@ use the `autocreate` option to automatically add new users.
 * `Username Claim` (`username-claim`): OpenID claim used to generate the unique
 username (`subject`, `username` or `email`).
 
+* `Autocreate Groups` (`groups-autocreate`): Create all groups in the claim
+instead of using existing PVE groups (default behavior).
+
+* `Groups Claim` (`groups-claim`): OpenID claim used to retrieve the groups from
+the ID token or userinfo endpoint.
+
+* `Overwrite Groups` (`groups-overwrite`): Overwrite all groups assigned to user
+instead of appending to existing groups (default behavior).
+
 Username mapping
 ^^^^^^^^^^^^^^^^
 
@@ -479,6 +488,31 @@ Another option is to use `email`, which also yields human readable
 usernames. Again, only use this setting if the server guarantees the
 uniqueness of this attribute.
 
+Groups mapping
+^^^^^^^^^^^^^^
+
+Specifying the `groups-claim` setting in the OpenID configuration enables group
+mapping functionality. The data provided in the `groups-claim` should be
+a list of strings that correspond to groups that a user should be a member of in
+{pve}. To prevent collisions, group names from the OpenID claim are suffixed 
+with `-<realm name>` (e.g. for the OpenID group name `my-openid-group` in the 
+realm `oidc`, the group name in {pve} would be `my-openid-group-oidc`).
+
+Any groups reported by the OpenID provider that do not exist in {pve} are
+ignored by default. If all groups reported by the OpenID provider should exist
+in {pve}, the `groups-autocreate` option may be used to automatically create
+these groups on user logins.
+
+By default, groups are appended to the user's existing groups. It may be 
+desirable to overwrite any groups that the user is already a member in {pve}
+with those from the OpenID provider. Enabling the `groups-overwrite` setting
+removes all groups from the user in {pve} before adding the groups reported by
+the OpenID provider.
+
+In some cases, OpenID servers may send groups claims which include invalid
+characters for {pve} group IDs. Any groups that contain characters not allowed
+in a {pve} group name are not included and a warning will be sent to the logs.
+
 Examples
 ^^^^^^^^
 
-- 
2.39.5


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

[pve-devel] [PATCH proxmox-openid v5 1/1] fix #4411: openid: add library code for generic id token claim support

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 proxmox-openid/src/lib.rs | 55 +++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index fe65fded..bf8c650b 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -15,8 +15,11 @@ pub use auth_state::*;
 use openidconnect::{
     //curl::http_client,
     core::{
-        CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreClient, CoreGenderClaim,
-        CoreIdTokenClaims, CoreIdTokenVerifier, CoreProviderMetadata,
+        CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreErrorResponseType,
+        CoreGenderClaim, CoreIdTokenVerifier, CoreJsonWebKey, CoreJsonWebKeyType,
+        CoreJsonWebKeyUse, CoreJweContentEncryptionAlgorithm, CoreJwsSigningAlgorithm,
+        CoreProviderMetadata, CoreRevocableToken, CoreRevocationErrorResponse,
+        CoreTokenIntrospectionResponse, CoreTokenType,
     },
     AdditionalClaims,
     AuthenticationContextClass,
@@ -24,6 +27,9 @@ use openidconnect::{
     ClientId,
     ClientSecret,
     CsrfToken,
+    EmptyExtraTokenFields,
+    IdTokenClaims,
+    IdTokenFields,
     IssuerUrl,
     Nonce,
     OAuth2TokenResponse,
@@ -31,15 +37,47 @@ use openidconnect::{
     PkceCodeVerifier,
     RedirectUrl,
     Scope,
+    StandardErrorResponse,
+    StandardTokenResponse,
     UserInfoClaims,
 };
 
 /// Stores Additional Claims into a serde_json::Value;
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
 pub struct GenericClaims(Value);
 impl AdditionalClaims for GenericClaims {}
 
 pub type GenericUserInfoClaims = UserInfoClaims<GenericClaims, CoreGenderClaim>;
+pub type GenericIdTokenClaims = IdTokenClaims<GenericClaims, CoreGenderClaim>;
+
+pub type GenericIdTokenFields = IdTokenFields<
+    GenericClaims,
+    EmptyExtraTokenFields,
+    CoreGenderClaim,
+    CoreJweContentEncryptionAlgorithm,
+    CoreJwsSigningAlgorithm,
+    CoreJsonWebKeyType,
+>;
+
+pub type GenericTokenResponse = StandardTokenResponse<GenericIdTokenFields, CoreTokenType>;
+
+pub type GenericClient = openidconnect::Client<
+    GenericClaims,
+    CoreAuthDisplay,
+    CoreGenderClaim,
+    CoreJweContentEncryptionAlgorithm,
+    CoreJwsSigningAlgorithm,
+    CoreJsonWebKeyType,
+    CoreJsonWebKeyUse,
+    CoreJsonWebKey,
+    CoreAuthPrompt,
+    StandardErrorResponse<CoreErrorResponseType>,
+    GenericTokenResponse,
+    CoreTokenType,
+    CoreTokenIntrospectionResponse,
+    CoreRevocableToken,
+    CoreRevocationErrorResponse,
+>;
 
 #[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct OpenIdConfig {
@@ -56,7 +94,7 @@ pub struct OpenIdConfig {
 }
 
 pub struct OpenIdAuthenticator {
-    client: CoreClient,
+    client: GenericClient,
     config: OpenIdConfig,
 }
 
@@ -120,8 +158,9 @@ impl OpenIdAuthenticator {
 
         let provider_metadata = CoreProviderMetadata::discover(&issuer_url, http_client)?;
 
-        let client = CoreClient::from_provider_metadata(provider_metadata, client_id, client_key)
-            .set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
+        let client =
+            GenericClient::from_provider_metadata(provider_metadata, client_id, client_key)
+                .set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
 
         Ok(Self {
             client,
@@ -195,7 +234,7 @@ impl OpenIdAuthenticator {
         &self,
         code: &str,
         private_auth_state: &PrivateAuthState,
-    ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
+    ) -> Result<(GenericIdTokenClaims, GenericUserInfoClaims), Error> {
         let code = AuthorizationCode::new(code.to_string());
         // Exchange the code with a token.
         let token_response = self
@@ -206,7 +245,7 @@ impl OpenIdAuthenticator {
             .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?;
 
         let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier();
-        let id_token_claims: &CoreIdTokenClaims = token_response
+        let id_token_claims: &GenericIdTokenClaims = token_response
             .extra_fields()
             .id_token()
             .expect("Server did not return an ID token")
-- 
2.39.5


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

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

[Thomas Skinner]

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

[pve-devel] [PATCH manager v5 1/1] fix #4411: openid: add ui config for openid groups support

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 www/manager6/dc/AuthEditOpenId.js | 34 ++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/www/manager6/dc/AuthEditOpenId.js b/www/manager6/dc/AuthEditOpenId.js
index 544c0de5..bf9c0ed4 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -40,6 +40,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
 	    },
 	    name: 'client-key',
 	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'scopes',
+	    fieldLabel: gettext('Scopes'),
+	    emptyText: `${Proxmox.Utils.defaultText} (email profile)`,
+	    submitEmpty: false,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
     ],
 
     column2: [
@@ -72,16 +82,34 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
 		editable: '{isCreate}',
 	    },
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Autocreate Groups'),
+	    name: 'groups-autocreate',
+	    value: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
-	    name: 'scopes',
-	    fieldLabel: gettext('Scopes'),
-	    emptyText: `${Proxmox.Utils.defaultText} (email profile)`,
+	    name: 'groups-claim',
+	    fieldLabel: gettext('Groups Claim'),
+	    emptyText: `${Proxmox.Utils.defaultText} ${gettext('(none)')}`,
 	    submitEmpty: false,
 	    cbind: {
 		deleteEmpty: '{!isCreate}',
 	    },
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Overwrite Groups'),
+	    name: 'groups-overwrite',
+	    value: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxKVComboBox',
 	    name: 'prompt',
-- 
2.39.5


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

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

[Mira Limbeck]

On 3/27/25 02:50, Thomas Skinner wrote:
> 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', 
trailing whitespace

> +					"openid group '$group' contains invalid characters"
> +				    );
> +				}
> +			    }
> +			    
trailing whitespace

> +			    # 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;
both lines above have wrong indentation (space only)

> +				my $groups_to_create;
> +				for my $group (keys %$oidc_groups) {
> +                                    $groups_to_create->{$group} = 1 if !$usercfg->{groups}->{$group};
> +			    	}
wrong combination of tabs and spaces, tabs - spaces - tab

> +				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);

There are some trailing whitespaces, wrongly mixed tabs and spaces, and
some space-only indentations.
The indentation scheme used [0] is not that straightforward. It can be
fixed up when applying the patch, so no need to send a v6 unless there
are some other issues.

Other than the whitespace issues the code looks good.
So consider this:
Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>



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

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

[Mira Limbeck]

forgot the link [0] in my previous reply:
https://pve.proxmox.com/wiki/Perl_Style_Guide


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

Re: [pve-devel] [PATCH SERIES access-control/docs/manager/proxmox-openid v5] fix #4411: add support for openid groups

[Mira Limbeck]

On 3/27/25 02:49, Thomas Skinner wrote:
> Changes since v4:
> - remove invalid section from docs
> 
> access-control:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add logic for openid groups support
> 
>  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(-)
> 
>  
> docs:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add docs for openid groups support
> 
>  pveum.adoc | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
>  
> manager:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add ui config for openid groups support
> 
>  www/manager6/dc/AuthEditOpenId.js | 34 ++++++++++++++++++++++++++++---
> 
> 
> proxmox-openid:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add library code for generic id token claim support
> 
>  proxmox-openid/src/lib.rs | 55 +++++++++++++++++++++++++++++++++------

Works as expected, tested with Authentik:

Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>


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

[pve-devel] applied-series: [PATCH SERIES access-control/docs/manager/proxmox-openid v5] fix #4411: add support for openid groups

[Fabian Grünbichler]

with some small fix- and follow-ups, thanks!

Quoting Thomas Skinner (2025-03-27 02:49:58)
> Changes since v4:
> - remove invalid section from docs
> 
> access-control:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add logic for openid groups support
> 
>  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(-)
> 
>  
> docs:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add docs for openid groups support
> 
>  pveum.adoc | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
>  
> manager:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add ui config for openid groups support
> 
>  www/manager6/dc/AuthEditOpenId.js | 34 ++++++++++++++++++++++++++++---
> 
> 
> proxmox-openid:
> 
> Thomas Skinner (1):
>   fix #4411: openid: add library code for generic id token claim support
> 
>  proxmox-openid/src/lib.rs | 55 +++++++++++++++++++++++++++++++++------
> -- 
> 2.39.5
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


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