* [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional
@ 2025-02-08 5:42 Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH docs v3 1/1] fix #4234: add docs for openid optional userinfo request Thomas Skinner
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: Thomas Skinner
Continues work on adding an option to disable querying the userinfo endpoint for an
OIDC provider.
Changes since v2:
- Adjust verify_authorization_code in pve-rs to be backwards compatible
- Fix defaults in wrapper functions
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 | 9 +++++++++
1 file changed, 9 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH docs v3 1/1] fix #4234: add docs for openid optional userinfo request
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
@ 2025-02-08 5:42 ` Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH manager v3 1/1] fix #4234: add GUI option " Thomas Skinner
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: 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..1d18d38 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
+^^^^^^^^^^^^^^^^^
+
+* `Disable userinfo request` (`disable-userinfo`): Enabling this option prevents
+the OpenID Connect authenticator from querying the "userinfo" endpoint for claim
+values. This 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager v3 1/1] fix #4234: add GUI option for openid optional userinfo request
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH docs v3 1/1] fix #4234: add docs for openid optional userinfo request Thomas Skinner
@ 2025-02-08 5:42 ` Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions " Thomas Skinner
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: Thomas Skinner
Signed-off-by: Thomas Skinner <thomas@atskinner.net>
---
www/manager6/dc/AuthEditOpenId.js | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/www/manager6/dc/AuthEditOpenId.js b/www/manager6/dc/AuthEditOpenId.js
index 544c0de5..904e508c 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -111,6 +111,15 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
deleteEmpty: '{!isCreate}',
},
},
+ {
+ xtype: 'proxmoxcheckbox',
+ fieldLabel: gettext('Disable userinfo request'),
+ name: 'disable-userinfo',
+ value: 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions for openid optional userinfo request
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH docs v3 1/1] fix #4234: add docs for openid optional userinfo request Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH manager v3 1/1] fix #4234: add GUI option " Thomas Skinner
@ 2025-02-08 5:42 ` Thomas Skinner
2025-03-06 15:20 ` Mira Limbeck
2025-02-08 5:42 ` [pve-devel] [PATCH proxmox-openid v3 1/1] fix #4234: openid: add library functions for optional userinfo endpoint Thomas Skinner
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: 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..456e96a 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->{'disable-userinfo'} // 0,
+ );
my $subject = $info->{'sub'};
my $unique_name;
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index c8e4db9..6efa696 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,
},
+ "disable-userinfo" => {
+ description => "Prevents querying the userinfo endpoint for claims values.",
+ type => 'boolean',
+ default => 0,
+ optional => 1,
+ },
};
}
@@ -78,6 +84,7 @@ sub options {
"acr-values" => { optional => 1 },
default => { optional => 1 },
comment => { optional => 1 },
+ "disable-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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH proxmox-openid v3 1/1] fix #4234: openid: add library functions for optional userinfo endpoint
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
` (2 preceding siblings ...)
2025-02-08 5:42 ` [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions " Thomas Skinner
@ 2025-02-08 5:42 ` Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option Thomas Skinner
2025-03-06 15:15 ` [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Mira Limbeck
5 siblings, 0 replies; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: 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..d2a53d45 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, false)
+ }
+
+ pub fn verify_authorization_code_userinfo(
+ &self,
+ code: &str,
+ private_auth_state: &PrivateAuthState,
+ disable_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 disable_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, false)
+ }
+
+ /// 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,
+ disable_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, disable_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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
` (3 preceding siblings ...)
2025-02-08 5:42 ` [pve-devel] [PATCH proxmox-openid v3 1/1] fix #4234: openid: add library functions for optional userinfo endpoint Thomas Skinner
@ 2025-02-08 5:42 ` Thomas Skinner
2025-03-06 15:23 ` Mira Limbeck
2025-03-06 15:15 ` [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Mira Limbeck
5 siblings, 1 reply; 9+ messages in thread
From: Thomas Skinner @ 2025-02-08 5:42 UTC (permalink / raw)
To: pve-devel; +Cc: 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..8f914ad 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,
+ disable_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,
+ disable_userinfo.unwrap_or(false),
+ )?;
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
` (4 preceding siblings ...)
2025-02-08 5:42 ` [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option Thomas Skinner
@ 2025-03-06 15:15 ` Mira Limbeck
5 siblings, 0 replies; 9+ messages in thread
From: Mira Limbeck @ 2025-03-06 15:15 UTC (permalink / raw)
To: pve-devel
On 2/8/25 06:42, Thomas Skinner wrote:
> Continues work on adding an option to disable querying the userinfo endpoint for an
> OIDC provider.
>
> Changes since v2:
> - Adjust verify_authorization_code in pve-rs to be backwards compatible
> - Fix defaults in wrapper functions
>
> 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 | 9 +++++++++
> 1 file changed, 9 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(-)
>
>
Thank you for the patch series!
I've tested it with Authentik, and checked with tcpdump to see if the
userinfo endpoint was queried. Works as I would expect.
I had to manually apply the pve-rs patch since code was moved around
since then. More information as reply to the patch itself.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions for openid optional userinfo request
2025-02-08 5:42 ` [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions " Thomas Skinner
@ 2025-03-06 15:20 ` Mira Limbeck
0 siblings, 0 replies; 9+ messages in thread
From: Mira Limbeck @ 2025-03-06 15:20 UTC (permalink / raw)
To: pve-devel
On 2/8/25 06:42, Thomas Skinner wrote:
> 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..456e96a 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->{'disable-userinfo'} // 0,
> + );
> my $subject = $info->{'sub'};
>
> my $unique_name;
> diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
> index c8e4db9..6efa696 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,
> },
> + "disable-userinfo" => {
> + description => "Prevents querying the userinfo endpoint for claims values.",
> + type => 'boolean',
> + default => 0,
> + optional => 1,
> + },
I would prefer `query-userinfo` (or similar) and a default of `1` over
`disable-userinfo` and a default of `0`.
> };
> }
>
> @@ -78,6 +84,7 @@ sub options {
> "acr-values" => { optional => 1 },
> default => { optional => 1 },
> comment => { optional => 1 },
> + "disable-userinfo" => { optional => 1 },
> };
> }
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option
2025-02-08 5:42 ` [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option Thomas Skinner
@ 2025-03-06 15:23 ` Mira Limbeck
0 siblings, 0 replies; 9+ messages in thread
From: Mira Limbeck @ 2025-03-06 15:23 UTC (permalink / raw)
To: pve-devel
On 2/8/25 06:42, 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..8f914ad 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,
> + disable_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,
> + disable_userinfo.unwrap_or(false),
> + )?;
>
> Ok(to_value(&claims)?)
> }
Commit 9ee9ad4 moved the code to common/src/oidc. I had to apply those
manually at the new location, and update the forwarding call in
pve-rs/src/openid/mod.rs.
Other than that, worked as espected.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-03-06 15:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-08 5:42 [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH docs v3 1/1] fix #4234: add docs for openid optional userinfo request Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH manager v3 1/1] fix #4234: add GUI option " Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH access-control v3 1/1] fix #4234: add library functions " Thomas Skinner
2025-03-06 15:20 ` Mira Limbeck
2025-02-08 5:42 ` [pve-devel] [PATCH proxmox-openid v3 1/1] fix #4234: openid: add library functions for optional userinfo endpoint Thomas Skinner
2025-02-08 5:42 ` [pve-devel] [PATCH perl-rs v3 1/1] fix #4234: openid: adjust openid verification function for userinfo option Thomas Skinner
2025-03-06 15:23 ` Mira Limbeck
2025-03-06 15:15 ` [pve-devel] [PATCH SERIES access-control/docs/manager/perl-rs/proxmox-openid v3] Make OIDC userinfo endpoint optional Mira Limbeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal