From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <l.wagner@proxmox.com>
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 75C86927A6
for <pbs-devel@lists.proxmox.com>; Mon, 13 Feb 2023 15:31:07 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 5F542252FA
for <pbs-devel@lists.proxmox.com>; Mon, 13 Feb 2023 15:31:07 +0100 (CET)
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 <pbs-devel@lists.proxmox.com>; Mon, 13 Feb 2023 15:31:06 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3D22E46F57
for <pbs-devel@lists.proxmox.com>; Mon, 13 Feb 2023 15:31:06 +0100 (CET)
From: Lukas Wagner <l.wagner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Mon, 13 Feb 2023 15:31:04 +0100
Message-Id: <20230213143104.905246-1-l.wagner@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.184 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
Subject: [pbs-devel] [PATCH proxmox-backup] realm sync: show warnings if
attributes do not meet their constraints
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 13 Feb 2023 14:31:07 -0000
Previously, if the value of a synced property did not validate properly
(e.g only 1 character in length instead of the required 2), the whole
sync job failed without any useful error message.
In this commit, the values are validated manually by their
respective StringSchema. If the validation fails, the value is
ignored and a warning is displayed in the task log.
In addition to that, some error messages have been improved.
Also, user sync is now more fault-tolerant in general, showing
warnings if something goes wrong while creating/updating a
single user, instead of aborting the whole sync job.
Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/server/realm_sync_job.rs | 98 ++++++++++++++++++++++++++----------
1 file changed, 72 insertions(+), 26 deletions(-)
diff --git a/src/server/realm_sync_job.rs b/src/server/realm_sync_job.rs
index 0fd8adfe..874fe850 100644
--- a/src/server/realm_sync_job.rs
+++ b/src/server/realm_sync_job.rs
@@ -1,16 +1,18 @@
use anyhow::{bail, Context, Error};
use pbs_config::{acl::AclTree, token_shadow, BackupLockGuard};
+use proxmox_lang::try_block;
use proxmox_ldap::{Config, Connection, SearchParameters, SearchResult};
use proxmox_rest_server::WorkerTask;
-use proxmox_schema::ApiType;
+use proxmox_schema::{ApiType, Schema};
use proxmox_section_config::SectionConfigData;
-use proxmox_sys::task_log;
+use proxmox_sys::{task_log, task_warn};
use std::{collections::HashSet, sync::Arc};
use pbs_api_types::{
ApiToken, Authid, LdapRealmConfig, Realm, RemoveVanished, SyncAttributes as LdapSyncAttributes,
- SyncDefaultsOptions, User, Userid, REMOVE_VANISHED_ARRAY, USER_CLASSES_ARRAY,
+ SyncDefaultsOptions, User, Userid, EMAIL_SCHEMA, FIRST_NAME_SCHEMA, LAST_NAME_SCHEMA,
+ REMOVE_VANISHED_ARRAY, USER_CLASSES_ARRAY,
};
use crate::{auth, server::jobstate::Job};
@@ -157,20 +159,32 @@ impl LdapRealmSyncJob {
let mut retrieved_users = HashSet::new();
for result in users {
- let mut username = result
- .attributes
- .get(&self.ldap_sync_settings.user_attr)
- .context("userid attribute not in search result")?
- .get(0)
- .context("userid attribute array is empty")?
- .clone();
-
- username.push_str(&format!("@{}", self.realm.as_str()));
-
- let userid: Userid = username.parse()?;
- retrieved_users.insert(userid.clone());
-
- self.create_or_update_user(user_config, userid, result)?;
+ let user_id_attribute = &self.ldap_sync_settings.user_attr;
+
+ let result = try_block!({
+ let username = result
+ .attributes
+ .get(user_id_attribute)
+ .context(format!(
+ "userid attribute `{user_id_attribute}` not in LDAP search result"
+ ))?
+ .get(0)
+ .context("userid attribute array is empty")?
+ .clone();
+
+ let username = format!("{username}@{realm}", realm = self.realm.as_str());
+
+ let userid: Userid = username
+ .parse()
+ .context(format!("could not parse username `{username}`"))?;
+ retrieved_users.insert(userid.clone());
+
+ self.create_or_update_user(user_config, &userid, result)?;
+ anyhow::Ok(())
+ });
+ if let Err(e) = result {
+ task_log!(self.worker, "could not create/update user: {e}");
+ }
}
Ok(retrieved_users)
@@ -179,7 +193,7 @@ impl LdapRealmSyncJob {
fn create_or_update_user(
&self,
user_config: &mut SectionConfigData,
- userid: Userid,
+ userid: &Userid,
result: &SearchResult,
) -> Result<(), Error> {
let existing_user = user_config.lookup::<User>("user", userid.as_str()).ok();
@@ -213,37 +227,67 @@ impl LdapRealmSyncJob {
fn construct_or_update_user(
&self,
result: &SearchResult,
- userid: Userid,
+ userid: &Userid,
existing_user: Option<&User>,
) -> User {
- let lookup = |a: Option<&String>| {
- a.and_then(|e| result.attributes.get(e))
+ let lookup = |attribute: &str, ldap_attribute: Option<&String>, schema: Schema| {
+ ldap_attribute
+ .and_then(|e| result.attributes.get(e))
.and_then(|v| v.get(0))
+ .and_then(|value| {
+ let schema = schema.unwrap_string_schema();
+
+ if let Err(e) = schema.check_constraints(value) {
+ task_warn!(
+ self.worker,
+ "{userid}: ignoring attribute `{attribute}`: {e}"
+ );
+
+ None
+ } else {
+ Some(value)
+ }
+ })
.cloned()
};
User {
- userid,
+ userid: userid.clone(),
comment: existing_user.as_ref().and_then(|u| u.comment.clone()),
enable: existing_user
.and_then(|o| o.enable)
.or(Some(self.general_sync_settings.enable_new)),
expire: existing_user.and_then(|u| u.expire).or(Some(0)),
- firstname: lookup(self.ldap_sync_settings.firstname_attr.as_ref()).or_else(|| {
+ firstname: lookup(
+ "firstname",
+ self.ldap_sync_settings.firstname_attr.as_ref(),
+ FIRST_NAME_SCHEMA,
+ )
+ .or_else(|| {
if !self.general_sync_settings.should_remove_properties() {
existing_user.and_then(|o| o.firstname.clone())
} else {
None
}
}),
- lastname: lookup(self.ldap_sync_settings.lastname_attr.as_ref()).or_else(|| {
+ lastname: lookup(
+ "lastname",
+ self.ldap_sync_settings.lastname_attr.as_ref(),
+ LAST_NAME_SCHEMA,
+ )
+ .or_else(|| {
if !self.general_sync_settings.should_remove_properties() {
existing_user.and_then(|o| o.lastname.clone())
} else {
None
}
}),
- email: lookup(self.ldap_sync_settings.email_attr.as_ref()).or_else(|| {
+ email: lookup(
+ "email",
+ self.ldap_sync_settings.email_attr.as_ref(),
+ EMAIL_SCHEMA,
+ )
+ .or_else(|| {
if !self.general_sync_settings.should_remove_properties() {
existing_user.and_then(|o| o.email.clone())
} else {
@@ -304,7 +348,9 @@ impl LdapRealmSyncJob {
user_config.sections.remove(&tokenid_string);
if !self.dry_run {
- token_shadow::delete_secret(&tokenid)?;
+ if let Err(e) = token_shadow::delete_secret(&tokenid) {
+ task_warn!(self.worker, "could not delete token for user {userid}: {e}",)
+ }
}
if self.general_sync_settings.should_remove_acls() {
--
2.30.2