From aa2e872ae9b9191485b25b8fb70bea3d96314fba Mon Sep 17 00:00:00 2001 From: William Brown Date: Wed, 29 Apr 2020 14:36:57 +1000 Subject: [PATCH] Revert and fix cache change, it was an issue with sqlite --- kanidm_unix_int/src/constants.rs | 2 +- kanidm_unix_int/src/daemon.rs | 12 ++--- kanidm_unix_int/src/db.rs | 76 +++++++++++++++++++++----------- 3 files changed, 59 insertions(+), 31 deletions(-) diff --git a/kanidm_unix_int/src/constants.rs b/kanidm_unix_int/src/constants.rs index aa75805a4..d50c632a5 100644 --- a/kanidm_unix_int/src/constants.rs +++ b/kanidm_unix_int/src/constants.rs @@ -1,4 +1,4 @@ pub const DEFAULT_SOCK_PATH: &'static str = "/var/run/kanidm-unixd/sock"; -pub const DEFAULT_DB_PATH: &'static str = "/var/lib/kanidm-unixd/kanidm.cache.db"; +pub const DEFAULT_DB_PATH: &'static str = "/var/cache/kanidm-unixd/kanidm.cache.db"; pub const DEFAULT_CONN_TIMEOUT: u64 = 2; pub const DEFAULT_CACHE_TIMEOUT: u64 = 15; diff --git a/kanidm_unix_int/src/daemon.rs b/kanidm_unix_int/src/daemon.rs index b80d3e570..493f44cf2 100644 --- a/kanidm_unix_int/src/daemon.rs +++ b/kanidm_unix_int/src/daemon.rs @@ -241,11 +241,13 @@ async fn main() { match socket_res { Ok(socket) => { let cachelayer_ref = cachelayer.clone(); - tokio::spawn(async move { - if let Err(e) = handle_client(socket, cachelayer_ref.clone()).await { - error!("an error occured; error = {:?}", e); - } - }); + tokio::spawn( + async move { + if let Err(e) = handle_client(socket, cachelayer_ref.clone()).await { + error!("an error occured; error = {:?}", e); + } + }, + ); } Err(err) => { error!("Accept error -> {:?}", err); diff --git a/kanidm_unix_int/src/db.rs b/kanidm_unix_int/src/db.rs index f56892c41..8f6751225 100644 --- a/kanidm_unix_int/src/db.rs +++ b/kanidm_unix_int/src/db.rs @@ -313,8 +313,11 @@ impl<'a> DbTxn<'a> { () })?; - // This isn't needed because insert or replace into seems to clean up dups! - /* + // This is needed because sqlites 'insert or replace into', will null the password field + // if present, and upsert MUST match the exact conflicting column, so that means we have + // to manually manage the update or insert :( :( + + // Find anything conflicting and purge it. self.conn.execute_named("DELETE FROM account_t WHERE NOT uuid = :uuid AND (name = :name OR spn = :spn OR gidnumber = :gidnumber)", &[ (":uuid", &account.uuid), @@ -327,32 +330,50 @@ impl<'a> DbTxn<'a> { debug!("sqlite delete account_t duplicate failure -> {:?}", e); () }) - .map(|_| ()) - */ + .map(|_| ())?; - let mut stmt = self.conn - .prepare("INSERT OR REPLACE INTO account_t (uuid, name, spn, gidnumber, token, expiry) VALUES (:uuid, :name, :spn, :gidnumber, :token, :expiry)") + let updated = self.conn.execute_named( + "UPDATE account_t SET name=:name, spn=:spn, gidnumber=:gidnumber, token=:token, expiry=:expiry WHERE uuid = :uuid", + &[ + (":uuid", &account.uuid), + (":name", &account.name), + (":spn", &account.spn), + (":gidnumber", &account.gidnumber), + (":token", &data), + (":expiry", &expire), + ] + ) .map_err(|e| { - error!("sqlite prepare error -> {:?}", e); + debug!("sqlite delete account_t duplicate failure -> {:?}", e); + () + }) + .map(|c| c)?; + + if updated == 0 { + let mut stmt = self.conn + .prepare("INSERT INTO account_t (uuid, name, spn, gidnumber, token, expiry) VALUES (:uuid, :name, :spn, :gidnumber, :token, :expiry) ON CONFLICT(uuid) DO UPDATE SET name=excluded.name, spn=excluded.name, gidnumber=excluded.gidnumber, token=excluded.token, expiry=excluded.expiry") + .map_err(|e| { + error!("sqlite prepare error -> {:?}", e); + () + })?; + + stmt.execute_named(&[ + (":uuid", &account.uuid), + (":name", &account.name), + (":spn", &account.spn), + (":gidnumber", &account.gidnumber), + (":token", &data), + (":expiry", &expire), + ]) + .map(|r| { + debug!("insert -> {:?}", r); + () + }) + .map_err(|e| { + error!("sqlite execute_named error -> {:?}", e); () })?; - - stmt.execute_named(&[ - (":uuid", &account.uuid), - (":name", &account.name), - (":spn", &account.spn), - (":gidnumber", &account.gidnumber), - (":token", &data), - (":expiry", &expire), - ]) - .map(|r| { - debug!("insert -> {:?}", r); - () - }) - .map_err(|e| { - error!("sqlite execute_named error -> {:?}", e); - () - })?; + } // Now, we have to update the group memberships. @@ -944,7 +965,7 @@ mod tests { assert!(dbtxn.migrate().is_ok()); let uuid1 = "0302b99c-f0f6-41ab-9492-852692b0fd16"; - let ut1 = UnixUserToken { + let mut ut1 = UnixUserToken { name: "testuser".to_string(), spn: "testuser@example.com".to_string(), displayname: "Test User".to_string(), @@ -976,6 +997,11 @@ mod tests { assert!(dbtxn.check_account_password(uuid1, TESTACCOUNT1_PASSWORD_A) == Ok(false)); assert!(dbtxn.check_account_password(uuid1, TESTACCOUNT1_PASSWORD_B) == Ok(true)); + // Check that updating the account does not break the password. + ut1.displayname = "Test User Update".to_string(); + dbtxn.update_account(&ut1, 0).unwrap(); + assert!(dbtxn.check_account_password(uuid1, TESTACCOUNT1_PASSWORD_B) == Ok(true)); + assert!(dbtxn.commit().is_ok()); }