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

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

[Thomas Skinner]

Continues work on adding an option to disable querying the userinfo endpoint for an
OIDC provider.

Changes since v1:
- Adjust to add option in the UI to enable the functionality
- Add documentation for the option
- Adjust API back to previous behavior


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 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)


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 access-control v2 1/5] 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..ea1de16 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_userinfo(
+		$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

[pve-devel] [PATCH docs v2 2/5] 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..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

[pve-devel] [PATCH manager v2 3/5] 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 | 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

[pve-devel] [PATCH perl-rs v2 4/5] 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 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/pve-rs/src/openid/mod.rs b/pve-rs/src/openid/mod.rs
index 1fa7572..cd573ee 100644
--- a/pve-rs/src/openid/mod.rs
+++ b/pve-rs/src/openid/mod.rs
@@ -50,13 +50,18 @@ mod export {
     }
 
     #[export(raw_return)]
-    pub fn verify_authorization_code(
+    pub fn verify_authorization_code_userinfo(
         #[try_from_ref] this: &OpenId,
         code: &str,
         private_auth_state: PrivateAuthState,
+        disable_userinfo: 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,
+        )?;
 
         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

[pve-devel] [PATCH proxmox v2 5/5] 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..87be1c8a 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,
+        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, 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,
+        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