all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2] openid: allow to configure scopes, prompt, ACRs and arbitrary username-claim values
Date: Fri,  6 Aug 2021 13:57:38 +0200	[thread overview]
Message-ID: <20210806115744.1959420-3-dietmar@proxmox.com> (raw)
In-Reply-To: <20210806115744.1959420-1-dietmar@proxmox.com>

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

Depend on proxmox-openid 0.7.0.
---

Changes in v2;
- support ACRs
- no update API for im username-claim

 Cargo.toml                       |  2 +-
 pbs-api-types/src/lib.rs         | 48 ++++++++++++++++++++++-
 src/api2/access/openid.rs        | 65 ++++++++++++++++++++++----------
 src/api2/config/access/openid.rs | 29 ++++++++++++++
 src/config/domains.rs            | 41 ++++++++++----------
 5 files changed, 145 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..ee497789 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,52 @@ 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_ACR_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&PROXMOX_SAFE_ID_REGEX);
+
+pub const OPENID_ACR_SCHEMA: Schema = StringSchema::new("OpenID Authentication Context Class Reference.")
+    .format(&OPENID_SCOPE_FORMAT)
+    .schema();
+
+pub const OPENID_ACR_ARRAY_SCHEMA: Schema = ArraySchema::new(
+    "Array of OpenId ACRs.", &OPENID_ACR_SCHEMA).schema();
+
+pub const OPENID_ACR_LIST_FORMAT: ApiStringFormat =
+    ApiStringFormat::PropertyString(&OPENID_ACR_ARRAY_SCHEMA);
+
+pub const OPENID_ACR_LIST_SCHEMA: Schema = StringSchema::new("OpenID ACR List")
+    .format(&OPENID_ACR_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 \
+    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..34dc9789 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,35 @@ use crate::api2::types::*;
 use crate::auth_helpers::*;
 
 fn openid_authenticator(realm_config: &OpenIdRealmConfig, redirect_url: &str) -> Result<OpenIdAuthenticator, Error> {
+
+    let scopes: Vec<String> = realm_config.scopes.as_deref().unwrap_or(OPENID_DEFAILT_SCOPE_LIST)
+        .split(|c: char| c == ',' || c == ';' || char::is_ascii_whitespace(&c))
+        .filter(|s| !s.is_empty())
+        .map(String::from)
+        .collect();
+
+    let mut acr_values = None;
+    if let Some(ref list) = realm_config.acr_values {
+        acr_values = Some(
+            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),
+        acr_values,
     };
     OpenIdAuthenticator::discover(&config, redirect_url)
 }
 
-
 #[api(
     input: {
         properties: {
@@ -93,22 +113,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 +151,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..06079e4e 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 acr_values property
+    acr_values,
 }
 
 #[api(
@@ -179,6 +185,20 @@ pub enum DeletableProperty {
                 type: String,
                 optional: true,
             },
+            "scopes": {
+                schema: OPENID_SCOPE_LIST_SCHEMA,
+                optional: true,
+            },
+            "acr-values": {
+                schema: OPENID_ACR_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>,
+    acr_values: Option<String>,
+    prompt: 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::acr_values => { config.acr_values = 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 acr_values.is_some() { config.acr_values = acr_values; }
 
     domains.set_data(&realm, "openid", &config)?;
 
diff --git a/src/config/domains.rs b/src/config/domains.rs
index 0d695777..f227410f 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,20 @@ pub enum OpenIdUserAttribute {
             type: String,
             optional: true,
         },
+        "scopes": {
+            schema: OPENID_SCOPE_LIST_SCHEMA,
+            optional: true,
+        },
+        "acr-values": {
+            schema: OPENID_ACR_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 +63,7 @@ pub enum OpenIdUserAttribute {
             default: false,
         },
         "username-claim": {
-            type: OpenIdUserAttribute,
+            schema: OPENID_USERNAME_CLAIM_SCHEMA,
             optional: true,
         },
     },
@@ -79,13 +76,19 @@ 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 acr_values: 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





  parent reply	other threads:[~2021-08-06 11:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 11:57 [pbs-devel] [PATCH proxmox-openid-rs v2 1/6] allow to configure used scopes Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH pve-rs] depend on proxmox-openid 0.7.0, bump version to 0.3.0 Dietmar Maurer
2021-08-06 11:57 ` Dietmar Maurer [this message]
2021-08-06 11:57 ` [pbs-devel] [PATCH pve-access-control] openid: support scopes, prompt, ACRs and arbitrary username-claim values Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH proxmox-openid-rs v2 2/6] also return data from UserInfo endpoint Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH proxmox-openid-rs v2 3/6] new helper verify_authorization_code_simple() Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH proxmox-openid-rs v2 4/6] allow to configure prompt behaviour Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH proxmox-openid-rs v2 5/6] allow to configure acr values Dietmar Maurer
2021-08-06 11:57 ` [pbs-devel] [PATCH proxmox-openid-rs v2 6/6] bump version to 0.7.0-1 Dietmar Maurer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210806115744.1959420-3-dietmar@proxmox.com \
    --to=dietmar@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal