[pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v4] Make OIDC userinfo endpoint optional

[pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v4] Make OIDC userinfo endpoint optional

[Thomas Skinner]

Changes since v3:
- adjust option to "query userinfo endpoint" with default enabled

access-control:

Thomas Skinner (1):
  fix #4234: add library functions for openid optional userinfo request

 src/PVE/API2/OpenId.pm | 6 +++++-
 src/PVE/Auth/OpenId.pm | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

 
docs:

Thomas Skinner (1):
  fix #4234: add docs for openid optional userinfo request

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

 
manager:

Thomas Skinner (1):
  fix #4234: add GUI option for openid optional userinfo request

 www/manager6/dc/AuthEditOpenId.js | 10 ++++++++++
 1 file changed, 10 insertions(+)

 
perl-rs:

Thomas Skinner (1):
  fix #4234: openid: adjust openid verification function for userinfo
    option

 pve-rs/src/openid/mod.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

 
proxmox-openid:

Thomas Skinner (1):
  fix #4234: openid: add library functions for optional userinfo
    endpoint

 proxmox-openid/src/lib.rs | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

 
-- 
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 v4 1/1] fix #4234: add docs for openid optional userinfo request

[Thomas Skinner]

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

diff --git a/pveum.adoc b/pveum.adoc
index 81565ab..0d74d1d 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -479,6 +479,14 @@ 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.
 
+Advanced settings
+^^^^^^^^^^^^^^^^^
+
+* `Query userinfo endpoint` (`query-userinfo`): Enabling this option requires
+the OpenID Connect authenticator to query the "userinfo" endpoint for claim
+values. Disabling this option is useful for some identity providers that do not
+support the "userinfo" endpoint (e.g. ADFS).
+
 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 manager v4 1/1] fix #4234: add GUI option for openid optional userinfo request

[Thomas Skinner]

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

diff --git a/www/manager6/dc/AuthEditOpenId.js b/www/manager6/dc/AuthEditOpenId.js
index 544c0de5..7687652d 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -111,6 +111,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
 		deleteEmpty: '{!isCreate}',
 	    },
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Query userinfo endpoint'),
+	    name: 'query-userinfo',
+	    checked: true,
+	    uncheckedValue: 0,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
     ],
 
     initComponent: function() {
-- 
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 v4 1/1] fix #4234: add library functions for openid optional userinfo request

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 src/PVE/API2/OpenId.pm | 6 +++++-
 src/PVE/Auth/OpenId.pm | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index 77410e6..f70d242 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -170,7 +170,11 @@ __PACKAGE__->register_method ({
 
 	    my ($config, $openid) = $lookup_openid_auth->($realm, $redirect_url);
 
-	    my $info = $openid->verify_authorization_code($param->{code}, $private_auth_state);
+	    my $info = $openid->verify_authorization_code(
+		$param->{code},
+		$private_auth_state,
+		$config->{'query-userinfo'} // 1,
+	    );
 	    my $subject = $info->{'sub'};
 
 	    my $unique_name;
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..cf74229 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -63,6 +63,12 @@ sub properties {
 	    pattern => '^[^\x00-\x1F\x7F <>#"]*$', # Prohibit characters not allowed in URI RFC 2396.
 	    optional => 1,
 	},
+	"query-userinfo" => {
+	    description => "Enables querying the userinfo endpoint for claims values.",
+	    type => 'boolean',
+	    default => 1,
+	    optional => 1,
+	},
    };
 }
 
@@ -78,6 +84,7 @@ sub options {
 	"acr-values" => { optional => 1 },
 	default => { optional => 1 },
 	comment => { optional => 1 },
+	"query-userinfo" => { optional => 1 },
     };
 }
 
-- 
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 v4 1/1] fix #4234: openid: add library functions for optional userinfo endpoint

[Thomas Skinner]

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

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index fe65fded..b36172c7 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -31,6 +31,7 @@ use openidconnect::{
     PkceCodeVerifier,
     RedirectUrl,
     Scope,
+    StandardClaims,
     UserInfoClaims,
 };
 
@@ -195,6 +196,15 @@ impl OpenIdAuthenticator {
         &self,
         code: &str,
         private_auth_state: &PrivateAuthState,
+    ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
+        self.verify_authorization_code_userinfo(code, private_auth_state, true)
+    }
+
+    pub fn verify_authorization_code_userinfo(
+        &self,
+        code: &str,
+        private_auth_state: &PrivateAuthState,
+        query_userinfo: bool,
     ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
         let code = AuthorizationCode::new(code.to_string());
         // Exchange the code with a token.
@@ -213,6 +223,14 @@ impl OpenIdAuthenticator {
             .claims(&id_token_verifier, &private_auth_state.nonce)
             .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
 
+        if !query_userinfo {
+            let empty_userinfo_claims = UserInfoClaims::new(
+                StandardClaims::new(id_token_claims.subject().clone()),
+                GenericClaims(Value::Null),
+            );
+            return Ok((id_token_claims.clone(), empty_userinfo_claims));
+        }
+
         let userinfo_claims: GenericUserInfoClaims = self
             .client
             .user_info(token_response.access_token().to_owned(), None)?
@@ -227,9 +245,19 @@ impl OpenIdAuthenticator {
         &self,
         code: &str,
         private_auth_state: &PrivateAuthState,
+    ) -> Result<Value, Error> {
+        self.verify_authorization_code_simple_userinfo(code, private_auth_state, true)
+    }
+
+    /// Like verify_authorization_code_simple_userinfo(), but returns claims as serde_json::Value
+    pub fn verify_authorization_code_simple_userinfo(
+        &self,
+        code: &str,
+        private_auth_state: &PrivateAuthState,
+        query_userinfo: bool,
     ) -> Result<Value, Error> {
         let (id_token_claims, userinfo_claims) =
-            self.verify_authorization_code(code, private_auth_state)?;
+            self.verify_authorization_code_userinfo(code, private_auth_state, query_userinfo)?;
 
         let mut data = serde_json::to_value(id_token_claims)?;
 
-- 
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 perl-rs v4 1/1] fix #4234: openid: adjust openid verification function for userinfo option

[Thomas Skinner]

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
 pve-rs/src/openid/mod.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
index 1fa7572..095ef26 100644
--- a/pve-rs/src/openid/mod.rs
+++ b/pve-rs/src/openid/mod.rs
@@ -54,9 +54,14 @@ mod export {
         #[try_from_ref] this: &OpenId,
         code: &str,
         private_auth_state: PrivateAuthState,
+        query_userinfo: Option<bool>,
     ) -> Result<Value, Error> {
         let open_id = this.inner.lock().unwrap();
-        let claims = open_id.verify_authorization_code_simple(code, &private_auth_state)?;
+        let claims = open_id.verify_authorization_code_simple_userinfo(
+            code,
+            &private_auth_state,
+            query_userinfo.unwrap_or(true),
+        )?;
 
         Ok(to_value(&claims)?)
     }
-- 
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 perl-rs v4 1/1] fix #4234: openid: adjust openid verification function for userinfo option

[Mira Limbeck]

On 3/24/25 04:37, Thomas Skinner wrote:
> Signed-off-by: Thomas Skinner <thomas@atskinner.net>
> ---
>  pve-rs/src/openid/mod.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
> index 1fa7572..095ef26 100644
> --- a/pve-rs/src/openid/mod.rs
> +++ b/pve-rs/src/openid/mod.rs
> @@ -54,9 +54,14 @@ mod export {
>          #[try_from_ref] this: &OpenId,
>          code: &str,
>          private_auth_state: PrivateAuthState,
> +        query_userinfo: Option<bool>,
>      ) -> Result<Value, Error> {
>          let open_id = this.inner.lock().unwrap();
> -        let claims = open_id.verify_authorization_code_simple(code, &private_auth_state)?;
> +        let claims = open_id.verify_authorization_code_simple_userinfo(
> +            code,
> +            &private_auth_state,
> +            query_userinfo.unwrap_or(true),
> +        )?;
>  
>          Ok(to_value(&claims)?)
>      }

As mentioned in my reply to version v3, the code has changed in commit
`9ee9ad4` which moved the code to common/src/oidc.
Please rebase this patch on top of the current proxmox-perl-rs master
branch.
You'll have to make changes in both common/src/oidc as well as
pve-rs/src/openid/mod.rs.


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

[pve-devel] [PATCH v4 perl-rs] fix #4234: openid: adjust openid verification function for userinfo option

[Mira Limbeck]

From: Thomas Skinner <thomas@atskinner.net>

Signed-off-by: Thomas Skinner <thomas@atskinner.net>
[ML: rebased on master, added code to common and pve-rs forwarding]
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
PMG uses the functions as well, depending on how perlmod handles
unspecified parameters, those callsites may need to be updated.

 common/src/oidc/mod.rs   | 7 ++++++-
 pve-rs/src/openid/mod.rs | 3 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/common/src/oidc/mod.rs b/common/src/oidc/mod.rs
index 29791bc..1b398d6 100644
--- a/common/src/oidc/mod.rs
+++ b/common/src/oidc/mod.rs
@@ -54,9 +54,14 @@ pub mod export {
         #[try_from_ref] this: &OIDC,
         code: &str,
         private_auth_state: PrivateAuthState,
+        query_userinfo: Option<bool>,
     ) -> Result<Value, Error> {
         let oidc = this.inner.lock().unwrap();
-        let claims = oidc.verify_authorization_code_simple(code, &private_auth_state)?;
+        let claims = oidc.verify_authorization_code_simple_userinfo(
+            code,
+            &private_auth_state,
+            query_userinfo.unwrap_or(true),
+        )?;
 
         Ok(to_value(&claims)?)
     }
diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
index 2adb8bb..f18639c 100644
--- a/pve-rs/src/openid/mod.rs
+++ b/pve-rs/src/openid/mod.rs
@@ -41,7 +41,8 @@ mod export {
         #[try_from_ref] this: &OpenId,
         code: &str,
         private_auth_state: PrivateAuthState,
+        query_userinfo: Option<bool>,
     ) -> Result<Value, Error> {
-        common::verify_authorization_code(this, code, private_auth_state)
+        common::verify_authorization_code(this, code, private_auth_state, query_userinfo)
     }
 }
-- 
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 SERIES access-control/docs/manager/perl-rs/proxmox-openid v4] Make OIDC userinfo endpoint optional

[Mira Limbeck]

On 3/24/25 04:37, Thomas Skinner wrote:
> Changes since v3:
> - adjust option to "query userinfo endpoint" with default enabled
> 
> access-control:
> 
> Thomas Skinner (1):
>   fix #4234: add library functions for openid optional userinfo request
> 
>  src/PVE/API2/OpenId.pm | 6 +++++-
>  src/PVE/Auth/OpenId.pm | 7 +++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
>  
> docs:
> 
> Thomas Skinner (1):
>   fix #4234: add docs for openid optional userinfo request
> 
>  pveum.adoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
>  
> manager:
> 
> Thomas Skinner (1):
>   fix #4234: add GUI option for openid optional userinfo request
> 
>  www/manager6/dc/AuthEditOpenId.js | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
>  
> perl-rs:
> 
> Thomas Skinner (1):
>   fix #4234: openid: adjust openid verification function for userinfo
>     option
> 
>  pve-rs/src/openid/mod.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
>  
> proxmox-openid:
> 
> Thomas Skinner (1):
>   fix #4234: openid: add library functions for optional userinfo
>     endpoint
> 
>  proxmox-openid/src/lib.rs | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
>  

Tested the series with Authentik. Verified the userinfo endpoint query
with tcpdump.
Rebased proxmox-perl-rs patch on top of master, see modified patch in reply.

Consider this:

Tested-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/perl-rs/proxmox-openid v4] Make OIDC userinfo endpoint optional

[Fabian Grünbichler]

with Mira's follow-up folded in.

Quoting Thomas Skinner (2025-03-24 04:37:32)
> Changes since v3:
> - adjust option to "query userinfo endpoint" with default enabled
> 
> access-control:
> 
> Thomas Skinner (1):
>   fix #4234: add library functions for openid optional userinfo request
> 
>  src/PVE/API2/OpenId.pm | 6 +++++-
>  src/PVE/Auth/OpenId.pm | 7 +++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
>  
> docs:
> 
> Thomas Skinner (1):
>   fix #4234: add docs for openid optional userinfo request
> 
>  pveum.adoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
>  
> manager:
> 
> Thomas Skinner (1):
>   fix #4234: add GUI option for openid optional userinfo request
> 
>  www/manager6/dc/AuthEditOpenId.js | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
>  
> perl-rs:
> 
> Thomas Skinner (1):
>   fix #4234: openid: adjust openid verification function for userinfo
>     option
> 
>  pve-rs/src/openid/mod.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
>  
> proxmox-openid:
> 
> Thomas Skinner (1):
>   fix #4234: openid: add library functions for optional userinfo
>     endpoint
> 
>  proxmox-openid/src/lib.rs | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
>  
> -- 
> 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