From e9cb71b9a7729cac2a259096502e21f3e368e290 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Fri, 27 Sep 2019 09:59:23 +1000 Subject: [PATCH] Add tooling for accounts to self-set their password (#107) Partially Implements #6 - add ability for accounts to self set password. This is good for now, as I get closer to a trial radius deployment, but I think I'm finding the rest api probably needs a better plan at this point, as well as probably the way we do the proto and the communication needs some more thoughts too. --- README.md | 2 + kanidm_client/src/lib.rs | 56 +++++++++++++++++++++++----- kanidm_client/tests/proto_v1_test.rs | 28 ++++++++++++++ kanidm_proto/src/v1.rs | 13 +++++++ kanidm_tools/src/main.rs | 20 ++++++++++ kanidmd/src/lib/actors/v1.rs | 51 ++++++++++++++++++++++++- kanidmd/src/lib/core.rs | 22 ++++++++++- kanidmd/src/lib/event.rs | 7 ++++ kanidmd/src/lib/idm/account.rs | 5 +++ kanidmd/src/lib/idm/authsession.rs | 3 +- kanidmd/src/lib/idm/event.rs | 22 +++++++++++ kanidmd/src/lib/idm/mod.rs | 2 +- kanidmd/src/lib/idm/server.rs | 33 +++++++++++++--- 13 files changed, 242 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index bbb4572b1..7ee8184a1 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ let's encrypt, but if this is not possible, please use our insecure cert tool: You can now build and run the server with: cd kanidm + cargo run -- recover_account -D /tmp/kanidm.db -n admin cargo run -- server -D /tmp/kanidm.db -C ../insecure/ca.pem -c ../insecure/cert.pem -k ../insecure/key.pem --domain localhost --bindaddr 127.0.0.1:8080 In a new terminal, you can now build and run the client tools with: @@ -62,6 +63,7 @@ In a new terminal, you can now build and run the client tools with: cd kanidm_tools cargo run -- --help cargo run -- whoami -H https://localhost:8080 -D anonymous -C ../insecure/ca.pem + cargo run -- whoami -H https://localhost:8080 -D admin -C ../insecure/ca.pem ## Development and Testing diff --git a/kanidm_client/src/lib.rs b/kanidm_client/src/lib.rs index f5101460f..2b41e4c50 100644 --- a/kanidm_client/src/lib.rs +++ b/kanidm_client/src/lib.rs @@ -12,7 +12,8 @@ use std::io::Read; use kanidm_proto::v1::{ AuthCredential, AuthRequest, AuthResponse, AuthState, AuthStep, CreateRequest, Entry, Filter, - OperationResponse, SearchRequest, SearchResponse, UserAuthToken, WhoamiResponse, + OperationResponse, SearchRequest, SearchResponse, SingleStringRequest, UserAuthToken, + WhoamiResponse, }; #[derive(Debug)] @@ -28,6 +29,7 @@ pub enum ClientError { pub struct KanidmClient { client: reqwest::Client, addr: String, + ca: Option, } impl KanidmClient { @@ -41,22 +43,32 @@ impl KanidmClient { reqwest::Certificate::from_pem(&buf).expect("Failed to parse ca") }); - let client_builder = reqwest::Client::builder().cookie_store(true); + let client = Self::build_reqwest(&ca).expect("Unexpected reqwest builder failure!"); - let client_builder = match ca { - Some(cert) => client_builder.add_root_certificate(cert), - None => client_builder, - }; - - let client = client_builder - .build() - .expect("Unexpected reqwest builder failure!"); KanidmClient { client: client, addr: addr.to_string(), + ca: ca, } } + pub fn logout(&mut self) -> Result<(), reqwest::Error> { + let mut r_client = Self::build_reqwest(&self.ca)?; + std::mem::swap(&mut self.client, &mut r_client); + Ok(()) + } + + fn build_reqwest(ca: &Option) -> Result { + let client_builder = reqwest::Client::builder().cookie_store(true); + + let client_builder = match ca { + Some(cert) => client_builder.add_root_certificate(cert.clone()), + None => client_builder, + }; + + client_builder.build() + } + fn auth_step_init(&self, ident: &str, appid: Option<&str>) -> Result { // TODO: Way to avoid formatting so much? let auth_dest = format!("{}/v1/auth", self.addr); @@ -238,4 +250,28 @@ impl KanidmClient { // modify // + + // === idm actions here == + pub fn idm_account_set_password(&self, cleartext: String) -> Result<(), ClientError> { + let s = SingleStringRequest { value: cleartext }; + + let dest = format!("{}/v1/idm/account/set_password", self.addr); + + let mut response = self + .client + .post(dest.as_str()) + .body(serde_json::to_string(&s).unwrap()) + .send() + .map_err(|e| ClientError::Transport(e))?; + + match response.status() { + reqwest::StatusCode::OK => {} + unexpect => return Err(ClientError::Http(unexpect)), + } + + // TODO: What about errors + let _r: OperationResponse = + serde_json::from_str(response.text().unwrap().as_str()).unwrap(); + Ok(()) + } } diff --git a/kanidm_client/tests/proto_v1_test.rs b/kanidm_client/tests/proto_v1_test.rs index 2786743aa..56e191e65 100644 --- a/kanidm_client/tests/proto_v1_test.rs +++ b/kanidm_client/tests/proto_v1_test.rs @@ -32,6 +32,7 @@ extern crate tokio; static PORT_ALLOC: AtomicUsize = AtomicUsize::new(8080); static ADMIN_TEST_PASSWORD: &'static str = "integration test admin password"; +static ADMIN_TEST_PASSWORD_CHANGE: &'static str = "integration test admin new🎉"; // Test external behaviorus of the service. @@ -172,4 +173,31 @@ fn test_server_search() { }); } +#[test] +fn test_server_admin_change_simple_password() { + run_test(|mut rsclient: KanidmClient| { + // First show we are un-authenticated. + let pre_res = rsclient.whoami(); + // This means it was okay whoami, but no uat attached. + assert!(pre_res.unwrap().is_none()); + + let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD); + assert!(res.is_ok()); + + // Now change the password. + let _ = rsclient + .idm_account_set_password(ADMIN_TEST_PASSWORD_CHANGE.to_string()) + .unwrap(); + + // Now "reset" the client. + let _ = rsclient.logout(); + // Old password fails + let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD); + assert!(res.is_err()); + // New password works! + let res = rsclient.auth_simple_password("admin", ADMIN_TEST_PASSWORD_CHANGE); + assert!(res.is_ok()); + }); +} + // Test hitting all auth-required endpoints and assert they give unauthorized. diff --git a/kanidm_proto/src/v1.rs b/kanidm_proto/src/v1.rs index 98caedd04..729ec1b2d 100644 --- a/kanidm_proto/src/v1.rs +++ b/kanidm_proto/src/v1.rs @@ -360,6 +360,19 @@ impl WhoamiResponse { } } +// Simple string value provision. +#[derive(Debug, Serialize, Deserialize)] +pub struct SingleStringRequest { + pub value: String, +} + +impl SingleStringRequest { + pub fn new(s: String) -> Self { + SingleStringRequest { value: s } + } +} +// Use OperationResponse here ... + #[cfg(test)] mod tests { use crate::v1::Filter as ProtoFilter; diff --git a/kanidm_tools/src/main.rs b/kanidm_tools/src/main.rs index 28c32b470..f10f08775 100644 --- a/kanidm_tools/src/main.rs +++ b/kanidm_tools/src/main.rs @@ -47,12 +47,20 @@ struct SearchOpt { commonopts: CommonOpt, } +#[derive(Debug, StructOpt)] +enum AccountOpt { + #[structopt(name = "set_password")] + SetPassword(CommonOpt), +} + #[derive(Debug, StructOpt)] enum ClientOpt { #[structopt(name = "search")] Search(SearchOpt), #[structopt(name = "whoami")] Whoami(CommonOpt), + #[structopt(name = "account")] + Account(AccountOpt), } impl ClientOpt { @@ -60,6 +68,9 @@ impl ClientOpt { match self { ClientOpt::Whoami(copt) => copt.debug, ClientOpt::Search(sopt) => sopt.commonopts.debug, + ClientOpt::Account(aopt) => match aopt { + AccountOpt::SetPassword(copt) => copt.debug, + }, } } } @@ -98,5 +109,14 @@ fn main() { println!("{:?}", e); } } + ClientOpt::Account(aopt) => match aopt { + AccountOpt::SetPassword(copt) => { + let client = copt.to_client(); + + let password = rpassword::prompt_password_stderr("Enter new password: ").unwrap(); + + client.idm_account_set_password(password).unwrap(); + } + }, } } diff --git a/kanidmd/src/lib/actors/v1.rs b/kanidmd/src/lib/actors/v1.rs index d9f9329ab..a1d39d02e 100644 --- a/kanidmd/src/lib/actors/v1.rs +++ b/kanidmd/src/lib/actors/v1.rs @@ -7,6 +7,7 @@ use crate::event::{ AuthEvent, CreateEvent, DeleteEvent, ModifyEvent, PurgeRecycledEvent, PurgeTombstoneEvent, SearchEvent, SearchResult, WhoamiResult, }; +use crate::idm::event::PasswordChangeEvent; use kanidm_proto::v1::OperationError; use crate::idm::server::IdmServer; @@ -14,7 +15,7 @@ use crate::server::{QueryServer, QueryServerTransaction}; use kanidm_proto::v1::{ AuthRequest, AuthResponse, CreateRequest, DeleteRequest, ModifyRequest, OperationResponse, - SearchRequest, SearchResponse, UserAuthToken, WhoamiResponse, + SearchRequest, SearchResponse, SingleStringRequest, UserAuthToken, WhoamiResponse, }; use actix::prelude::*; @@ -121,6 +122,26 @@ impl Message for SearchMessage { type Result = Result; } +pub struct IdmAccountSetPasswordMessage { + pub uat: Option, + pub cleartext: String, +} + +impl IdmAccountSetPasswordMessage { + pub fn new(uat: Option, req: SingleStringRequest) -> Self { + IdmAccountSetPasswordMessage { + uat: uat, + cleartext: req.value, + } + } +} + +impl Message for IdmAccountSetPasswordMessage { + type Result = Result; +} + +// =========================================================== + pub struct QueryServerV1 { log: actix::Addr, qs: QueryServer, @@ -381,6 +402,34 @@ impl Handler for QueryServerV1 { } } +impl Handler for QueryServerV1 { + type Result = Result; + + fn handle(&mut self, msg: IdmAccountSetPasswordMessage, _: &mut Self::Context) -> Self::Result { + let mut audit = AuditScope::new("idm_account_set_password"); + let res = audit_segment!(&mut audit, || { + let mut idms_prox_write = self.idms.proxy_write(); + + let pce = PasswordChangeEvent::from_idm_account_set_password( + &mut audit, + &idms_prox_write.qs_write, + msg, + ) + .map_err(|e| { + audit_log!(audit, "Failed to begin idm_account_set_password: {:?}", e); + e + })?; + + idms_prox_write + .set_account_password(&mut audit, &pce) + .and_then(|_| idms_prox_write.commit(&mut audit)) + .map(|_| OperationResponse::new(())) + }); + self.log.do_send(audit); + res + } +} + // These below are internal only types. impl Handler for QueryServerV1 { diff --git a/kanidmd/src/lib/core.rs b/kanidmd/src/lib/core.rs index 4e367bc18..af7364176 100644 --- a/kanidmd/src/lib/core.rs +++ b/kanidmd/src/lib/core.rs @@ -14,7 +14,8 @@ use crate::config::Configuration; // SearchResult use crate::actors::v1::QueryServerV1; use crate::actors::v1::{ - AuthMessage, CreateMessage, DeleteMessage, ModifyMessage, SearchMessage, WhoamiMessage, + AuthMessage, CreateMessage, DeleteMessage, IdmAccountSetPasswordMessage, ModifyMessage, + SearchMessage, WhoamiMessage, }; use crate::async_log; use crate::audit::AuditScope; @@ -29,7 +30,7 @@ use crate::utils::SID; use kanidm_proto::v1::OperationError; use kanidm_proto::v1::{ AuthRequest, AuthState, CreateRequest, DeleteRequest, ModifyRequest, SearchRequest, - UserAuthToken, + SingleStringRequest, UserAuthToken, }; use uuid::Uuid; @@ -262,6 +263,17 @@ fn auth( ) } +fn idm_account_set_password( + (req, state): (HttpRequest, State), +) -> impl Future { + json_event_post!( + req, + state, + IdmAccountSetPasswordMessage, + SingleStringRequest + ) +} + fn setup_backend(config: &Configuration) -> Result { let mut audit_be = AuditScope::new("backend_setup"); let pool_size: u32 = config.threads as u32; @@ -638,6 +650,12 @@ pub fn create_server_core(config: Configuration) { .resource("/v1/auth", |r| { r.method(http::Method::POST).with_async(auth) }) + // Start IDM resources. We'll probably add more restful types later. + .resource("/v1/idm/account/set_password", |r| { + r.method(http::Method::POST) + .with_async(idm_account_set_password) + }) + // Add an ldap compat search function type? /* .resource("/v1/list/{class_list}", |r| { diff --git a/kanidmd/src/lib/event.rs b/kanidmd/src/lib/event.rs index 7d2c59ce3..d25439558 100644 --- a/kanidmd/src/lib/event.rs +++ b/kanidmd/src/lib/event.rs @@ -195,6 +195,13 @@ impl Event { _ => false, } } + + pub fn get_uuid(&self) -> Option<&Uuid> { + match &self.origin { + EventOrigin::Internal => None, + EventOrigin::User(e) => Some(e.get_uuid()), + } + } } #[derive(Debug)] diff --git a/kanidmd/src/lib/idm/account.rs b/kanidmd/src/lib/idm/account.rs index 51a07dfda..4020101ee 100644 --- a/kanidmd/src/lib/idm/account.rs +++ b/kanidmd/src/lib/idm/account.rs @@ -3,6 +3,7 @@ use kanidm_proto::v1::OperationError; use kanidm_proto::v1::UserAuthToken; +use crate::constants::UUID_ANONYMOUS; use crate::credential::Credential; use crate::idm::claim::Claim; use crate::idm::group::Group; @@ -93,6 +94,10 @@ impl Account { }) } + pub fn is_anonymous(&self) -> bool { + self.uuid == *UUID_ANONYMOUS + } + pub(crate) fn gen_password_mod( &self, cleartext: &str, diff --git a/kanidmd/src/lib/idm/authsession.rs b/kanidmd/src/lib/idm/authsession.rs index 7c0a79f52..dcfab8ddc 100644 --- a/kanidmd/src/lib/idm/authsession.rs +++ b/kanidmd/src/lib/idm/authsession.rs @@ -1,5 +1,4 @@ use crate::audit::AuditScope; -use crate::constants::UUID_ANONYMOUS; use crate::idm::account::Account; use crate::idm::claim::Claim; use kanidm_proto::v1::OperationError; @@ -159,7 +158,7 @@ impl AuthSession { // We want the primary handler - this is where we make a decision // based on the anonymous ... in theory this could be cleaner // and interact with the account more? - if account.uuid == UUID_ANONYMOUS.clone() { + if account.is_anonymous() { CredHandler::Anonymous } else { // Now we see if they have one ... diff --git a/kanidmd/src/lib/idm/event.rs b/kanidmd/src/lib/idm/event.rs index 3f5d34ff9..97f835118 100644 --- a/kanidmd/src/lib/idm/event.rs +++ b/kanidmd/src/lib/idm/event.rs @@ -1,6 +1,12 @@ +use crate::actors::v1::IdmAccountSetPasswordMessage; +use crate::audit::AuditScope; use crate::event::Event; +use crate::server::QueryServerWriteTransaction; + use uuid::Uuid; +use kanidm_proto::v1::OperationError; + #[derive(Debug)] pub struct PasswordChangeEvent { pub event: Event, @@ -18,4 +24,20 @@ impl PasswordChangeEvent { appid: appid.map(|v| v.to_string()), } } + + pub fn from_idm_account_set_password( + audit: &mut AuditScope, + qs: &QueryServerWriteTransaction, + msg: IdmAccountSetPasswordMessage, + ) -> Result { + let e = Event::from_rw_uat(audit, qs, msg.uat)?; + let u = e.get_uuid().ok_or(OperationError::InvalidState)?.clone(); + + Ok(PasswordChangeEvent { + event: e, + target: u, + cleartext: msg.cleartext, + appid: None, + }) + } } diff --git a/kanidmd/src/lib/idm/mod.rs b/kanidmd/src/lib/idm/mod.rs index c8d726292..36151c302 100644 --- a/kanidmd/src/lib/idm/mod.rs +++ b/kanidmd/src/lib/idm/mod.rs @@ -1,10 +1,10 @@ #[macro_use] mod macros; -mod event; pub(crate) mod account; pub(crate) mod authsession; pub(crate) mod claim; +pub(crate) mod event; pub(crate) mod group; pub(crate) mod server; // mod identity; diff --git a/kanidmd/src/lib/idm/server.rs b/kanidmd/src/lib/idm/server.rs index 55444dc15..5f95aacb9 100644 --- a/kanidmd/src/lib/idm/server.rs +++ b/kanidmd/src/lib/idm/server.rs @@ -51,7 +51,7 @@ pub struct IdmServerReadTransaction<'a> { pub struct IdmServerProxyWriteTransaction<'a> { // This does NOT take any read to the memory content, allowing safe // qs operations to occur through this interface. - qs_write: QueryServerWriteTransaction<'a>, + pub qs_write: QueryServerWriteTransaction<'a>, } impl IdmServer { @@ -225,15 +225,25 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { au: &mut AuditScope, pce: &PasswordChangeEvent, ) -> Result<(), OperationError> { + // Get the account + let account_entry = try_audit!(au, self.qs_write.internal_search_uuid(au, &pce.target)); + let account = try_audit!(au, Account::try_from_entry(account_entry)); + // Ask if tis all good - this step checks pwpolicy and such + + // Deny the change if the account is anonymous! + if account.is_anonymous() { + return Err(OperationError::SystemProtectedObject); + } + // TODO: Is it a security issue to reveal pw policy checks BEFORE permission is // determined over the credential modification? // // I don't think so - because we should only be showing how STRONG the pw is ... - // Get the account - let account_entry = try_audit!(au, self.qs_write.internal_search_uuid(au, &pce.target)); - let account = try_audit!(au, Account::try_from_entry(account_entry)); - // Ask if tis all good - this step checks pwpolicy and such + // is the password long enough? + + // check a password badlist + // it returns a modify let modlist = try_audit!( au, @@ -281,7 +291,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { #[cfg(test)] mod tests { - use crate::constants::{AUTH_SESSION_TIMEOUT, UUID_ADMIN}; + use crate::constants::{AUTH_SESSION_TIMEOUT, UUID_ADMIN, UUID_ANONYMOUS}; use crate::credential::Credential; use crate::event::{AuthEvent, AuthResult, ModifyEvent}; use crate::idm::event::PasswordChangeEvent; @@ -551,6 +561,17 @@ mod tests { }) } + #[test] + fn test_idm_anonymous_set_password_denied() { + run_idm_test!(|_qs: &QueryServer, idms: &IdmServer, au: &mut AuditScope| { + let pce = PasswordChangeEvent::new_internal(&UUID_ANONYMOUS, TEST_PASSWORD, None); + + let mut idms_prox_write = idms.proxy_write(); + assert!(idms_prox_write.set_account_password(au, &pce).is_err()); + assert!(idms_prox_write.commit(au).is_ok()); + }) + } + #[test] fn test_idm_session_expire() { run_idm_test!(|qs: &QueryServer, idms: &IdmServer, au: &mut AuditScope| {