mart-w/kanidm
1
0
Fork 0
mirror of https://github.com/kanidm/kanidm.git synced 2025-02-23 20:47:01 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Qnn idiomatic refactor (#570)

Browse source
This commit is contained in:
Firstyear 2021-08-21 14:44:55 +10:00 committed by GitHub
parent a3c0b8ccfe
commit 9456cac15b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 466 additions and 631 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

126
kanidmd/src/lib/access.rs
View file

@ -582,7 +582,7 @@ pub trait AccessControlsTransaction<'a> {
// add search_attrs to allowed.
Some(acs.attrs.iter().map(|s| s.as_str()))
} else {
trace!(entry = ?e.get_uuid(), acs = %acs.acp.name, "entry matches acs"); // should this be `security_access`?
trace!(entry = ?e.get_uuid(), acs = %acs.acp.name, "entry DOES NOT match acs"); // should this be `security_access`?
ltrace!(
audit,
"entry {:?} DOES NOT match acs {}",
@ -764,18 +764,9 @@ pub trait AccessControlsTransaction<'a> {
Some(
acs.attrs
.iter()
.map(|s| s.as_str()) // TODO: Refactor in another PR
.map(|s| s.as_str())
.filter(|s| {
// could simplify with as_ref, map, and unwrap_or(true)
match &req_attrs {
// We return all as we requested all.
None => true,
Some(r_attrs) => {
// If we have a req_attrs set, we only return
// things that were requested.
r_attrs.contains(s)
}
}
req_attrs.as_ref().map(|r_attrs| r_attrs.contains(s)).unwrap_or(true)
}),
)
} else {
@ -865,16 +856,11 @@ pub trait AccessControlsTransaction<'a> {
let acp_resolve_filter_cache = self.get_acp_resolve_filter_cache();
// Pre-check if the no-no purge class is present
let disallow = me.modlist.iter().fold(false, |acc, m| {
if acc {
acc
} else {
match m {
Modify::Purged(a) => a == "class",
_ => false,
}
}
});
let disallow = me
.modlist
.iter()
.any(|m| matches!(m, Modify::Purged(a) if a == "class"));
if disallow {
lsecurity_access!(audit, "Disallowing purge class in modification");
return Ok(false);
@ -899,9 +885,7 @@ pub trait AccessControlsTransaction<'a> {
e
})
.ok()
.map(|f_res|
(acs, f_res)
)
.map(|f_res| (acs, f_res))
} else {
None
}
@ -977,10 +961,7 @@ pub trait AccessControlsTransaction<'a> {
lsecurity_access!(audit, "Requested remove set: {:?}", requested_rem);
lsecurity_access!(audit, "Requested class set: {:?}", requested_classes);
let r = entries.iter().fold(true, |acc, e| {
if !acc {
false
} else {
let r = entries.iter().all(|e| {
// For this entry, find the acp's that apply to it from the
// set that apply to the entry that is performing the operation
let scoped_acp: Vec<&AccessControlModify> = related_acp
@ -1022,17 +1003,11 @@ pub trait AccessControlsTransaction<'a> {
false
} else if !requested_classes.is_subset(&allowed_classes) {
lsecurity_access!(audit, "requested_classes is not a subset of allowed");
lsecurity_access!(
audit,
"{:?} !⊆ {:?}",
requested_classes,
allowed_classes
);
lsecurity_access!(audit, "{:?} !⊆ {:?}", requested_classes, allowed_classes);
false
} else {
lsecurity_access!(audit, "passed pres, rem, classes check.");
true
}
} // if acc == false
});
if r {
@ -1070,10 +1045,14 @@ pub trait AccessControlsTransaction<'a> {
let related_acp: Vec<(&AccessControlCreate, _)> = create_state
.iter()
.filter_map(|acs| {
match (&acs.acp.receiver).resolve(&ce.ident, None, Some(acp_resolve_filter_cache)) {
Ok(f_res) => {
if rec_entry.entry_match_no_index(&f_res) {
(&acs.acp.targetscope)
match acs
.acp
.receiver
.resolve(&ce.ident, None, Some(acp_resolve_filter_cache))
{
Ok(f_res) if rec_entry.entry_match_no_index(&f_res) => acs
.acp
.targetscope
.resolve(&ce.ident, None, Some(acp_resolve_filter_cache))
.map_err(|e| {
ladmin_error!(
@ -1084,13 +1063,8 @@ pub trait AccessControlsTransaction<'a> {
e
})
.ok()
.map(|f_res|
(acs, f_res)
)
} else {
None
}
}
.map(|f_res| (acs, f_res)),
Ok(_) => None,
Err(e) => {
ladmin_error!(
audit,
@ -1106,11 +1080,7 @@ pub trait AccessControlsTransaction<'a> {
// lsecurity_access!(audit, "Related acc -> {:?}", related_acp);
// For each entry
let r = entries.iter().fold(true, |acc, e| {
if !acc {
// We have already failed, move on.
false
} else {
let r = entries.iter().all(|e| {
// Build the set of requested classes and attrs here.
let create_attrs: BTreeSet<&str> = e.get_ava_names().collect();
// If this is empty, we make an empty set, which is fine because
@ -1135,11 +1105,7 @@ pub trait AccessControlsTransaction<'a> {
}
};
related_acp.iter().fold(false, |r_acc, (accr, f_res)| {
if r_acc {
// Already allowed, continue.
r_acc
} else {
related_acp.iter().any(|(accr, f_res)| {
// Check to see if allowed.
if e.entry_match_no_index(&f_res) {
lsecurity_access!(audit, "entry {:?} matches acs {:?}", e, accr);
@ -1152,23 +1118,12 @@ pub trait AccessControlsTransaction<'a> {
accr.classes.iter().map(|s| s.as_str()).collect();
if !create_attrs.is_subset(&allowed_attrs) {
lsecurity_access!(
audit,
"create_attrs is not a subset of allowed"
);
lsecurity_access!(
audit,
"{:?} !⊆ {:?}",
create_attrs,
allowed_attrs
);
lsecurity_access!(audit, "create_attrs is not a subset of allowed");
lsecurity_access!(audit, "{:?} !⊆ {:?}", create_attrs, allowed_attrs);
return false;
}
if !create_classes.is_subset(&allowed_classes) {
lsecurity_access!(
audit,
"create_classes is not a subset of allowed"
);
lsecurity_access!(audit, "create_classes is not a subset of allowed");
lsecurity_access!(
audit,
"{:?} !⊆ {:?}",
@ -1181,18 +1136,11 @@ pub trait AccessControlsTransaction<'a> {
true
} else {
ltrace!(
audit,
"entry {:?} DOES NOT match acs {}",
e,
accr.acp.name
);
ltrace!(audit, "entry {:?} DOES NOT match acs {}", e, accr.acp.name);
// Does not match, fail this rule.
false
}
}
})
}
// Find the set of related acps for this entry.
//
// For each "created" entry.
@ -1237,10 +1185,10 @@ pub trait AccessControlsTransaction<'a> {
let related_acp: Vec<(&AccessControlDelete, _)> = delete_state
.iter()
.filter_map(|acs| {
match (&acs.acp.receiver).resolve(&de.ident, None, Some(acp_resolve_filter_cache)) {
match acs.acp.receiver.resolve(&de.ident, None, Some(acp_resolve_filter_cache)) {
Ok(f_res) => {
if rec_entry.entry_match_no_index(&f_res) {
(&acs.acp.targetscope)
acs.acp.targetscope
.resolve(&de.ident, None, Some(acp_resolve_filter_cache))
.map_err(|e| {
ladmin_error!(
@ -1277,16 +1225,9 @@ pub trait AccessControlsTransaction<'a> {
*/
// For each entry
let r = entries.iter().fold(true, |acc, e| {
if !acc {
// Any false, denies the whole operation.
false
} else {
related_acp.iter().fold(false, |r_acc, (acd, f_res)| {
if r_acc {
// If something allowed us to delete, skip doing silly work.
r_acc
} else if e.entry_match_no_index(&f_res) {
let r = entries.iter().all(|e| {
related_acp.iter().any(|(acd, f_res)| {
if e.entry_match_no_index(&f_res) {
lsecurity_access!(
audit,
"entry {:?} matches acs {}",
@ -1306,8 +1247,7 @@ pub trait AccessControlsTransaction<'a> {
// Does not match, fail.
false
} // else
}) // fold related_acp
} // if/else
}) // any related_acp
});
if r {
lsecurity_access!(audit, "allowed ✅");

73
kanidmd/src/lib/actors/v1_read.rs
View file

@ -122,30 +122,21 @@ impl QueryServerReadV1 {
})?;
// Make an event from the request
// TODO: Refactor this in another PR to use `map_err` and `?`
let search = match SearchEvent::from_message(
&mut audit,
ident,
&req,
&idms_prox_read.qs_read,
) {
Ok(s) => s,
Err(e) => {
let search =
SearchEvent::from_message(&mut audit, ident, &req, &idms_prox_read.qs_read)
.map_err(|e| {
admin_error!(?e, "Failed to begin search");
ladmin_error!(audit, "Failed to begin search: {:?}", e);
return Err(e);
}
};
e
})?;
trace!(?search, "Begin event");
ltrace!(audit, "Begin event {:?}", search);
// TODO: Refactor this in another PR to use `?`
match idms_prox_read.qs_read.search_ext(&mut audit, &search) {
Ok(entries) => SearchResult::new(&mut audit, &idms_prox_read.qs_read, &entries)
.map(SearchResult::response),
Err(e) => Err(e),
}
let entries = idms_prox_read.qs_read.search_ext(&mut audit, &search)?;
SearchResult::new(&mut audit, &idms_prox_read.qs_read, &entries)
.map(SearchResult::response)
})
});
// At the end of the event we send it for logging.
@ -387,48 +378,26 @@ impl QueryServerReadV1 {
e
})?;
// TODO: Refactor this in another PR to use `?`
let srch = match SearchEvent::from_whoami_request(
&mut audit,
ident,
&idms_prox_read.qs_read,
) {
Ok(s) => s,
Err(e) => {
let srch =
SearchEvent::from_whoami_request(&mut audit, ident, &idms_prox_read.qs_read)
.map_err(|e| {
admin_error!(?e, "Failed to begin whoami");
ladmin_error!(audit, "Failed to begin whoami: {:?}", e);
return Err(e);
}
};
e
})?;
trace!(search = ?srch, "Begin event");
ltrace!(audit, "Begin event {:?}", srch);
// TODO: Refactor this in another PR to use `?`
match idms_prox_read.qs_read.search_ext(&mut audit, &srch) {
Ok(mut entries) => {
// assert there is only one ...
match entries.len() {
0 => Err(OperationError::NoMatchingEntries),
1 => {
#[allow(clippy::expect_used)]
let e = entries.pop().expect("Entry length mismatch!!!");
// Now convert to a response, and return
let mut entries = idms_prox_read.qs_read.search_ext(&mut audit, &srch)?;
match entries.pop() {
Some(e) if entries.is_empty() => {
WhoamiResult::new(&mut audit, &idms_prox_read.qs_read, &e, uat)
.map(|ok_wr| ok_wr.response())
.map(WhoamiResult::response)
}
// Somehow we matched multiple, which should be impossible.
_ => Err(OperationError::InvalidState),
}
// TODO: Refactor this in another PR use use the following
// entries.pop() {
// Some(e) if entries.is_empty() => // whoami stuff...
// Some(_) => Err(OperationError::InvalidState) // matched multiple
// _ => Err(OperationError::NoMatchingEntries),
// }
}
// Something else went wrong ...
Err(e) => Err(e),
Some(_) => Err(OperationError::InvalidState), // Somehow matched multiple entries...
_ => Err(OperationError::NoMatchingEntries),
}
})
});

2
kanidmd/src/lib/be/idl_sqlite.rs
View file

@ -21,11 +21,13 @@ use uuid::Uuid;
const DBV_ID2ENTRY: &str = "id2entry";
const DBV_INDEXV: &str = "indexv";
#[allow(clippy::needless_pass_by_value)] // needs to accept value from `map_err`
fn sqlite_error(e: rusqlite::Error) -> OperationError {
admin_error!(?e, "SQLite Error");
OperationError::SqliteError
}
#[allow(clippy::needless_pass_by_value)] // needs to accept value from `map_err`
fn serde_cbor_error(e: serde_cbor::Error) -> OperationError {
admin_error!(?e, "Serde CBOR Error");
OperationError::SerdeCborError

27
kanidmd/src/lib/credential/mod.rs
View file

@ -341,8 +341,7 @@ impl TryFrom<DbCredV1> for Credential {
None => None,
};
let v_webauthn = match webauthn {
Some(dbw) => Some(
let v_webauthn = webauthn.map(|dbw| {
dbw.into_iter()
.map(|wc| {
(
@ -356,10 +355,8 @@ impl TryFrom<DbCredV1> for Credential {
},
)
})
.collect(),
),
None => None,
};
.collect()
});
let v_backup_code = match backup_code {
Some(dbb) => Some(BackupCodes::try_from(dbb)?),
@ -544,11 +541,11 @@ impl Credential {
}
CredentialType::PasswordMfa(_, _, map, _) | CredentialType::Webauthn(map) => map
.iter()
.fold(None, |acc, (k, v)| {
if acc.is_none() && &v.cred_id == cid && v.counter < counter {
.find_map(|(k, v)| {
if &v.cred_id == cid && v.counter < counter {
Some(k)
} else {
acc
None
}
})
.map(|label| {
@ -841,13 +838,13 @@ impl Credential {
match &self.type_ {
CredentialType::PasswordMfa(_, _, _, opt_bc) => opt_bc
.as_ref()
.ok_or(OperationError::InvalidAccountState(String::from(
"No backup codes are available for this account",
)))
.and_then(|bc| {
Ok(BackupCodesView {
backup_codes: bc.code_set.clone().into_iter().collect(),
.ok_or_else(|| {
OperationError::InvalidAccountState(
"No backup codes are available for this account".to_string(),
)
})
.map(|bc| BackupCodesView {
backup_codes: bc.code_set.clone().into_iter().collect(),
}),
_ => Err(OperationError::InvalidAccountState(
"Non-MFA credential type".to_string(),

2
kanidmd/src/lib/entry.rs
View file

@ -605,7 +605,7 @@ impl<STATE> Entry<EntryInvalid, STATE> {
.map(|s| {
// This should NOT fail - if it does, it means our schema is
// in an invalid state!
Ok(schema_attributes.get(s).ok_or(SchemaError::Corrupted)?)
schema_attributes.get(s).ok_or(SchemaError::Corrupted)
})
.collect();

58
kanidmd/src/lib/idm/authsession.rs
View file

@ -957,12 +957,7 @@ mod tests {
);
if let AuthState::Choose(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthMech::Anonymous => true,
_ => acc,
})
);
assert!(auth_mechs.iter().any(|x| matches!(x, AuthMech::Anonymous)));
} else {
panic!("Invalid auth state")
}
@ -973,12 +968,9 @@ mod tests {
.expect("Failed to select anonymous mech.");
if let AuthState::Continue(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthAllowed::Anonymous => true,
_ => acc,
})
);
assert!(auth_mechs
.iter()
.any(|x| matches!(x, AuthAllowed::Anonymous)));
} else {
panic!("Invalid auth state")
}
@ -999,12 +991,7 @@ mod tests {
let mut session = session.unwrap();
if let AuthState::Choose(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthMech::Password => true,
_ => acc,
})
);
assert!(auth_mechs.iter().any(|x| matches!(x, AuthMech::Password)));
} else {
panic!();
}
@ -1014,12 +1001,9 @@ mod tests {
.expect("Failed to select anonymous mech.");
if let AuthState::Continue(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthAllowed::Password => true,
_ => acc,
})
);
assert!(auth_mechs
.iter()
.any(|x| matches!(x, AuthAllowed::Password)));
} else {
panic!("Invalid auth state")
}
@ -1147,12 +1131,9 @@ mod tests {
let mut session = session.expect("Session was unable to be created.");
if let AuthState::Choose(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthMech::PasswordMfa => true,
_ => acc,
})
);
assert!(auth_mechs
.iter()
.any(|x| matches!(x, AuthMech::PasswordMfa)))
} else {
panic!();
}
@ -1171,10 +1152,20 @@ mod tests {
rchal = Some(chal.clone());
true
}
// Why does this also return `true`? If we hit this but not
// Webauthn, then we will panic when unwrapping `rchal` later...
AuthAllowed::Totp => true,
_ => acc,
})
);
// I feel like this is what we should be doing
// assuming there will only be one `AuthAllowed::Webauthn`.
// rchal = auth_mechs.iter().find_map(|x| match x {
// AuthAllowed::Webauthn(chal) => Some(chal),
// _ => None,
// });
// assert!(rchal.is_some());
} else {
panic!("Invalid auth state")
}
@ -1441,12 +1432,7 @@ mod tests {
let mut session = session.unwrap();
if let AuthState::Choose(auth_mechs) = state {
assert!(
true == auth_mechs.iter().fold(false, |acc, x| match x {
AuthMech::Webauthn => true,
_ => acc,
})
);
assert!(auth_mechs.iter().any(|x| matches!(x, AuthMech::Webauthn)));
} else {
panic!();
}

3
kanidmd/src/lib/idm/group.rs
View file

@ -22,7 +22,7 @@ macro_rules! try_from_account_e {
($au:expr, $value:expr, $qs:expr) => {{
let name = $value
.get_ava_single_str("name")
.map(|s| s.to_string()) // TODO: Refactor in another PR
.map(str::to_string)
.ok_or_else(|| {
OperationError::InvalidAccountState("Missing attribute: name".to_string())
})?;
@ -31,7 +31,6 @@ macro_rules! try_from_account_e {
let upg = Group { name, uuid };
// TODO: Refactor to use `map` and `unwrap_or_else`
let mut groups: Vec<Group> = match $value.get_ava_as_refuuid("memberof") {
Some(riter) => {
// given a list of uuid, make a filter: even if this is empty, the be will

45
kanidmd/src/lib/plugins/protected.rs
View file

@ -60,9 +60,7 @@ impl Plugin for Protected {
return Ok(());
}
cand.iter().fold(Ok(()), |acc, cand| match acc {
Err(_) => acc,
Ok(_) => {
cand.iter().try_fold((), |(), cand| {
if cand.attribute_equality("class", &PVCLASS_SYSTEM)
|| cand.attribute_equality("class", &PVCLASS_DOMAIN_INFO)
|| cand.attribute_equality("class", &PVCLASS_SYSTEM_INFO)
@ -72,8 +70,7 @@ impl Plugin for Protected {
{
Err(OperationError::SystemProtectedObject)
} else {
acc
}
Ok(())
}
})
}
@ -93,11 +90,7 @@ impl Plugin for Protected {
return Ok(());
}
// Prevent adding class: system, domain_info, tombstone, or recycled.
me.modlist.iter().fold(Ok(()), |acc, m| {
if acc.is_err() {
acc
} else {
match m {
me.modlist.iter().try_fold((), |(), m| match m {
Modify::Present(a, v) => {
// TODO: Can we avoid this clone?
if a == "class"
@ -114,34 +107,25 @@ impl Plugin for Protected {
}
}
_ => Ok(()),
}
}
})?;
// HARD block mods on tombstone or recycle. We soft block on the rest as they may
// have some allowed attrs.
cand.iter().fold(Ok(()), |acc, cand| match acc {
Err(_) => acc,
Ok(_) => {
cand.iter().try_fold((), |(), cand| {
if cand.attribute_equality("class", &PVCLASS_TOMBSTONE)
|| cand.attribute_equality("class", &PVCLASS_RECYCLED)
{
Err(OperationError::SystemProtectedObject)
} else {
acc
}
Ok(())
}
})?;
// if class: system, check the mods are "allowed"
let system_pres = cand.iter().fold(false, |acc, c| {
if acc {
acc
} else {
let system_pres = cand.iter().any(|c| {
// We don't need to check for domain info here because domain_info has a class
// system also. We just need to block it from being created.
c.attribute_equality("class", &PVCLASS_SYSTEM)
}
});
ltrace!(au, "class: system -> {}", system_pres);
@ -151,21 +135,15 @@ impl Plugin for Protected {
}
// Something altered is system, check if it's allowed.
me.modlist.iter().fold(Ok(()), |acc, m| {
me.modlist.iter().try_fold((), |(), m| {
// Already hit an error, move on.
if acc.is_err() {
acc
} else {
let a = match m {
Modify::Present(a, _) => a,
Modify::Removed(a, _) => a,
Modify::Purged(a) => a,
Modify::Present(a, _) | Modify::Removed(a, _) | Modify::Purged(a) => a,
};
match ALLOWED_ATTRS.get(a.as_str()) {
Some(_) => Ok(()),
None => Err(OperationError::SystemProtectedObject),
}
}
})
}
@ -184,9 +162,7 @@ impl Plugin for Protected {
return Ok(());
}
cand.iter().fold(Ok(()), |acc, cand| match acc {
Err(_) => acc,
Ok(_) => {
cand.iter().try_fold((), |(), cand| {
if cand.attribute_equality("class", &PVCLASS_SYSTEM)
|| cand.attribute_equality("class", &PVCLASS_DOMAIN_INFO)
|| cand.attribute_equality("class", &PVCLASS_SYSTEM_INFO)
@ -196,8 +172,7 @@ impl Plugin for Protected {
{
Err(OperationError::SystemProtectedObject)
} else {
acc
}
Ok(())
}
})
}

16
kanidmd/src/lib/plugins/spn.rs
View file

@ -139,18 +139,14 @@ impl Plugin for Spn {
// trigger the spn regen ... which is expensive. Future
// TODO #157: will be improvements to modify on large txns.
let domain_name_changed =
cand.iter()
.zip(pre_cand.iter())
.fold(None, |acc, (post, pre)| {
if acc.is_some() {
acc
} else if post.attribute_equality("uuid", &PV_UUID_DOMAIN_INFO)
&& post.get_ava_single("domain_name") != pre.get_ava_single("domain_name")
let domain_name_changed = cand.iter().zip(pre_cand.iter()).find_map(|(post, pre)| {
let domain_name = post.get_ava_single("domain_name");
if post.attribute_equality("uuid", &PV_UUID_DOMAIN_INFO)
&& domain_name != pre.get_ava_single("domain_name")
{
post.get_ava_single("domain_name")
domain_name
} else {
acc
None
}
});

162
kanidmd/src/lib/server.rs
View file

@ -451,23 +451,10 @@ pub trait QueryServerTransaction<'a> {
})?;
let se = SearchEvent::new_internal(f_valid);
// TODO: Refactor to use this
// let mut vs = self.search(audit, &se)?;
// match vs.pop() {
// Some(entry) if vs.is_empty() => Ok(entry),
// _ => Err(OperationError::NoMatchingEntries),
// }
let res = self.search(audit, &se);
match res {
Ok(vs) => {
if vs.len() > 1 {
return Err(OperationError::NoMatchingEntries);
}
vs.into_iter()
.next()
.ok_or(OperationError::NoMatchingEntries)
}
Err(e) => Err(e),
let mut vs = self.search(audit, &se)?;
match vs.pop() {
Some(entry) if vs.is_empty() => Ok(entry),
_ => Err(OperationError::NoMatchingEntries),
}
})
})
@ -484,23 +471,11 @@ pub trait QueryServerTransaction<'a> {
lperf_segment!(audit, "server::internal_search_ext_uuid", || {
let filter_intent = filter_all!(f_eq("uuid", PartialValue::new_uuid(*uuid)));
let filter = filter!(f_eq("uuid", PartialValue::new_uuid(*uuid)));
let res = self.impersonate_search_ext(audit, filter, filter_intent, event);
// TODO: Refactor to use this
// let mut vs = self.impersonate_search_ext(audit, filter, filter_intent, event)?;
// match vs.pop() {
// Some(entry) if vs.is_empty() => Ok(entry),
// _ => Err(OperationError::NoMatchingEntries),
// }
match res {
Ok(vs) => {
if vs.len() > 1 {
return Err(OperationError::NoMatchingEntries);
}
vs.into_iter()
.next()
.ok_or(OperationError::NoMatchingEntries)
}
Err(e) => Err(e),
let mut vs = self.impersonate_search_ext(audit, filter, filter_intent, event)?;
match vs.pop() {
Some(entry) if vs.is_empty() => Ok(entry),
_ => Err(OperationError::NoMatchingEntries),
}
})
})
@ -517,23 +492,11 @@ pub trait QueryServerTransaction<'a> {
lperf_segment!(audit, "server::internal_search_uuid", || {
let filter_intent = filter_all!(f_eq("uuid", PartialValue::new_uuid(*uuid)));
let filter = filter!(f_eq("uuid", PartialValue::new_uuid(*uuid)));
let res = self.impersonate_search(audit, filter, filter_intent, event);
// TODO: Refactor to use this
// let mut vs = self.impersonate_search(audit, filter, filter_intent, event)?;
// match vs.pop() {
// Some(entry) if vs.is_empty() => Ok(entry),
// _ => Err(OperationError::NoMatchingEntries),
// }
match res {
Ok(vs) => {
if vs.len() > 1 {
return Err(OperationError::NoMatchingEntries);
}
vs.into_iter()
.next()
.ok_or(OperationError::NoMatchingEntries)
}
Err(e) => Err(e),
let mut vs = self.impersonate_search(audit, filter, filter_intent, event)?;
match vs.pop() {
Some(entry) if vs.is_empty() => Ok(entry),
_ => Err(OperationError::NoMatchingEntries),
}
})
})
@ -767,7 +730,7 @@ pub trait QueryServerTransaction<'a> {
} else if value.is_sshkey() {
value
.get_sshkey()
.map(|s| s.to_string()) // TODO: Refactor in another PR
.map(str::to_string)
.ok_or(OperationError::InvalidValueState)
} else {
// Not? Okay, do the to string.
@ -782,7 +745,7 @@ pub trait QueryServerTransaction<'a> {
self.internal_search_uuid(audit, &UUID_DOMAIN_INFO)
.and_then(|e| {
e.get_ava_single_str("domain_name")
.map(|s| s.to_string()) // TODO: Refactor in another PR
.map(str::to_string)
.ok_or(OperationError::InvalidEntryState)
})
.map_err(|e| {
@ -804,7 +767,7 @@ pub trait QueryServerTransaction<'a> {
let mut badlist_hashset = HashSet::with_capacity(badlist_entry.len());
badlist_entry
.iter()
.filter_map(|e| e.as_string()) // TODO: Refactor in another PR
.filter_map(Value::as_string)
.cloned()
.for_each(|s| {
badlist_hashset.insert(s);
@ -1305,18 +1268,17 @@ impl<'a> QueryServerWriteTransaction<'a> {
}
// Now, delete only what you can see
let pre_candidates = match self.impersonate_search_valid(
let pre_candidates = self
.impersonate_search_valid(
audit,
de.filter.clone(),
de.filter_orig.clone(),
&de.ident,
) {
Ok(results) => results,
Err(e) => {
)
.map_err(|e| {
ladmin_error!(audit, "delete: error in pre-candidate selection {:?}", e);
return Err(e);
}
};
e
})?;
// Apply access controls to reduce the set if required.
// delete_allow_operation
@ -1367,7 +1329,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
OperationError::SchemaViolation(e)
})
// seal if it worked.
.map(|r| r.seal())
.map(Entry::seal)
})
.collect();
@ -1440,16 +1402,13 @@ impl<'a> QueryServerWriteTransaction<'a> {
ladmin_error!(audit, "Unable to generate search cid {:?}", e);
e
})?;
let ts = match self.internal_search(
let ts = self.internal_search(
audit,
filter_all!(f_and!([
f_eq("class", PVCLASS_TOMBSTONE.clone()),
f_lt("last_modified_cid", PartialValue::new_cid(cid)),
])),
) {
Ok(r) => r,
Err(e) => return Err(e),
};
)?;
if ts.is_empty() {
ladmin_info!(audit, "No Tombstones present - purge operation success");
@ -1477,16 +1436,13 @@ impl<'a> QueryServerWriteTransaction<'a> {
ladmin_error!(audit, "Unable to generate search cid {:?}", e);
e
})?;
let rc = match self.internal_search(
let rc = self.internal_search(
audit,
filter_all!(f_and!([
f_eq("class", PVCLASS_RECYCLED.clone()),
f_lt("last_modified_cid", PartialValue::new_cid(cid)),
])),
) {
Ok(r) => r,
Err(e) => return Err(e),
};
)?;
if rc.is_empty() {
ladmin_info!(audit, "No recycled present - purge operation success");
@ -1504,7 +1460,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
OperationError::SchemaViolation(e)
})
// seal if it worked.
.map(|r| r.seal())
.map(Entry::seal)
})
.collect();
@ -1560,12 +1516,12 @@ impl<'a> QueryServerWriteTransaction<'a> {
let mut dm_mods: HashMap<Uuid, ModifyList<ModifyInvalid>> =
HashMap::with_capacity(revive_cands.len());
revive_cands.into_iter().for_each(|e| {
for e in revive_cands {
// Get this entries uuid.
let u: Uuid = *e.get_uuid();
if let Some(riter) = e.get_ava_as_refuuid("directmemberof") {
riter.for_each(|g_uuid| {
for g_uuid in riter {
dm_mods
.entry(*g_uuid)
.and_modify(|mlist| {
@ -1582,9 +1538,9 @@ impl<'a> QueryServerWriteTransaction<'a> {
);
ModifyList::new_list(vec![m])
});
});
};
});
}
}
}
// Now impersonate the modify
self.impersonate_modify_valid(
@ -1596,13 +1552,13 @@ impl<'a> QueryServerWriteTransaction<'a> {
)?;
// If and only if that succeeds, apply the direct membership modifications
// if possible.
let r: Result<_, _> = dm_mods.into_iter().try_for_each(|(g, mods)| {
for (g, mods) in dm_mods {
// I think the filter/filter_all shouldn't matter here because the only
// valid direct memberships should be still valid/live references.
let f = filter_all!(f_eq("uuid", PartialValue::new_uuid(g)));
self.internal_modify(audit, &f, &mods)
});
r
self.internal_modify(audit, &f, &mods)?;
}
Ok(())
})
}
@ -1640,19 +1596,18 @@ impl<'a> QueryServerWriteTransaction<'a> {
// This is now done in the event transform
// This also checks access controls due to use of the impersonation.
let pre_candidates = match self.impersonate_search_valid(
let pre_candidates = self
.impersonate_search_valid(
audit,
me.filter.clone(),
me.filter_orig.clone(),
&me.ident,
) {
Ok(results) => results,
Err(e) => {
)
.map_err(|e| {
admin_error!("modify: error in pre-candidate selection {:?}", e);
ladmin_error!(audit, "modify: error in pre-candidate selection {:?}", e);
return Err(e);
}
};
e
})?;
if pre_candidates.is_empty() {
if me.ident.is_internal() {
@ -1905,7 +1860,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
ladmin_error!(audit, "Schema Violation {:?}", e);
OperationError::SchemaViolation(e)
})
.map(|e| e.seal())
.map(Entry::seal)
})
.collect();
@ -2008,12 +1963,12 @@ impl<'a> QueryServerWriteTransaction<'a> {
candidates.iter_mut().for_each(|er| {
let opt_names: Option<BTreeSet<_>> = er.pop_ava("name").map(|vs| {
vs.into_iter()
.filter_map(|v| v.migrate_iutf8_iname())
.filter_map(Value::migrate_iutf8_iname)
.collect()
});
let opt_dnames: Option<BTreeSet<_>> = er.pop_ava("domain_name").map(|vs| {
vs.into_iter()
.filter_map(|v| v.migrate_iutf8_iname())
.filter_map(Value::migrate_iutf8_iname)
.collect()
});
@ -2030,7 +1985,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
// Schema check all.
let res: Result<Vec<Entry<EntrySealed, EntryCommitted>>, SchemaError> = candidates
.into_iter()
.map(|e| e.validate(&self.schema).map(|e| e.seal()))
.map(|e| e.validate(&self.schema).map(Entry::seal))
.collect();
let norm_cand: Vec<Entry<_, _>> = match res {
@ -2227,8 +2182,8 @@ impl<'a> QueryServerWriteTransaction<'a> {
ltrace!(audit, "internal_migrate_or_create search {:?}", filt);
match self.internal_search(audit, filt.clone()) {
Ok(results) => {
let results = self.internal_search(audit, filt.clone())?;
if results.is_empty() {
// It does not exist. Create it.
self.internal_create(audit, vec![e])
@ -2252,12 +2207,6 @@ impl<'a> QueryServerWriteTransaction<'a> {
Err(OperationError::InvalidDbState)
}
}
Err(e) => {
// An error occured. pass it back up.
Err(e)
}
}
}
/*
pub fn internal_assert_or_create_str(
@ -2340,6 +2289,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
} else {
ladmin_error!(audit, "initialise_schema_core -> Error {:?}", r);
}
// why do we have error handling if it's always supposed to be `Ok`?
debug_assert!(r.is_ok());
r
}
@ -2380,19 +2330,19 @@ impl<'a> QueryServerWriteTransaction<'a> {
JSON_SCHEMA_ATTR_NSUNIQUEID,
];
let r: Result<Vec<()>, _> = idm_schema
let r = idm_schema
.iter()
// Each item individually logs it's result
.map(|e_str| self.internal_migrate_or_create_str(audit, e_str))
.collect();
.try_for_each(|e_str| self.internal_migrate_or_create_str(audit, e_str));
if r.is_ok() {
ladmin_info!(audit, "initialise_schema_idm -> Ok!");
} else {
ladmin_error!(audit, "initialise_schema_idm -> Error {:?}", r);
}
debug_assert!(r.is_ok());
debug_assert!(r.is_ok()); // why return a result if we assert it's `Ok`?
r.map(|_| ())
r
}
// This function is idempotent

4
kanidmd/src/lib/tracing_tree/formatter.rs
View file

@ -111,6 +111,7 @@ fn format_json(processed_logs: &TreeProcessed) -> Vec<u8> {
}
}
#[allow(clippy::expect_used)]
let uuid = span
.uuid
.as_deref()
@ -135,6 +136,7 @@ fn format_json(processed_logs: &TreeProcessed) -> Vec<u8> {
let mut writer = vec![];
let mut spans = vec![];
#[allow(clippy::expect_used)]
fmt_rec(&processed_logs, &mut spans, None, &mut writer).expect("Write failed");
writer
}
@ -195,6 +197,7 @@ fn format_pretty(processed_logs: &TreeProcessed) -> Vec<u8> {
writeln!(writer)
}
TreeProcessed::Span(span) => {
#[allow(clippy::expect_used)]
let uuid = span
.uuid
.as_deref()
@ -287,6 +290,7 @@ fn format_pretty(processed_logs: &TreeProcessed) -> Vec<u8> {
let mut writer = vec![];
let mut indent = vec![];
#[allow(clippy::expect_used)]
fmt_rec(&processed_logs, &mut indent, None, None, &mut writer).expect("Write failed");
writer
}

2
kanidmd/src/lib/tracing_tree/processor.rs
View file

@ -19,6 +19,7 @@ impl ExportProcessor {
impl Processor for ExportProcessor {
fn process(&self, preprocessed: TreePreProcessed) {
#[allow(clippy::expect_used)]
self.sender
.send(preprocessed)
.expect("Processing channel has been closed, cannot log events.");
@ -27,6 +28,7 @@ impl Processor for ExportProcessor {
impl Processor for TestProcessor {
fn process(&self, preprocessed: TreePreProcessed) {
#[allow(clippy::expect_used)]
preprocessed.process().expect("Failed to write logs");
}
}

19
kanidmd/src/lib/tracing_tree/subscriber.rs
View file

@ -105,6 +105,7 @@ impl TreeSubscriber<ExportProcessor> {
let subscriber = TreeSubscriber::new_with(fmt, log_tx);
let logger = async move {
while let Some(processor) = log_rx.recv().await {
#[allow(clippy::expect_used)]
processor.process().expect("Failed to write logs");
}
};
@ -130,6 +131,7 @@ impl<P: Processor> TreeSubscriber<P> {
let current = self.inner.current_span();
// If there's no current span, we short-circuit.
let id = current.id()?;
#[allow(clippy::expect_used)]
let span = self
.inner
.span(id)
@ -138,12 +140,14 @@ impl<P: Processor> TreeSubscriber<P> {
span.scope().into_iter().find_map(|span| {
let extensions = span.extensions();
// If `uuid` is `None`, then we keep searching.
#[allow(clippy::expect_used)]
let uuid = extensions
.get::<TreeSpan>()
.expect("Span buffer not found, this is a bug")
.uuid
.as_ref()?;
// TODO: make spans store UUID's as a u128 or 2 u64's
#[allow(clippy::expect_used)]
Some(Uuid::parse_str(uuid.as_str()).expect("Unable to parse UUID, this is a bug"))
})
}
@ -204,6 +208,7 @@ impl<P: Processor> TreeLayer<P> {
fn log_to_parent(&self, logs: Tree, parent: Option<SpanRef<Registry>>) {
match parent {
// The parent exists- write to them
#[allow(clippy::expect_used)]
Some(span) => span
.extensions_mut()
.get_mut::<TreeSpan>()
@ -220,6 +225,7 @@ impl<P: Processor> TreeLayer<P> {
impl<P: Processor> Layer<Registry> for TreeLayer<P> {
fn new_span(&self, attrs: &Attributes, id: &Id, ctx: Context<Registry>) {
#[allow(clippy::expect_used)]
let span = ctx.span(id).expect("Span not found, this is a bug");
let name = attrs.metadata().name();
@ -261,6 +267,7 @@ impl<P: Processor> Layer<Registry> for TreeLayer<P> {
if immediate {
use super::formatter::format_immediate_event;
let maybe_scope = ctx.event_scope(event).map(Scope::from_root);
#[allow(clippy::expect_used)]
let formatted_event = format_immediate_event(&tree_event, maybe_scope)
.expect("Formatting immediate event failed");
eprintln!("{}", formatted_event);
@ -270,6 +277,7 @@ impl<P: Processor> Layer<Registry> for TreeLayer<P> {
}
fn on_enter(&self, id: &Id, ctx: Context<Registry>) {
#[allow(clippy::expect_used)]
ctx.span(id)
.expect("Span not found, this is a bug")
.extensions_mut()
@ -279,6 +287,7 @@ impl<P: Processor> Layer<Registry> for TreeLayer<P> {
}
fn on_exit(&self, id: &Id, ctx: Context<Registry>) {
#[allow(clippy::expect_used)]
ctx.span(id)
.expect("Span not found, this is a bug")
.extensions_mut()
@ -288,14 +297,17 @@ impl<P: Processor> Layer<Registry> for TreeLayer<P> {
}
fn on_close(&self, id: Id, ctx: Context<Registry>) {
#[allow(clippy::expect_used)]
let span = ctx.span(&id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();
#[allow(clippy::expect_used)]
let span_buf = extensions
.remove::<TreeSpan>()
.expect("Span buffer not found, this is a bug");
#[allow(clippy::expect_used)]
let duration = extensions
.remove::<Timer>()
.expect("Timer not found, this is a bug")
@ -321,9 +333,9 @@ impl TreeEvent {
impl Visit for Visitor {
fn record_u64(&mut self, field: &Field, value: u64) {
if field.name() == "event_tag" {
if field.name() == "event_tag_id" {
let tag = EventTag::try_from(value).unwrap_or_else(|_| {
panic!("Invalid `event_tag`: {}, this is a bug", value)
panic!("Invalid `event_tag_id`: {}, this is a bug", value)
});
self.tag = Some(tag);
} else {
@ -342,6 +354,7 @@ impl TreeEvent {
fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
if field.name() == "message" {
use fmt::Write;
#[allow(clippy::expect_used)]
write!(self.message, "{:?}", value).expect("Write failed");
} else {
self.values.push((field.name(), format!("{:?}", value)));
@ -491,6 +504,7 @@ impl TreeProcessed {
pub fn operation_id() -> Option<Uuid> {
tracing::dispatcher::get_default(|dispatch| {
// Try to find the release subscriber
#[allow(clippy::expect_used)]
dispatch
.downcast_ref::<TreeSubscriber<ExportProcessor>>()
.map(TreeSubscriber::<ExportProcessor>::thread_operation_id)
@ -506,6 +520,7 @@ pub fn operation_id() -> Option<Uuid> {
pub fn main_init() -> JoinHandle<()> {
let (subscriber, logger) = TreeSubscriber::pretty();
#[allow(clippy::expect_used)]
tracing::subscriber::set_global_default(subscriber)
.expect("🚨🚨🚨 Global subscriber already set, this is a bug 🚨🚨🚨");
tokio::spawn(logger)