[pbs-devel] [PATCH proxmox-backup] openid: allow to configure scopes, prompt and arbitrary username-claim values

[Dietmar Maurer <dietmar@proxmox.com>]

- no longer set prompt to 'login' (makes auto-login possible)
- new prompt configuration
- allow arbitrary username-claim values

We now allow to change the username-claim in the update API.

Depend on proxmox-openid 0.7.0.
---
 Cargo.toml                       |  2 +-
 pbs-api-types/src/lib.rs         | 31 +++++++++++++++++-
 src/api2/access/openid.rs        | 54 +++++++++++++++++++++-----------
 src/api2/config/access/openid.rs | 29 +++++++++++++++++
 src/config/domains.rs            | 35 ++++++++++-----------
 5 files changed, 111 insertions(+), 40 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index c6141842..e71b87ba 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -92,7 +92,7 @@ proxmox = { version = "0.12.1", features = [ "sortable-macro", "api-macro", "cli
 proxmox-acme-rs = "0.2.1"
 proxmox-apt = "0.6.0"
 proxmox-http = { version = "0.3.0", features = [ "client", "http-helpers", "websocket" ] }
-proxmox-openid = "0.6.1"
+proxmox-openid = "0.7.0"
 
 pbs-api-types = { path = "pbs-api-types" }
 pbs-buildcfg = { path = "pbs-buildcfg" }
diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 576099eb..1cba21f4 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -3,7 +3,7 @@
 use serde::{Deserialize, Serialize};
 
 use proxmox::api::api;
-use proxmox::api::schema::{ApiStringFormat, EnumEntry, IntegerSchema, Schema, StringSchema};
+use proxmox::api::schema::{ApiStringFormat, EnumEntry, IntegerSchema, Schema, StringSchema, ArraySchema};
 use proxmox::const_regex;
 use proxmox::{IPRE, IPRE_BRACKET, IPV4OCTET, IPV4RE, IPV6H16, IPV6LS32, IPV6RE};
 
@@ -184,6 +184,35 @@ pub const PRUNE_SCHEMA_KEEP_YEARLY: Schema =
 pub const PROXMOX_SAFE_ID_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&PROXMOX_SAFE_ID_REGEX);
 
+pub const OPENID_SCOPE_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&PROXMOX_SAFE_ID_REGEX);
+
+pub const OPENID_SCOPE_SCHEMA: Schema = StringSchema::new("OpenID Scope Name.")
+    .format(&OPENID_SCOPE_FORMAT)
+    .schema();
+
+pub const OPENID_SCOPE_ARRAY_SCHEMA: Schema = ArraySchema::new(
+    "Array of OpenId Scopes.", &OPENID_SCOPE_SCHEMA).schema();
+
+pub const OPENID_SCOPE_LIST_FORMAT: ApiStringFormat =
+    ApiStringFormat::PropertyString(&OPENID_SCOPE_ARRAY_SCHEMA);
+
+pub const OPENID_DEFAILT_SCOPE_LIST: &'static str = "email profile";
+pub const OPENID_SCOPE_LIST_SCHEMA: Schema = StringSchema::new("OpenID Scope List")
+    .format(&OPENID_SCOPE_LIST_FORMAT)
+    .default(OPENID_DEFAILT_SCOPE_LIST)
+    .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 \
+    OpenID specification only guarantees that Subject ('sub') is \
+    unique. Also make sure that the user is not allowed to change that \
+    attribute by himself!")
+    .max_length(64)
+    .min_length(1)
+    .format(&PROXMOX_SAFE_ID_FORMAT) .schema();
+
 pub const SINGLE_LINE_COMMENT_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&SINGLE_LINE_COMMENT_REGEX);
 
diff --git a/src/api2/access/openid.rs b/src/api2/access/openid.rs
index f78e4674..43dc77e4 100644
--- a/src/api2/access/openid.rs
+++ b/src/api2/access/openid.rs
@@ -18,7 +18,7 @@ use pbs_tools::ticket::Ticket;
 
 use crate::server::ticket::ApiTicket;
 
-use crate::config::domains::{OpenIdUserAttribute, OpenIdRealmConfig};
+use crate::config::domains::OpenIdRealmConfig;
 use crate::config::cached_user_info::CachedUserInfo;
 
 use crate::backup::open_backup_lockfile;
@@ -27,15 +27,24 @@ use crate::api2::types::*;
 use crate::auth_helpers::*;
 
 fn openid_authenticator(realm_config: &OpenIdRealmConfig, redirect_url: &str) -> Result<OpenIdAuthenticator, Error> {
+
+    let ref list = realm_config.scopes.as_deref().unwrap_or(OPENID_DEFAILT_SCOPE_LIST);
+    let scopes: Vec<String> = 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(),
         client_key: realm_config.client_key.clone(),
+        prompt: realm_config.prompt.clone(),
+        scopes: Some(scopes),
     };
     OpenIdAuthenticator::discover(&config, redirect_url)
 }
 
-
 #[api(
     input: {
         properties: {
@@ -93,22 +102,29 @@ pub fn openid_login(
 
     let open_id = openid_authenticator(&config, &redirect_url)?;
 
-    let info = open_id.verify_authorization_code(&code, &private_auth_state)?;
+    let info = open_id.verify_authorization_code_simple(&code, &private_auth_state)?;
 
-    // eprintln!("VERIFIED {} {:?} {:?}", info.subject().as_str(), info.name(), info.email());
+    // eprintln!("VERIFIED {:?}", info);
 
-    let unique_name = match config.username_claim {
-        None | Some(OpenIdUserAttribute::Subject) => info.subject().as_str(),
-        Some(OpenIdUserAttribute::Username) => {
-            match info.preferred_username() {
-                Some(name) => name.as_str(),
-                None => bail!("missing claim 'preferred_name'"),
-            }
-        }
-        Some(OpenIdUserAttribute::Email) => {
-            match info.email() {
-                Some(name) => name.as_str(),
-                None => bail!("missing claim 'email'"),
+    let name_attr = config.username_claim.as_deref().unwrap_or("sub");
+
+    // Try to be compatible with previous versions
+    let try_attr = match name_attr {
+        "subject" => Some("sub"),
+        "username" => Some("preferred_username"),
+        _ => None,
+    };
+
+    let unique_name = match info[name_attr].as_str() {
+        Some(name) => name.to_owned(),
+        None => {
+            if let Some(try_attr) = try_attr {
+                match info[try_attr].as_str() {
+                    Some(name) => name.to_owned(),
+                    None => bail!("missing claim '{}'", name_attr),
+                }
+            } else {
+                bail!("missing claim '{}'", name_attr);
             }
         }
     };
@@ -124,9 +140,9 @@ pub fn openid_login(
                 comment: None,
                 enable: None,
                 expire: None,
-                firstname: info.given_name().and_then(|n| n.get(None)).map(|n| n.to_string()),
-                lastname: info.family_name().and_then(|n| n.get(None)).map(|n| n.to_string()),
-                email: info.email().map(|e| e.to_string()),
+                firstname: info["given_name"].as_str().map(|n| n.to_string()),
+                lastname: info["family_name"].as_str().map(|n| n.to_string()),
+                email: info["email"].as_str().map(|e| e.to_string()),
             };
             let (mut config, _digest) = user::config()?;
             if config.sections.get(user.userid.as_str()).is_some() {
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index b8b07306..9c7ff101 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -155,6 +155,12 @@ pub enum DeletableProperty {
     comment,
     /// Delete the autocreate property
     autocreate,
+    /// Delete the scopes property
+    scopes,
+    /// Delete the prompt property
+    prompt,
+    /// Delete the username-claim property
+    username_claim,
 }
 
 #[api(
@@ -179,6 +185,20 @@ pub enum DeletableProperty {
                 type: String,
                 optional: true,
             },
+            "username-claim": {
+                schema: OPENID_USERNAME_CLAIM_SCHEMA,
+                optional: true,
+            },
+            "scopes": {
+                schema: OPENID_SCOPE_LIST_SCHEMA,
+                optional: true,
+            },
+            prompt: {
+                description: "OpenID Prompt",
+                type: String,
+                format: &PROXMOX_SAFE_ID_FORMAT,
+                optional: true,
+            },
             autocreate: {
                 description: "Automatically create users if they do not exist.",
                 optional: true,
@@ -213,6 +233,9 @@ pub fn update_openid_realm(
     issuer_url: Option<String>,
     client_id: Option<String>,
     client_key: Option<String>,
+    scopes: Option<String>,
+    prompt: Option<String>,
+    username_claim: Option<String>,
     autocreate: Option<bool>,
     comment: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
@@ -237,6 +260,9 @@ pub fn update_openid_realm(
                 DeletableProperty::client_key => { config.client_key = None; },
                 DeletableProperty::comment => { config.comment = None; },
                 DeletableProperty::autocreate => { config.autocreate = None; },
+                DeletableProperty::scopes => { config.scopes = None; },
+                DeletableProperty::prompt => { config.prompt = None; },
+                DeletableProperty::username_claim => { config.username_claim = None; },
             }
         }
     }
@@ -255,6 +281,9 @@ pub fn update_openid_realm(
 
     if client_key.is_some() { config.client_key = client_key; }
     if autocreate.is_some() { config.autocreate = autocreate; }
+    if scopes.is_some() { config.scopes = scopes; }
+    if prompt.is_some() { config.prompt = prompt; }
+    if username_claim.is_some() { config.username_claim = username_claim; }
 
     domains.set_data(&realm, "openid", &config)?;
 
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 0d695777..5752e8ad 100644
--- a/src/config/domains.rs
+++ b/src/config/domains.rs
@@ -20,23 +20,6 @@ lazy_static! {
     pub static ref CONFIG: SectionConfig = init();
 }
 
-#[api()]
-#[derive(Eq, PartialEq, Debug, Serialize, Deserialize)]
-#[serde(rename_all = "lowercase")]
-/// Use the value of this attribute/claim as unique user name. It is
-/// up to the identity provider to guarantee the uniqueness. The
-/// OpenID specification only guarantees that Subject ('sub') is unique. Also
-/// make sure that the user is not allowed to change that attribute by
-/// himself!
-pub enum OpenIdUserAttribute {
-    /// Subject (OpenId 'sub' claim)
-    Subject,
-    /// Username (OpenId 'preferred_username' claim)
-    Username,
-    /// Email (OpenId 'email' claim)
-    Email,
-}
-
 #[api(
     properties: {
         realm: {
@@ -55,6 +38,16 @@ pub enum OpenIdUserAttribute {
             type: String,
             optional: true,
         },
+        "scopes": {
+            schema: OPENID_SCOPE_LIST_SCHEMA,
+            optional: true,
+        },
+        prompt: {
+            description: "OpenID Prompt",
+            type: String,
+            format: &PROXMOX_SAFE_ID_FORMAT,
+            optional: true,
+        },
         comment: {
             optional: true,
             schema: SINGLE_LINE_COMMENT_SCHEMA,
@@ -66,7 +59,7 @@ pub enum OpenIdUserAttribute {
             default: false,
         },
         "username-claim": {
-            type: OpenIdUserAttribute,
+            schema: OPENID_USERNAME_CLAIM_SCHEMA,
             optional: true,
         },
     },
@@ -79,13 +72,17 @@ pub struct OpenIdRealmConfig {
     pub issuer_url: String,
     pub client_id: String,
     #[serde(skip_serializing_if="Option::is_none")]
+    pub scopes: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub prompt: Option<String>,
+    #[serde(skip_serializing_if="Option::is_none")]
     pub client_key: Option<String>,
     #[serde(skip_serializing_if="Option::is_none")]
     pub comment: Option<String>,
     #[serde(skip_serializing_if="Option::is_none")]
     pub autocreate: Option<bool>,
     #[serde(skip_serializing_if="Option::is_none")]
-    pub username_claim: Option<OpenIdUserAttribute>,
+    pub username_claim: Option<String>,
 }
 
 fn init() -> SectionConfig {
-- 
2.30.2