From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C3D046C2D2 for ; Fri, 6 Aug 2021 09:17:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 74D43A86C for ; Fri, 6 Aug 2021 09:17:16 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 6BCFFA7CF for ; Fri, 6 Aug 2021 09:17:13 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 436E542DF4; Fri, 6 Aug 2021 09:17:13 +0200 (CEST) From: Dietmar Maurer To: pbs-devel@lists.proxmox.com Date: Fri, 6 Aug 2021 09:17:05 +0200 Message-Id: <20210806071709.1796794-3-dietmar@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210806071709.1796794-1-dietmar@proxmox.com> References: <20210806071709.1796794-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.722 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [openid.rs, domains.rs, lib.rs] Subject: [pbs-devel] [PATCH proxmox-backup] openid: allow to configure scopes, prompt and arbitrary username-claim values X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Aug 2021 07:17:46 -0000 - 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 { + + let ref list = realm_config.scopes.as_deref().unwrap_or(OPENID_DEFAILT_SCOPE_LIST); + let scopes: Vec = 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, client_id: Option, client_key: Option, + scopes: Option, + prompt: Option, + username_claim: Option, autocreate: Option, comment: Option, delete: Option>, @@ -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, + #[serde(skip_serializing_if="Option::is_none")] + pub prompt: Option, + #[serde(skip_serializing_if="Option::is_none")] pub client_key: Option, #[serde(skip_serializing_if="Option::is_none")] pub comment: Option, #[serde(skip_serializing_if="Option::is_none")] pub autocreate: Option, #[serde(skip_serializing_if="Option::is_none")] - pub username_claim: Option, + pub username_claim: Option, } fn init() -> SectionConfig { -- 2.30.2