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.
This commit is contained in:
Firstyear 2019-09-27 09:59:23 +10:00 committed by GitHub
parent 879095c450
commit e9cb71b9a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 242 additions and 22 deletions

View file

@ -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

View file

@ -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<reqwest::Certificate>,
}
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<reqwest::Certificate>) -> Result<reqwest::Client, reqwest::Error> {
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<AuthState, ClientError> {
// 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(())
}
}

View file

@ -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.

View file

@ -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;

View file

@ -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();
}
},
}
}

View file

@ -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<SearchResponse, OperationError>;
}
pub struct IdmAccountSetPasswordMessage {
pub uat: Option<UserAuthToken>,
pub cleartext: String,
}
impl IdmAccountSetPasswordMessage {
pub fn new(uat: Option<UserAuthToken>, req: SingleStringRequest) -> Self {
IdmAccountSetPasswordMessage {
uat: uat,
cleartext: req.value,
}
}
}
impl Message for IdmAccountSetPasswordMessage {
type Result = Result<OperationResponse, OperationError>;
}
// ===========================================================
pub struct QueryServerV1 {
log: actix::Addr<EventLog>,
qs: QueryServer,
@ -381,6 +402,34 @@ impl Handler<WhoamiMessage> for QueryServerV1 {
}
}
impl Handler<IdmAccountSetPasswordMessage> for QueryServerV1 {
type Result = Result<OperationResponse, OperationError>;
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<PurgeTombstoneEvent> for QueryServerV1 {

View file

@ -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<AppState>, State<AppState>),
) -> impl Future<Item = HttpResponse, Error = Error> {
json_event_post!(
req,
state,
IdmAccountSetPasswordMessage,
SingleStringRequest
)
}
fn setup_backend(config: &Configuration) -> Result<Backend, OperationError> {
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| {

View file

@ -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)]

View file

@ -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,

View file

@ -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 ...

View file

@ -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<Self, OperationError> {
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,
})
}
}

View file

@ -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;

View file

@ -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| {