Re: [pbs-devel] [PATCH proxmox-backup 03/12] api-types: implement `LdapMode` -> `ConnectionMode` conversion

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Aug 08, 2023 at 02:22:05PM +0200, Christoph Heiss wrote:
> No functional changes.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  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.

>  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<LdapMode> 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