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 C8D9DC641 for ; Mon, 14 Aug 2023 11:40:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AA3D2C66 for ; Mon, 14 Aug 2023 11:40:12 +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 for ; Mon, 14 Aug 2023 11:40:12 +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 DBC474744E for ; Mon, 14 Aug 2023 11:40:11 +0200 (CEST) Date: Mon, 14 Aug 2023 11:40:10 +0200 From: Christoph Heiss To: Wolfgang Bumiller Cc: pbs-devel@lists.proxmox.com Message-ID: References: <20230808122239.1025524-1-c.heiss@proxmox.com> <20230808122239.1025524-4-c.heiss@proxmox.com> <25eohhxnkmqyeagjnzqa7is7cuziud7sjk4at6oah2mokpt66x@4vznehpsv6nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <25eohhxnkmqyeagjnzqa7is7cuziud7sjk4at6oah2mokpt66x@4vznehpsv6nl> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.042 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy 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. [auth.rs, ldap.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup 03/12] api-types: implement `LdapMode` -> `ConnectionMode` conversion 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: Mon, 14 Aug 2023 09:40:42 -0000 Thanks for the review! On Fri, Aug 11, 2023 at 12:36:41PM +0200, Wolfgang Bumiller wrote: > > On Tue, Aug 08, 2023 at 02:22:05PM +0200, Christoph Heiss wrote: > > No functional changes. > > > > Signed-off-by: Christoph Heiss > > --- > > pbs-api-types/Cargo.toml | 1 + > > pbs-api-types/src/ldap.rs | 11 +++++++++++ > > src/auth.rs | 12 +++--------- > > 3 files changed, 15 insertions(+), 9 deletions(-) > > > > diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml > > index 31b69f62..cb584cb5 100644 > > --- a/pbs-api-types/Cargo.toml > > +++ b/pbs-api-types/Cargo.toml > > @@ -17,6 +17,7 @@ serde_plain.workspace = true > > proxmox-auth-api = { workspace = true, features = [ "api-types" ] } > > proxmox-human-byte.workspace = true > > proxmox-lang.workspace=true > > +proxmox-ldap.workspace = true > > The api type crate should strive to be somewhat lightweight, as it will > also end up being used in with wasm at some point where we definitely > can't pull this in. > > If it really makes sense to have this locally, it should be > feature-guarded. Ack, I'll drop this then and go with a simple, local function in src/auth.rs instead. It is only needed in two places there anyway, and the feature-gating isn't worth it just to be able to use `.into()` in two places IMO .. > > > proxmox-schema = { workspace = true, features = [ "api-macro" ] } > > proxmox-serde.workspace = true > > proxmox-time.workspace = true > > diff --git a/pbs-api-types/src/ldap.rs b/pbs-api-types/src/ldap.rs > > index f3df90a0..e1f7c452 100644 > > --- a/pbs-api-types/src/ldap.rs > > +++ b/pbs-api-types/src/ldap.rs > > @@ -1,5 +1,6 @@ > > use serde::{Deserialize, Serialize}; > > > > +use proxmox_ldap::ConnectionMode; > > use proxmox_schema::{api, ApiStringFormat, ApiType, ArraySchema, Schema, StringSchema, Updater}; > > > > use super::{REALM_ID_SCHEMA, SINGLE_LINE_COMMENT_SCHEMA}; > > @@ -20,6 +21,16 @@ pub enum LdapMode { > > Ldaps, > > } > > > > +impl From for ConnectionMode { > > + fn from(value: LdapMode) -> ConnectionMode { > > + match value { > > + LdapMode::Ldap => ConnectionMode::Ldap, > > + LdapMode::StartTls => ConnectionMode::StartTls, > > + LdapMode::Ldaps => ConnectionMode::Ldaps, > > + } > > + } > > +} > > + > > #[api( > > properties: { > > "realm": { > > diff --git a/src/auth.rs b/src/auth.rs > > index 318d1ff2..e375ebc4 100644 > > --- a/src/auth.rs > > +++ b/src/auth.rs > > @@ -16,10 +16,10 @@ use proxmox_auth_api::api::{Authenticator, LockedTfaConfig}; > > use proxmox_auth_api::ticket::{Empty, Ticket}; > > use proxmox_auth_api::types::Authid; > > use proxmox_auth_api::Keyring; > > -use proxmox_ldap::{Config, Connection, ConnectionMode}; > > +use proxmox_ldap::{Config, Connection}; > > use proxmox_tfa::api::{OpenUserChallengeData, TfaConfig}; > > > > -use pbs_api_types::{LdapMode, LdapRealmConfig, OpenIdRealmConfig, RealmRef, Userid, UsernameRef}; > > +use pbs_api_types::{LdapRealmConfig, OpenIdRealmConfig, RealmRef, Userid, UsernameRef}; > > use pbs_buildcfg::configdir; > > > > use crate::auth_helpers; > > @@ -185,12 +185,6 @@ impl LdapAuthenticator { > > servers.push(server.clone()); > > } > > > > - let tls_mode = match config.mode.unwrap_or_default() { > > - LdapMode::Ldap => ConnectionMode::Ldap, > > - LdapMode::StartTls => ConnectionMode::StartTls, > > - LdapMode::Ldaps => ConnectionMode::Ldaps, > > - }; > > - > > let (ca_store, trusted_cert) = if let Some(capath) = config.capath.as_deref() { > > let path = PathBuf::from(capath); > > if path.is_dir() { > > @@ -209,7 +203,7 @@ impl LdapAuthenticator { > > base_dn: config.base_dn.clone(), > > bind_dn: config.bind_dn.clone(), > > bind_password: password, > > - tls_mode, > > + tls_mode: config.mode.unwrap_or_default().into(), > > verify_certificate: config.verify.unwrap_or_default(), > > additional_trusted_certificates: trusted_cert, > > certificate_store_path: ca_store, > > -- > > 2.41.0