From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Lukas Wagner <l.wagner@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH proxmox-backup 06/17] auth: add LDAP realm authenticator
Date: Wed, 4 Jan 2023 14:32:06 +0100 [thread overview]
Message-ID: <20230104133206.czwci777swboxggi@casey.proxmox.com> (raw)
In-Reply-To: <20230103142308.656240-7-l.wagner@proxmox.com>
On Tue, Jan 03, 2023 at 03:22:57PM +0100, Lukas Wagner wrote:
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> src/auth.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/src/auth.rs b/src/auth.rs
> index f1d5c0a1..101bec0e 100644
> --- a/src/auth.rs
> +++ b/src/auth.rs
> @@ -8,9 +8,12 @@ use std::process::{Command, Stdio};
> use anyhow::{bail, format_err, Error};
> use serde_json::json;
>
> -use pbs_api_types::{RealmRef, Userid, UsernameRef};
> +use pbs_api_types::{LdapMode, LdapRealmConfig, RealmRef, Userid, UsernameRef};
> use pbs_buildcfg::configdir;
>
> +use crate::auth_helpers;
> +use crate::server::ldap::{LdapConfig, LdapConnection, LdapConnectionMode};
> +
> pub trait ProxmoxAuthenticator {
> fn authenticate_user(&self, username: &UsernameRef, password: &str) -> Result<(), Error>;
> fn store_password(&self, username: &UsernameRef, password: &str) -> Result<(), Error>;
> @@ -122,12 +125,45 @@ impl ProxmoxAuthenticator for PBS {
> }
> }
>
> +#[allow(clippy::upper_case_acronyms)]
> +pub struct LDAP {
> + config: LdapRealmConfig,
> +}
> +
> +impl ProxmoxAuthenticator for LDAP {
> + /// Authenticate user in LDAP realm
> + fn authenticate_user(&self, username: &UsernameRef, password: &str) -> Result<(), Error> {
> + let ldap_config = ldap_api_type_to_ldap_config(&self.config)?;
> +
> + let ldap = LdapConnection::new(ldap_config);
> +
> + proxmox_async::runtime::block_on(ldap.authenticate_user(username.as_str(), password))
> + }
> +
> + fn store_password(&self, _username: &UsernameRef, _password: &str) -> Result<(), Error> {
> + // do not store password for LDAP users
> + Ok(())
Actually this should fail.
Otherwise this will make change-password API calls "succeed" without
actually doing anything, but IMO it makes more sense to return a
meaningful error there.
(Perhaps even a http_bail!(NOT_IMPLEMENTED) though I'm not really sure
how the GUI would deal with that ;-) )
> + }
> +
> + fn remove_password(&self, _username: &UsernameRef) -> Result<(), Error> {
> + // do not remove password for LDAP users
^ same here
> + Ok(())
> + }
> +}
> +
> /// Lookup the autenticator for the specified realm
> pub fn lookup_authenticator(realm: &RealmRef) -> Result<Box<dyn ProxmoxAuthenticator>, Error> {
> match realm.as_str() {
> "pam" => Ok(Box::new(PAM())),
> "pbs" => Ok(Box::new(PBS())),
> - _ => bail!("unknown realm '{}'", realm.as_str()),
> + realm => {
> + let (domains, _digest) = pbs_config::domains::config()?;
> + if let Ok(config) = domains.lookup::<LdapRealmConfig>("ldap", realm) {
> + Ok(Box::new(LDAP { config }))
> + } else {
> + bail!("unknown realm '{}'", realm);
> + }
> + }
> }
> }
>
> @@ -135,3 +171,28 @@ pub fn lookup_authenticator(realm: &RealmRef) -> Result<Box<dyn ProxmoxAuthentic
> pub fn authenticate_user(userid: &Userid, password: &str) -> Result<(), Error> {
> lookup_authenticator(userid.realm())?.authenticate_user(userid.name(), password)
> }
> +
> +// TODO: Is there a better place for this?
IMO the best way is to just not :-) (as I mentioned in a reply to the
earlier patches)
Although that would make it more difficult to make this reusable and...
well...
we may still need it in the end, we'll see...
> +pub fn ldap_api_type_to_ldap_config(config: &LdapRealmConfig) -> Result<LdapConfig, Error> {
> + let mut servers = vec![config.server1.clone()];
> + if let Some(server) = &config.server2 {
> + servers.push(server.clone());
> + }
> +
> + let tls_mode = match config.mode.unwrap_or_default() {
> + LdapMode::Ldap => LdapConnectionMode::Ldap,
> + LdapMode::StartTls => LdapConnectionMode::StartTls,
> + LdapMode::Ldaps => LdapConnectionMode::Ldaps,
> + };
> +
> + Ok(LdapConfig {
> + servers,
> + port: config.port,
> + user_attr: config.user_attr.clone(),
> + base_dn: config.base_dn.clone(),
> + bind_dn: config.bind_dn.clone(),
> + bind_password: auth_helpers::get_ldap_bind_password(&config.realm)?,
> + tls_mode,
> + verify_certificate: config.verify.unwrap_or_default(),
> + })
> +}
> --
> 2.30.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
next prev parent reply other threads:[~2023-01-04 13:32 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-03 14:22 [pbs-devel] [PATCH-SERIES proxmox-{backup, widget-toolkit} 00/17] add LDAP realm support Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 01/17] pbs-config: add delete_authid to ACL-tree Lukas Wagner
2023-01-04 10:23 ` Wolfgang Bumiller
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 02/17] ui: add 'realm' field in user edit Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 03/17] api-types: add LDAP configuration type Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 04/17] api: add routes for managing LDAP realms Lukas Wagner
2023-01-04 11:16 ` Wolfgang Bumiller
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 05/17] auth: add LDAP module Lukas Wagner
2023-01-04 13:23 ` Wolfgang Bumiller
2023-01-09 10:52 ` Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 06/17] auth: add LDAP realm authenticator Lukas Wagner
2023-01-04 13:32 ` Wolfgang Bumiller [this message]
2023-01-04 14:48 ` Thomas Lamprecht
2023-01-09 11:00 ` Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 07/17] api-types: add config options for LDAP user sync Lukas Wagner
2023-01-04 13:40 ` Wolfgang Bumiller
2023-01-09 13:58 ` Lukas Wagner
2023-01-03 14:22 ` [pbs-devel] [PATCH proxmox-backup 08/17] server: add LDAP realm sync job Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-backup 09/17] manager: add LDAP commands Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-backup 10/17] manager: add sync command for LDAP realms Lukas Wagner
2023-01-04 13:56 ` Wolfgang Bumiller
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-backup 11/17] docs: add configuration file reference for domains.cfg Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-backup 12/17] docs: add documentation for LDAP realms Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-backup 13/17] auth ldap: add `certificate-path` option Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-widget-toolkit 14/17] auth ui: add LDAP realm edit panel Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-widget-toolkit 15/17] auth ui: add LDAP sync UI Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-widget-toolkit 16/17] auth ui: add `onlineHelp` for AuthEditLDAP Lukas Wagner
2023-01-03 14:23 ` [pbs-devel] [PATCH proxmox-widget-toolkit 17/17] auth ui: add `firstname` and `lastname` sync-attribute fields Lukas Wagner
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=20230104133206.czwci777swboxggi@casey.proxmox.com \
--to=w.bumiller@proxmox.com \
--cc=l.wagner@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox