[PATCH many v5 0/8] fix #5076: add support for open id audiences

[PATCH many v5 0/8] fix #5076: add support for open id audiences

[Shannon Sterz]

this series adapts the original patch series by Alexander Abraham [1]. below is
the text of the original cover letter:

> fix #5076: Added Open ID audiences
>
> This series adds support for handling Open ID audiences as described in bug
> #5076. PVE's API schema was updated to accept an optional field, an array of
> strings and the Rust code was also updated to accordingly handle any incoming
> audiences and compare them to the realm config's audiences. In the realm
> dialogue for adding an Open ID realm, a new field titled "Audiences" was added
> so that users can save any audiences in their realm domains config file.

essentially, some open id providers such as zitadel [2] may provide additional
audiences that their id tokens are valid for instead of just the client id.
these patches allow setting such additional audiences. if an audience that is
not explicitly allowed is encountered, the id token is rejected as before.

Changelog
---------

changes since v4:

* add patches to support audiences in proxmox-backup and
  proxmox-datacenter-manager as well
* instead of the api type and configs taking an array, it will now take a string
  that is a list of audiences. this is more consistent with other parameters 
  here.
* rebased on current master.

changes since v3:

* rebased on current master
* see the list of changes made by Shannon Sterz specified in each commit message

[1]: https://lore.proxmox.com/pve-devel/20250603091256.40923-1-a.abraham@proxmox.com/
[2]: https://zitadel.com/


proxmox:

Shannon Sterz (2):
  fix #5076: openid: add logic to handle OIDC audiences
  fix #5076: pbs-api-types: add audiences to open id realm config

 pbs-api-types/src/openid.rs | 26 ++++++++++++++++++++++++++
 proxmox-openid/src/lib.rs   | 21 +++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)


access-control:

Shannon Sterz (1):
  fix #5076: auth: open id: add an optional "audiences" field

 src/PVE/API2/OpenId.pm | 4 ++++
 src/PVE/Auth/OpenId.pm | 9 +++++++++
 2 files changed, 13 insertions(+)


manager:

Shannon Sterz (1):
  fix #5076: ui: dc: add an optional "audiences" field for open id
    realms

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


yew-comp:

Shannon Sterz (1):
  fix #5076: auth edit openid: add advanced  "audiences" field

 src/auth_edit_openid.rs | 2 ++
 1 file changed, 2 insertions(+)


datacenter-manager:

Shannon Sterz (1):
  fix #5076: api-types/api: support audiences property for open id
    realms

 lib/pdm-api-types/src/openid.rs        | 30 +++++++++++++++++++++++++-
 server/src/api/access/openid.rs        |  8 +++++++
 server/src/api/config/access/openid.rs |  8 +++++++
 3 files changed, 45 insertions(+), 1 deletion(-)


backup:

Shannon Sterz (1):
  fix #5076: api: support audiences property for open id realms

 src/api2/access/openid.rs        | 8 ++++++++
 src/api2/config/access/openid.rs | 8 ++++++++
 2 files changed, 16 insertions(+)


widget-toolkit:

Shannon Sterz (1):
  fix #5076: ui: dc: add an optional "audiences" field for open id
    realms

 src/window/AuthEditOpenId.js | 9 +++++++++
 1 file changed, 9 insertions(+)


Summary over all repositories:
  12 files changed, 139 insertions(+), 3 deletions(-)

-- 
Generated by murpp 0.10.0

[PATCH proxmox v5 1/8] fix #5076: openid: add logic to handle OIDC audiences

[Shannon Sterz]

allows specifying multiple additionally allowed audiences. an open id
provider may include multiple audiences in its ID token. this allows
specifying all valid audiences, rejecting the token only if an invalid
audience is also included.

Originally-by: Alexander Abraham <a.abraham@proxmox.com>
[SS:
 * reformatted the code for clarity and to avoid unnecessary variables
   and parentheses
 * rephrased the commit message for style and clarity
]

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-openid/src/lib.rs | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
index 0388a8fa..a8cfd66f 100644
--- a/proxmox-openid/src/lib.rs
+++ b/proxmox-openid/src/lib.rs
@@ -95,6 +95,8 @@ pub struct OpenIdConfig {
     pub prompt: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub acr_values: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub audiences: Option<Vec<String>>,
 }
 
 pub struct OpenIdAuthenticator {
@@ -258,12 +260,27 @@ impl OpenIdAuthenticator {
             .request(&http_client)
             .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?;
 
-        let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier();
+        let verifier = &self
+            .client
+            .id_token_verifier()
+            .require_audience_match(true)
+            .set_other_audience_verifier_fn(|aud| {
+                if self.config.client_id == **aud {
+                    return true;
+                }
+
+                if let Some(allowed_audiences) = self.config.audiences.as_ref() {
+                    return allowed_audiences.contains(aud);
+                }
+
+                false
+            });
+
         let id_token_claims: &GenericIdTokenClaims = token_response
             .extra_fields()
             .id_token()
             .expect("Server did not return an ID token")
-            .claims(&id_token_verifier, &private_auth_state.nonce)
+            .claims(verifier, &private_auth_state.nonce)
             .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
 
         if !query_userinfo {
-- 
2.47.3

[PATCH proxmox v5 2/8] fix #5076: pbs-api-types: add audiences to open id realm config

[Shannon Sterz]

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 pbs-api-types/src/openid.rs | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/pbs-api-types/src/openid.rs b/pbs-api-types/src/openid.rs
index 0333272b..739f6c89 100644
--- a/pbs-api-types/src/openid.rs
+++ b/pbs-api-types/src/openid.rs
@@ -42,6 +42,26 @@ pub const OPENID_ACR_LIST_SCHEMA: Schema = StringSchema::new("OpenID ACR List")
     .format(&OPENID_ACR_LIST_FORMAT)
     .schema();
 
+pub const OPENID_AUDIENCE_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&GENERIC_URI_REGEX);
+
+pub const OPENID_AUDIENCE_SCHEMA: Schema = StringSchema::new("OpenID audience.")
+    .format(&OPENID_AUDIENCE_FORMAT)
+    .max_length(256)
+    .schema();
+
+pub const OPENID_AUDIENCE_ARRAY_SCHEMA: Schema = ArraySchema::new(
+    "A list of OpenID audiences that is allowed in addition to the 'client-id'.",
+    &OPENID_AUDIENCE_SCHEMA,
+)
+.schema();
+
+pub const OPENID_AUDIENCE_LIST_FORMAT: ApiStringFormat =
+    ApiStringFormat::PropertyString(&OPENID_AUDIENCE_ARRAY_SCHEMA);
+
+pub const OPENID_AUDIENCE_LIST_SCHEMA: Schema = StringSchema::new("OpenID audience list.")
+    .format(&OPENID_AUDIENCE_LIST_FORMAT)
+    .schema();
+
 pub const OPENID_USERNAME_CLAIM_SCHEMA: Schema = StringSchema::new(
     "Use the value of this attribute/claim as unique user name. It \
     is up to the identity provider to guarantee the uniqueness. The \
@@ -70,6 +90,10 @@ pub const OPENID_USERNAME_CLAIM_SCHEMA: Schema = StringSchema::new(
             schema: OPENID_ACR_LIST_SCHEMA,
             optional: true,
         },
+        audiences: {
+            schema: OPENID_AUDIENCE_LIST_SCHEMA,
+            optional: true,
+        },
         prompt: {
             description: "OpenID Prompt",
             type: String,
@@ -109,6 +133,8 @@ pub struct OpenIdRealmConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub acr_values: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub audiences: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub prompt: Option<String>,
     /// OpenID Client Key
     #[serde(skip_serializing_if = "Option::is_none")]
-- 
2.47.3

[PATCH access-control v5 3/8] fix #5076: auth: open id: add an optional "audiences" field

[Shannon Sterz]

adds an optional "audiences" field to the open id configuration
schema. "audiences" is a list of additionally accepted audiences that
an open id provider may include.

Originally-by: Alexander Abraham <a.abraham@proxmox.com>
[SS:
 * rephrased the commit message for style and clarity
 * adjusted the parameter description to be clearer
 * adjusted the schema definition to accept a string instead of an
   array to be more consistent with other parameters here
 * removed changes that only reformatted unrelated code
]

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/PVE/API2/OpenId.pm | 4 ++++
 src/PVE/Auth/OpenId.pm | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index cccd615..429cb3a 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -46,6 +46,10 @@ my $lookup_openid_auth = sub {
         $openid_config->{acr_values} = [PVE::Tools::split_list($acr)];
     }
 
+    if (defined(my $audiences = $config->{'audiences'})) {
+        $openid_config->{audiences} = [PVE::Tools::split_list($audiences)];
+    }
+
     my $openid = PVE::RS::OpenId->discover($openid_config, $redirect_url);
     return ($config, $openid);
 };
diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm
index 2209040..86c91bd 100755
--- a/src/PVE/Auth/OpenId.pm
+++ b/src/PVE/Auth/OpenId.pm
@@ -92,6 +92,14 @@ sub properties {
             default => 1,
             optional => 1,
         },
+        'audiences' => {
+            description =>
+                "A list of audiences that the OpenID Issuer may include that are accepted in "
+                . "addition to 'client-id'.",
+            type => 'string',
+            pattern => '^[^\x00-\x1F\x7F <>#"]*$', # Prohibit characters not allowed in URI RFC 2396.
+            optional => 1,
+        },
     };
 }
 
@@ -108,6 +116,7 @@ sub options {
         prompt => { optional => 1 },
         scopes => { optional => 1 },
         "acr-values" => { optional => 1 },
+        audiences => { optional => 1 },
         default => { optional => 1 },
         comment => { optional => 1 },
         "query-userinfo" => { optional => 1 },
-- 
2.47.3

[PATCH manager v5 4/8] fix #5076: ui: dc: add an optional "audiences" field for open id realms

[Shannon Sterz]

allows users to configure additional open id audiences that may be
provided by the open id realm.

Originally-by: Alexander Abraham <a.abraham@proxmox.com>
[SS:
 * removed unnecessary helper functions
 * send the 'audiences' property as a string to the backend instead of
   an array to be consistent with other options
 * rephrased commit message for clarity
]

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 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 dc6c22e61..088eb8191 100644
--- a/www/manager6/dc/AuthEditOpenId.js
+++ b/www/manager6/dc/AuthEditOpenId.js
@@ -149,6 +149,15 @@ Ext.define('PVE.panel.OpenIDInputPanel', {
                 deleteEmpty: '{!isCreate}',
             },
         },
+        {
+            xtype: 'proxmoxtextfield',
+            name: 'audiences',
+            fieldLabel: gettext('Audiences'),
+            submitEmpty: false,
+            cbind: {
+                deleteEmpty: '{!isCreate}',
+            },
+        },
     ],
 
     initComponent: function () {
-- 
2.47.3

[PATCH yew-comp v5 5/8] fix #5076: auth edit openid: add advanced "audiences" field

[Shannon Sterz]

to allow configuring open id audiences.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/auth_edit_openid.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/auth_edit_openid.rs b/src/auth_edit_openid.rs
index 01f0552..bcb35df 100644
--- a/src/auth_edit_openid.rs
+++ b/src/auth_edit_openid.rs
@@ -60,6 +60,7 @@ async fn update_item(form_ctx: FormContext, base_url: String) -> Result<(), Erro
         &data,
         &[
             "acr-values",
+            "audiences",
             "autocreate",
             "comment",
             "client-key",
@@ -145,6 +146,7 @@ fn render_input_form(form_ctx: FormContext, props: AuthEditOpenID) -> Html {
         .with_large_field(tr!("Comment"), Field::new().name("comment"))
         .with_advanced_spacer()
         .with_large_advanced_field(tr!("ACR Values"), Field::new().name("acr-values"))
+        .with_large_advanced_field(tr!("Audiences"), Field::new().name("audiences"))
         .into()
 }
 
-- 
2.47.3

[PATCH datacenter-manager v5 6/8] fix #5076: api-types/api: support audiences property for open id realms

[Shannon Sterz]

to allow for better compatability with open id providers, support the
audiences field for open id realms. users can specify which audiences
are trusted in addition to the client-id.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 lib/pdm-api-types/src/openid.rs        | 30 +++++++++++++++++++++++++-
 server/src/api/access/openid.rs        |  8 +++++++
 server/src/api/config/access/openid.rs |  8 +++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/lib/pdm-api-types/src/openid.rs b/lib/pdm-api-types/src/openid.rs
index c129d35..a241c54 100644
--- a/lib/pdm-api-types/src/openid.rs
+++ b/lib/pdm-api-types/src/openid.rs
@@ -3,7 +3,9 @@ use serde::{Deserialize, Serialize};
 use proxmox_schema::api_types::SAFE_ID_REGEX;
 use proxmox_schema::{api, ApiStringFormat, ArraySchema, Schema, StringSchema, Updater};
 
-use super::{PROXMOX_SAFE_ID_FORMAT, REALM_ID_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA};
+use super::{
+    GENERIC_URI_REGEX, PROXMOX_SAFE_ID_FORMAT, REALM_ID_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA,
+};
 
 pub const OPENID_SCOPE_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&SAFE_ID_REGEX);
 
@@ -40,6 +42,26 @@ pub const OPENID_ACR_LIST_SCHEMA: Schema = StringSchema::new("OpenID ACR List")
     .format(&OPENID_ACR_LIST_FORMAT)
     .schema();
 
+pub const OPENID_AUDIENCE_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&GENERIC_URI_REGEX);
+
+pub const OPENID_AUDIENCE_SCHEMA: Schema = StringSchema::new("OpenID audience.")
+    .format(&OPENID_AUDIENCE_FORMAT)
+    .max_length(256)
+    .schema();
+
+pub const OPENID_AUDIENCE_ARRAY_SCHEMA: Schema = ArraySchema::new(
+    "A list of OpenID audiences that is allowed in addition to the 'client-id'.",
+    &OPENID_AUDIENCE_SCHEMA,
+)
+.schema();
+
+pub const OPENID_AUDIENCE_LIST_FORMAT: ApiStringFormat =
+    ApiStringFormat::PropertyString(&OPENID_AUDIENCE_ARRAY_SCHEMA);
+
+pub const OPENID_AUDIENCE_LIST_SCHEMA: Schema = StringSchema::new("OpenID audience list.")
+    .format(&OPENID_AUDIENCE_LIST_FORMAT)
+    .schema();
+
 pub const OPENID_USERNAME_CLAIM_SCHEMA: Schema = StringSchema::new(
     "Use the value of this attribute/claim as unique user name. It \
     is up to the identity provider to guarantee the uniqueness. The \
@@ -68,6 +90,10 @@ pub const OPENID_USERNAME_CLAIM_SCHEMA: Schema = StringSchema::new(
             schema: OPENID_ACR_LIST_SCHEMA,
             optional: true,
         },
+        audiences: {
+            schema: OPENID_AUDIENCE_LIST_SCHEMA,
+            optional: true,
+        },
         prompt: {
             description: "OpenID Prompt",
             type: String,
@@ -103,6 +129,8 @@ pub struct OpenIdRealmConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub acr_values: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub audiences: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub prompt: Option<String>,
     /// OpenID Client Key
     #[serde(skip_serializing_if = "Option::is_none")]
diff --git a/server/src/api/access/openid.rs b/server/src/api/access/openid.rs
index 1dccb29..dd03a7e 100644
--- a/server/src/api/access/openid.rs
+++ b/server/src/api/access/openid.rs
@@ -48,6 +48,13 @@ fn openid_authenticator(
         );
     }
 
+    let audiences = realm_config.audiences.as_ref().map(|list| {
+        list.split(|c: char| c == ',' || c == ';' || char::is_ascii_whitespace(&c))
+            .filter(|s| !s.is_empty())
+            .map(String::from)
+            .collect()
+    });
+
     let config = OpenIdConfig {
         issuer_url: realm_config.issuer_url.clone(),
         client_id: realm_config.client_id.clone(),
@@ -55,6 +62,7 @@ fn openid_authenticator(
         prompt: realm_config.prompt.clone(),
         scopes: Some(scopes),
         acr_values,
+        audiences,
     };
     OpenIdAuthenticator::discover(&config, redirect_url)
 }
diff --git a/server/src/api/config/access/openid.rs b/server/src/api/config/access/openid.rs
index 5e1764a..e6901fd 100644
--- a/server/src/api/config/access/openid.rs
+++ b/server/src/api/config/access/openid.rs
@@ -157,6 +157,8 @@ pub enum DeletableProperty {
     Prompt,
     /// Delete the acr_values property
     AcrValues,
+    /// Delete the audiences property
+    Audiences,
 }
 
 #[api(
@@ -227,6 +229,9 @@ pub fn update_openid_realm(
                 DeletableProperty::AcrValues => {
                     config.acr_values = None;
                 }
+                DeletableProperty::Audiences => {
+                    config.audiences = None;
+                }
             }
         }
     }
@@ -269,6 +274,9 @@ pub fn update_openid_realm(
     if update.acr_values.is_some() {
         config.acr_values = update.acr_values;
     }
+    if update.audiences.is_some() {
+        config.audiences = update.audiences;
+    }
 
     domains.set_data(&realm, "openid", &config)?;
 
-- 
2.47.3

[PATCH proxmox-backup v5 7/8] fix #5076: api: support audiences property for open id realms

[Shannon Sterz]

to allow for better compatability with open id providers, support the
audiences field for open id realms. users can specify which audiences
are trusted in addition to the client-id.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/api2/access/openid.rs        | 8 ++++++++
 src/api2/config/access/openid.rs | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
index cb94b0e18..818bcb555 100644
--- a/src/api2/access/openid.rs
+++ b/src/api2/access/openid.rs
@@ -50,6 +50,13 @@ fn openid_authenticator(
         );
     }
 
+    let audiences = realm_config.audiences.as_ref().map(|list| {
+        list.split(|c: char| c == ',' || c == ';' || char::is_ascii_whitespace(&c))
+            .filter(|s| !s.is_empty())
+            .map(String::from)
+            .collect()
+    });
+
     let config = OpenIdConfig {
         issuer_url: realm_config.issuer_url.clone(),
         client_id: realm_config.client_id.clone(),
@@ -57,6 +64,7 @@ fn openid_authenticator(
         prompt: realm_config.prompt.clone(),
         scopes: Some(scopes),
         acr_values,
+        audiences,
     };
     OpenIdAuthenticator::discover(&config, redirect_url)
 }
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index ab05bfb68..b71e4dae6 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -159,6 +159,8 @@ pub enum DeletableProperty {
     Prompt,
     /// Delete the acr_values property
     AcrValues,
+    /// Delete the audiences property
+    Audiences,
 }
 
 #[api(
@@ -231,6 +233,9 @@ pub fn update_openid_realm(
                 DeletableProperty::AcrValues => {
                     config.acr_values = None;
                 }
+                DeletableProperty::Audiences => {
+                    config.audiences = None;
+                }
             }
         }
     }
@@ -273,6 +278,9 @@ pub fn update_openid_realm(
     if update.acr_values.is_some() {
         config.acr_values = update.acr_values;
     }
+    if update.audiences.is_some() {
+        config.audiences = update.audiences;
+    }
 
     domains.set_data(&realm, "openid", &config)?;
 
-- 
2.47.3

[PATCH widget-toolkit v5 8/8] fix #5076: ui: dc: add an optional "audiences" field for open id realms

[Shannon Sterz]

allows users to configure additional open id audiences that may be
provided by the open id realm.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/window/AuthEditOpenId.js | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/window/AuthEditOpenId.js b/src/window/AuthEditOpenId.js
index c850019..d70b77b 100644
--- a/src/window/AuthEditOpenId.js
+++ b/src/window/AuthEditOpenId.js
@@ -148,5 +148,14 @@ Ext.define('Proxmox.panel.OpenIDInputPanel', {
                 deleteEmpty: '{!isCreate}',
             },
         },
+        {
+            xtype: 'proxmoxtextfield',
+            name: 'audiences',
+            fieldLabel: gettext('Audiences'),
+            submitEmpty: false,
+            cbind: {
+                deleteEmpty: '{!isCreate}',
+            },
+        },
     ],
 });
-- 
2.47.3