Sync account import improvements (#1873)

This commit is contained in:
Firstyear 2023-07-18 08:49:22 +10:00 committed by GitHub
parent 41aa315f23
commit 60a1cdf9d8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 321 additions and 70 deletions

8
Cargo.lock generated
View file

@ -2910,9 +2910,9 @@ dependencies = [
[[package]]
name = "ldap3_client"
version = "0.3.4"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "471c4b5d674dbfe41fd479ec0b9435fd765bc838347c5625d2d2f2229a36b104"
checksum = "7a229cd5ee2a4e5a1a279b6216494aa2a5053a189c5ce37bb31f9156b63b63de"
dependencies = [
"base64 0.13.1",
"base64urlsafedata",
@ -2930,9 +2930,9 @@ dependencies = [
[[package]]
name = "ldap3_proto"
version = "0.3.4"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2f4e16ad62e1881aa010c73dad1e693a208100f889512612cbe4fc717660cc31"
checksum = "93d073b5c98def43cec39ccd95e536b3b2448e36289222ecd64dfdf1389d1702"
dependencies = [
"bytes",
"lber",

View file

@ -88,8 +88,8 @@ kanidm_unix_int = { path = "./unix_integration" }
last-git-commit = "0.2.0"
# REMOVE this
lazy_static = "^1.4.0"
ldap3_client = "^0.3.4"
ldap3_proto = { version = "^0.3.4", features = ["serde"] }
ldap3_client = "^0.3.5"
ldap3_proto = { version = "^0.3.5", features = ["serde"] }
# ldap3_client = { path = "../ldap3/client", version = "0.3.2" }
# ldap3_proto = { path = "../ldap3/proto", version = "^0.3.2", features = ["serde"] }

View file

@ -95,6 +95,26 @@ impl Into<ScimComplexAttr> for ScimTotp {
}
}
#[derive(Serialize, Debug, Clone)]
pub struct ScimSshPubKey {
pub label: String,
pub value: String,
}
#[allow(clippy::from_over_into)]
impl Into<ScimComplexAttr> for ScimSshPubKey {
fn into(self) -> ScimComplexAttr {
let ScimSshPubKey { label, value } = self;
let mut attrs = BTreeMap::default();
attrs.insert("label".to_string(), ScimSimpleAttr::String(label));
attrs.insert("value".to_string(), ScimSimpleAttr::String(value));
ScimComplexAttr { attrs }
}
}
#[derive(Serialize, Debug, Clone)]
#[serde(into = "ScimEntry")]
pub struct ScimSyncPerson {
@ -107,6 +127,7 @@ pub struct ScimSyncPerson {
pub totp_import: Vec<ScimTotp>,
pub login_shell: Option<String>,
pub mail: Vec<MultiValueAttr>,
pub ssh_publickey: Vec<ScimSshPubKey>,
}
// Need to allow this because clippy is broken and doesn't realise scimentry is out of crate
@ -124,6 +145,7 @@ impl Into<ScimEntry> for ScimSyncPerson {
totp_import,
login_shell,
mail,
ssh_publickey,
} = self;
let schemas = if gidnumber.is_some() {
@ -148,6 +170,7 @@ impl Into<ScimEntry> for ScimSyncPerson {
set_multi_complex!(attrs, "totp_import", totp_import);
set_option_string!(attrs, "loginshell", login_shell);
set_multi_complex!(attrs, "mail", mail);
set_multi_complex!(attrs, "ssh_publickey", ssh_publickey);
ScimEntry {
schemas,

View file

@ -496,6 +496,37 @@ impl Credential {
.map(|pw| self.update_password(pw))
}
pub fn upgrade_password(
&self,
policy: &CryptoPolicy,
cleartext: &str,
) -> Result<Option<Self>, OperationError> {
let valid = self.password_ref().and_then(|pw| {
pw.verify(cleartext).map_err(|e| {
error!(crypto_err = ?e);
e.into()
})
})?;
if valid {
let pw = Password::new(policy, cleartext).map_err(|e| {
error!(crypto_err = ?e);
e.into()
})?;
// Note, during update_password we normally rotate the uuid, here we
// set it back to our current value. This is because we are just
// updating the hash value, not actually changing the password itself.
let mut cred = self.update_password(pw);
cred.uuid = self.uuid;
Ok(Some(cred))
} else {
// No updates needed, password has changed.
Ok(None)
}
}
/// Extend this credential with another alternate webauthn credential. This is especially
/// useful for `PasswordMfa` where you can have many webauthn credentials and a password
/// generally so that one is a backup.

View file

@ -380,6 +380,7 @@ impl Account {
self.uuid == UUID_ANONYMOUS
}
#[cfg(test)]
pub(crate) fn gen_password_mod(
&self,
cleartext: &str,
@ -401,6 +402,30 @@ impl Account {
}
}
pub(crate) fn gen_password_upgrade_mod(
&self,
cleartext: &str,
crypto_policy: &CryptoPolicy,
) -> Result<Option<ModifyList<ModifyInvalid>>, OperationError> {
match &self.primary {
// Change the cred
Some(primary) => {
if let Some(ncred) = primary.upgrade_password(crypto_policy, cleartext)? {
let vcred = Value::new_credential("primary", ncred);
Ok(Some(ModifyList::new_purge_and_set(
"primary_credential",
vcred,
)))
} else {
// No action, not the same pw
Ok(None)
}
}
// Nothing to do.
None => Ok(None),
}
}
pub(crate) fn gen_webauthn_counter_mod(
&mut self,
auth_result: &AuthenticationResult,
@ -463,20 +488,6 @@ impl Account {
}
}
pub(crate) fn check_credential_pw(&self, cleartext: &str) -> Result<bool, OperationError> {
self.primary
.as_ref()
.ok_or(OperationError::InvalidState)
.and_then(|cred| {
cred.password_ref().and_then(|pw| {
pw.verify(cleartext).map_err(|e| {
error!(crypto_err = ?e);
e.into()
})
})
})
}
pub(crate) fn regenerate_radius_secret_mod(
&self,
cleartext: &str,

View file

@ -1043,6 +1043,51 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
}
Ok(vs)
}
(SyntaxType::SshKey, true, ScimAttr::MultiComplex(values)) => {
let mut vs = Vec::with_capacity(values.len());
for complex in values.iter() {
let label = complex
.attrs
.get("label")
.ok_or_else(|| {
error!("Invalid scim complex attr - missing required key label");
OperationError::InvalidAttribute(format!(
"missing required key label - {scim_attr_name}"
))
})
.and_then(|external_id| match external_id {
ScimSimpleAttr::String(value) => Ok(value.clone()),
_ => {
error!("Invalid value attribute - must be scim simple string");
Err(OperationError::InvalidAttribute(format!(
"value must be scim simple string - {scim_attr_name}"
)))
}
})?;
let value = complex
.attrs
.get("value")
.ok_or_else(|| {
error!("Invalid scim complex attr - missing required key value");
OperationError::InvalidAttribute(format!(
"missing required key value - {scim_attr_name}"
))
})
.and_then(|external_id| match external_id {
ScimSimpleAttr::String(value) => Ok(value.clone()),
_ => {
error!("Invalid value attribute - must be scim simple string");
Err(OperationError::InvalidAttribute(format!(
"value must be scim simple string - {scim_attr_name}"
)))
}
})?;
vs.push(Value::SshKey(label, value))
}
Ok(vs)
}
(syn, mv, sa) => {
error!(?syn, ?mv, ?sa, "Unsupported scim attribute conversion. This may be a syntax error in your import, or a missing feature in Kanidm.");
Err(OperationError::InvalidAttribute(format!(
@ -2539,6 +2584,12 @@ mod tests {
assert!(testuser.get_ava_single_utf8("displayname") == Some("Test User"));
assert!(testuser.get_ava_single_iutf8("loginshell") == Some("/bin/sh"));
let mut ssh_keyiter = testuser
.get_ava_iter_sshpubkeys("ssh_publickey")
.expect("Failed to access ssh pubkeys");
assert_eq!(ssh_keyiter.next(), Some("sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBENubZikrb8hu+HeVRdZ0pp/VAk2qv4JDbuJhvD0yNdWDL2e3cBbERiDeNPkWx58Q4rVnxkbV1fa8E2waRtT91wAAAAEc3NoOg== testuser@fidokey"));
assert_eq!(ssh_keyiter.next(), None);
// Check memberof works.
let testgroup_mb = testgroup.get_ava_refer("member").expect("No members!");
assert!(testgroup_mb.contains(&testuser.get_uuid()));
@ -2948,6 +2999,12 @@ mod tests {
"value": "testuser@dev.blackhats.net.au"
}
],
"ssh_publickey": [
{
"label": "ssh-key",
"value": "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBENubZikrb8hu+HeVRdZ0pp/VAk2qv4JDbuJhvD0yNdWDL2e3cBbERiDeNPkWx58Q4rVnxkbV1fa8E2waRtT91wAAAAEc3NoOg== testuser@fidokey"
}
],
"password_import": "ipaNTHash: iEb36u6PsRetBr3YMLdYbA"
},
{

View file

@ -1824,18 +1824,14 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
info!(session_id = %pwu.target_uuid, "Processing password hash upgrade");
// check, does the pw still match?
let same = account.check_credential_pw(pwu.existing_password.as_str())?;
// if yes, gen the pw mod and apply.
if same {
let modlist = account
.gen_password_mod(pwu.existing_password.as_str(), self.crypto_policy)
.map_err(|e| {
admin_error!("Unable to generate password mod {:?}", e);
e
})?;
let maybe_modlist = account
.gen_password_upgrade_mod(pwu.existing_password.as_str(), self.crypto_policy)
.map_err(|e| {
admin_error!("Unable to generate password mod {:?}", e);
e
})?;
if let Some(modlist) = maybe_modlist {
self.qs_write.internal_modify(
&filter_all!(f_eq("uuid", PartialValue::Uuid(pwu.target_uuid))),
&modlist,
@ -1860,21 +1856,16 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
e
})?;
let same = account.check_existing_pw(pwu.existing_password.as_str())?;
if same {
let modlist = account
.gen_password_mod(pwu.existing_password.as_str(), self.crypto_policy)
.map_err(|e| {
admin_error!("Unable to generate password mod {:?}", e);
e
})?;
let maybe_modlist =
account.gen_password_upgrade_mod(pwu.existing_password.as_str(), self.crypto_policy)?;
if let Some(modlist) = maybe_modlist {
self.qs_write.internal_modify(
&filter_all!(f_eq("uuid", PartialValue::Uuid(pwu.target_uuid))),
&modlist,
)
} else {
// No action needed, it's probably been changed/updated already.
Ok(())
}
}
@ -2772,9 +2763,28 @@ mod tests {
}
// Still empty
idms_delayed.check_is_empty_or_panic();
let mut idms_prox_read = idms.proxy_read().await;
let admin_entry = idms_prox_read
.qs_read
.internal_search_uuid(UUID_ADMIN)
.expect("Can't access admin entry.");
let cred_before = admin_entry
.get_ava_single_credential("primary_credential")
.expect("No credential present")
.clone();
drop(idms_prox_read);
// Do an auth, this will trigger the action to send.
check_admin_password(idms, "password").await;
// ⚠️ We have to be careful here. Between these two actions, it's possible
// that on the pw upgrade that the credential uuid changes. This immediately
// causes the session to be invalidated.
// We need to check the credential id does not change between these steps to
// prevent this!
// process it.
let da = idms_delayed.try_recv().expect("invalid");
// The first task is the pw upgrade
@ -2785,6 +2795,19 @@ mod tests {
assert!(matches!(da, DelayedAction::AuthSessionRecord(_)));
assert!(Ok(true) == r);
let mut idms_prox_read = idms.proxy_read().await;
let admin_entry = idms_prox_read
.qs_read
.internal_search_uuid(UUID_ADMIN)
.expect("Can't access admin entry.");
let cred_after = admin_entry
.get_ava_single_credential("primary_credential")
.expect("No credential present")
.clone();
drop(idms_prox_read);
assert_eq!(cred_before.uuid, cred_after.uuid);
// Check the admin pw still matches
check_admin_password(idms, "password").await;
// Clear the next auth session record

View file

@ -178,6 +178,27 @@ impl UnixUserAccount {
Ok(ModifyList::new_purge_and_set("unix_password", vcred))
}
pub(crate) fn gen_password_upgrade_mod(
&self,
cleartext: &str,
crypto_policy: &CryptoPolicy,
) -> Result<Option<ModifyList<ModifyInvalid>>, OperationError> {
match &self.cred {
// Change the cred
Some(ucred) => {
if let Some(ncred) = ucred.upgrade_password(crypto_policy, cleartext)? {
let vcred = Value::new_credential("primary", ncred);
Ok(Some(ModifyList::new_purge_and_set("unix_password", vcred)))
} else {
// No action, not the same pw
Ok(None)
}
}
// Nothing to do.
None => Ok(None),
}
}
pub fn is_within_valid_time(&self, ct: Duration) -> bool {
let cot = OffsetDateTime::UNIX_EPOCH + ct;
@ -271,18 +292,6 @@ impl UnixUserAccount {
}
}
}
pub(crate) fn check_existing_pw(&self, cleartext: &str) -> Result<bool, OperationError> {
match &self.cred {
Some(cred) => cred.password_ref().and_then(|pw| {
pw.verify(cleartext).map_err(|e| {
error!(crypto_err = ?e);
e.into()
})
}),
None => Err(OperationError::InvalidState),
}
}
}
#[derive(Debug, Clone)]

View file

@ -234,7 +234,7 @@ function addBorrowedObject(obj) {
}
function __wbg_adapter_48(arg0, arg1, arg2) {
try {
wasm._dyn_core__ops__function__FnMut___A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h91fc06edd1d61942(arg0, arg1, addBorrowedObject(arg2));
wasm._dyn_core__ops__function__FnMut___A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h779328ae10e6b850(arg0, arg1, addBorrowedObject(arg2));
} finally {
heap[stack_pointer++] = undefined;
}
@ -242,14 +242,14 @@ function __wbg_adapter_48(arg0, arg1, arg2) {
function __wbg_adapter_51(arg0, arg1, arg2) {
try {
wasm._dyn_core__ops__function__FnMut___A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h69b5a7e71157cf78(arg0, arg1, addBorrowedObject(arg2));
wasm._dyn_core__ops__function__FnMut___A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h0b311056ce39bf03(arg0, arg1, addBorrowedObject(arg2));
} finally {
heap[stack_pointer++] = undefined;
}
}
function __wbg_adapter_54(arg0, arg1, arg2) {
wasm._dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h2d5c8ecfb4968ae0(arg0, arg1, addHeapObject(arg2));
wasm._dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__he3ad0511b1590f2c(arg0, arg1, addHeapObject(arg2));
}
/**
@ -1121,16 +1121,16 @@ function __wbg_get_imports() {
const ret = wasm.memory;
return addHeapObject(ret);
};
imports.wbg.__wbindgen_closure_wrapper2571 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1196, __wbg_adapter_48);
imports.wbg.__wbindgen_closure_wrapper2556 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1194, __wbg_adapter_48);
return addHeapObject(ret);
};
imports.wbg.__wbindgen_closure_wrapper3389 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1499, __wbg_adapter_51);
imports.wbg.__wbindgen_closure_wrapper3373 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1496, __wbg_adapter_51);
return addHeapObject(ret);
};
imports.wbg.__wbindgen_closure_wrapper4500 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1574, __wbg_adapter_54);
imports.wbg.__wbindgen_closure_wrapper4484 = function(arg0, arg1, arg2) {
const ret = makeMutClosure(arg0, arg1, 1571, __wbg_adapter_54);
return addHeapObject(ret);
};

View file

@ -358,6 +358,11 @@ impl ViewsApp {
async fn fetch_logout() -> Result<ViewsMsg, FetchError> {
let (kopid, status, value, _) = do_request("/v1/logout", RequestMethod::GET, None).await?;
// In both cases - clear the local token to prevent our client
// thinking we have auth.
models::clear_bearer_token();
if status == 200 {
Ok(ViewsMsg::LogoutComplete)
} else {

View file

@ -49,8 +49,8 @@ use uuid::Uuid;
use kanidm_client::KanidmClientBuilder;
use kanidm_proto::scim_v1::{
MultiValueAttr, ScimEntry, ScimExternalMember, ScimSyncGroup, ScimSyncPerson, ScimSyncRequest,
ScimSyncRetentionMode, ScimSyncState, ScimTotp,
MultiValueAttr, ScimEntry, ScimExternalMember, ScimSshPubKey, ScimSyncGroup, ScimSyncPerson,
ScimSyncRequest, ScimSyncRetentionMode, ScimSyncState, ScimTotp,
};
use kanidmd_lib::utils::file_permissions_readonly;
@ -436,7 +436,8 @@ async fn run_sync(
// process the entries to scim.
let entries = match process_ipa_sync_result(
ipa_client,
&mut ipa_client,
sync_config.ipa_sync_base_dn.clone(),
entries,
&sync_config.entry_map,
is_initialise,
@ -500,7 +501,8 @@ async fn run_sync(
}
async fn process_ipa_sync_result(
_ipa_client: LdapClient,
ipa_client: &mut LdapClient,
basedn: String,
ldap_entries: Vec<LdapSyncReplEntry>,
entry_config_map: &HashMap<Uuid, EntryConfig>,
is_initialise: bool,
@ -644,9 +646,65 @@ async fn process_ipa_sync_result(
debug!(?filter);
// Search
// Inject all new entries to our maps. At this point we discard the original content
// of the totp entries since we just fetched them all again anyway.
// Search - we use syncrepl here and discard the cookie because we need the
// entry uuid to be given from the nsuniqueid else we have issues.
let mode = proto::SyncRequestMode::RefreshOnly;
match ipa_client.syncrepl(basedn, filter, None, mode).await {
Ok(LdapSyncRepl::Success {
cookie: _,
refresh_deletes: _,
entries: sync_entries,
delete_uuids: _,
present_uuids: _,
}) => {
// Inject all new entries to our maps. At this point we discard the original content
// of the totp entries since we just fetched them all again anyway.
totp_entries.clear();
for lentry in sync_entries.into_iter() {
if lentry
.entry
.attrs
.get("objectclass")
.map(|oc| oc.contains("ipatokentotp"))
.unwrap_or_default()
{
let token_owner_dn = if let Some(todn) = lentry
.entry
.attrs
.get("ipatokenowner")
.and_then(|attr| if attr.len() != 1 { None } else { attr.first() })
{
debug!("totp with owner {}", todn);
todn.clone()
} else {
warn!("totp with invalid ownership will be ignored");
continue;
};
if !totp_entries.contains_key(&token_owner_dn) {
totp_entries.insert(token_owner_dn.clone(), Vec::default());
}
if let Some(v) = totp_entries.get_mut(&token_owner_dn) {
v.push(lentry)
}
} else {
let dn = lentry.entry.dn.clone();
entries.insert(dn, lentry);
}
}
}
Ok(LdapSyncRepl::RefreshRequired) => {
error!("Failed due to invalid search state from ipa");
return Err(());
}
Err(e) => {
error!(?e, "Failed to perform search from ipa");
return Err(());
}
}
}
// For each updated TOTP -> If it's related DN is not in Hash -> remove from map
@ -789,6 +847,19 @@ fn ipa_to_scim_entry(
Vec::default()
};
let ssh_publickey = entry
.remove_ava("ipasshpubkey")
.map(|set| {
set.into_iter()
.enumerate()
.map(|(i, value)| ScimSshPubKey {
label: format!("ipasshpubkey-{}", i),
value,
})
.collect()
})
.unwrap_or_default();
let login_shell = entry.remove_ava_single("loginshell");
let external_id = Some(entry.dn);
@ -803,6 +874,7 @@ fn ipa_to_scim_entry(
totp_import,
login_shell,
mail,
ssh_publickey,
}
.into(),
))

View file

@ -33,6 +33,10 @@ fn person_attr_mail() -> String {
"mail".to_string()
}
fn person_attr_ssh_public_key() -> String {
"sshpublickey".to_string()
}
fn group_objectclass() -> String {
"groupofnames".to_string()
}
@ -81,6 +85,8 @@ pub struct Config {
pub person_attr_login_shell: String,
#[serde(default = "person_attr_mail")]
pub person_attr_mail: String,
#[serde(default = "person_attr_ssh_public_key")]
pub person_attr_ssh_public_key: String,
#[serde(default = "group_objectclass")]
pub group_objectclass: String,

View file

@ -43,8 +43,8 @@ use tracing_subscriber::{fmt, EnvFilter};
use kanidm_client::KanidmClientBuilder;
use kanidm_proto::scim_v1::{
MultiValueAttr, ScimEntry, ScimExternalMember, ScimSyncGroup, ScimSyncPerson, ScimSyncRequest,
ScimSyncRetentionMode, ScimSyncState,
MultiValueAttr, ScimEntry, ScimExternalMember, ScimSshPubKey, ScimSyncGroup, ScimSyncPerson,
ScimSyncRequest, ScimSyncRetentionMode, ScimSyncState,
};
use kanidmd_lib::utils::file_permissions_readonly;
@ -552,6 +552,19 @@ fn ldap_to_scim_entry(
let totp_import = Vec::default();
let ssh_publickey = entry
.remove_ava(&sync_config.person_attr_ssh_public_key)
.map(|set| {
set.into_iter()
.enumerate()
.map(|(i, value)| ScimSshPubKey {
label: format!("sshpublickey-{}", i),
value,
})
.collect()
})
.unwrap_or_default();
let login_shell = entry.remove_ava_single(&sync_config.person_attr_login_shell);
let external_id = Some(entry.dn);
@ -566,6 +579,7 @@ fn ldap_to_scim_entry(
totp_import,
login_shell,
mail,
ssh_publickey,
}
.into(),
))