From fd4c43d585acf43043bbb833a4f4863433aea644 Mon Sep 17 00:00:00 2001 From: James Magahern Date: Thu, 1 May 2025 01:02:36 -0700 Subject: [PATCH] client: actually do authentication properly --- kordophone-db/src/database.rs | 19 -------- kordophone/src/api/http_client.rs | 45 +++++++++++-------- kordophone/src/api/mod.rs | 25 +++++++---- kordophoned/src/daemon/mod.rs | 68 +++++++++++++++++++---------- kordophoned/src/daemon/settings.rs | 10 ++++- kordophoned/src/dbus/server_impl.rs | 4 ++ kpcli/src/client/mod.rs | 8 ++-- 7 files changed, 105 insertions(+), 74 deletions(-) diff --git a/kordophone-db/src/database.rs b/kordophone-db/src/database.rs index c8d86ac..d7eb7d3 100644 --- a/kordophone-db/src/database.rs +++ b/kordophone-db/src/database.rs @@ -8,9 +8,6 @@ pub use tokio::sync::Mutex; use crate::repository::Repository; use crate::settings::Settings; -pub use kordophone::api::TokenStore; -use kordophone::model::JwtToken; - use diesel_migrations::{embed_migrations, EmbeddedMigrations, MigrationHarness}; pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!(); @@ -86,19 +83,3 @@ impl DatabaseAccess for Arc> { database.with_settings(f).await } } - -static TOKEN_KEY: &str = "token"; - -#[async_trait] -impl TokenStore for Database { - async fn get_token(&mut self) -> Option { - self.with_settings(|settings| { - let token: Result> = settings.get(TOKEN_KEY); - token.unwrap_or_default() - }).await - } - - async fn set_token(&mut self, token: JwtToken) { - self.with_settings(|settings| settings.put(TOKEN_KEY, &token).unwrap()).await; - } -} diff --git a/kordophone/src/api/http_client.rs b/kordophone/src/api/http_client.rs index 2cada8b..86c61de 100644 --- a/kordophone/src/api/http_client.rs +++ b/kordophone/src/api/http_client.rs @@ -3,7 +3,7 @@ extern crate serde; use std::{path::PathBuf, str}; -use crate::api::TokenStore; +use crate::api::AuthenticationStore; use hyper::{Body, Client, Method, Request, Uri}; use async_trait::async_trait; @@ -16,10 +16,9 @@ use crate::{ type HttpClient = Client; -pub struct HTTPAPIClient { +pub struct HTTPAPIClient { pub base_url: Uri, - pub token_store: K, - credentials: Option, + pub auth_store: K, client: HttpClient, } @@ -92,7 +91,7 @@ impl AuthSetting for hyper::http::Request { } #[async_trait] -impl APIInterface for HTTPAPIClient { +impl APIInterface for HTTPAPIClient { type Error = Error; async fn get_version(&mut self) -> Result { @@ -111,10 +110,15 @@ impl APIInterface for HTTPAPIClient { jwt: String, } + log::debug!("Authenticating with username: {:?}", credentials.username); + let body = || -> Body { serde_json::to_string(&credentials).unwrap().into() }; let token: AuthResponse = self.request_with_body_retry("authenticate", Method::POST, body, false).await?; let token = JwtToken::new(&token.jwt).map_err(|_| Error::DecodeError)?; - self.token_store.set_token(token.clone()).await; + + log::debug!("Saving token: {:?}", token); + self.auth_store.set_token(token.clone()).await; + Ok(token) } @@ -144,12 +148,11 @@ impl APIInterface for HTTPAPIClient { } } -impl HTTPAPIClient { - pub fn new(base_url: Uri, credentials: Option, token_store: K) -> HTTPAPIClient { +impl HTTPAPIClient { + pub fn new(base_url: Uri, auth_store: K) -> HTTPAPIClient { HTTPAPIClient { base_url, - credentials, - token_store, + auth_store, client: Client::new(), } } @@ -198,7 +201,7 @@ impl HTTPAPIClient { .expect("Unable to build request") }; - let token = self.token_store.get_token().await; + let token = self.auth_store.get_token().await; let request = build_request(&token); let mut response = self.client.request(request).await?; @@ -213,11 +216,14 @@ impl HTTPAPIClient { return Err(Error::ClientError("Unauthorized".into())); } - if let Some(credentials) = &self.credentials { - self.authenticate(credentials.clone()).await?; + if let Some(credentials) = &self.auth_store.get_credentials().await { + log::debug!("Renewing token using credentials: u: {:?}", credentials.username); + let new_token = self.authenticate(credentials.clone()).await?; - let request = build_request(&token); + let request = build_request(&Some(new_token)); response = self.client.request(request).await?; + } else { + return Err(Error::ClientError("Unauthorized, no credentials provided".into())); } }, @@ -233,6 +239,9 @@ impl HTTPAPIClient { let parsed: T = match serde_json::from_slice(&body) { Ok(result) => Ok(result), Err(json_err) => { + log::error!("Error deserializing JSON: {:?}", json_err); + log::error!("Body: {:?}", String::from_utf8_lossy(&body)); + // If JSON deserialization fails, try to interpret it as plain text // Unfortunately the server does return things like this... let s = str::from_utf8(&body).map_err(|_| Error::DecodeError)?; @@ -247,17 +256,17 @@ impl HTTPAPIClient { #[cfg(test)] mod test { use super::*; - use crate::api::InMemoryTokenStore; + use crate::api::InMemoryAuthenticationStore; #[cfg(test)] - fn local_mock_client() -> HTTPAPIClient { + fn local_mock_client() -> HTTPAPIClient { let base_url = "http://localhost:5738".parse().unwrap(); - let credentials = Credentials { + let credentials = Credentials { username: "test".to_string(), password: "test".to_string(), }; - HTTPAPIClient::new(base_url, credentials.into(), InMemoryTokenStore::new()) + HTTPAPIClient::new(base_url, InMemoryAuthenticationStore::new(Some(credentials))) } #[cfg(test)] diff --git a/kordophone/src/api/mod.rs b/kordophone/src/api/mod.rs index da9090a..159a657 100644 --- a/kordophone/src/api/mod.rs +++ b/kordophone/src/api/mod.rs @@ -33,29 +33,38 @@ pub trait APIInterface { } #[async_trait] -pub trait TokenStore { +pub trait AuthenticationStore { + async fn get_credentials(&mut self) -> Option; async fn get_token(&mut self) -> Option; async fn set_token(&mut self, token: JwtToken); } -pub struct InMemoryTokenStore { +pub struct InMemoryAuthenticationStore { + credentials: Option, token: Option, } -impl Default for InMemoryTokenStore { +impl Default for InMemoryAuthenticationStore { fn default() -> Self { - Self::new() + Self::new(None) } } -impl InMemoryTokenStore { - pub fn new() -> Self { - Self { token: None } +impl InMemoryAuthenticationStore { + pub fn new(credentials: Option) -> Self { + Self { + credentials, + token: None, + } } } #[async_trait] -impl TokenStore for InMemoryTokenStore { +impl AuthenticationStore for InMemoryAuthenticationStore { + async fn get_credentials(&mut self) -> Option { + self.credentials.clone() + } + async fn get_token(&mut self) -> Option { self.token.clone() } diff --git a/kordophoned/src/daemon/mod.rs b/kordophoned/src/daemon/mod.rs index 9cf182a..4528f40 100644 --- a/kordophoned/src/daemon/mod.rs +++ b/kordophoned/src/daemon/mod.rs @@ -1,5 +1,6 @@ pub mod settings; use settings::Settings; +use settings::keys as SettingsKey; pub mod events; use events::*; @@ -26,7 +27,7 @@ use kordophone::model::JwtToken; use kordophone::api::{ http_client::{Credentials, HTTPAPIClient}, APIInterface, - TokenStore, + AuthenticationStore, }; #[derive(Debug, Error)] @@ -37,18 +38,52 @@ pub enum DaemonError { pub type DaemonResult = Result>; -struct DatabaseTokenStore { +struct DatabaseAuthenticationStore { database: Arc>, } #[async_trait] -impl TokenStore for DatabaseTokenStore { +impl AuthenticationStore for DatabaseAuthenticationStore { + async fn get_credentials(&mut self) -> Option { + self.database.lock().await.with_settings(|settings| { + let username: Option = settings.get::(SettingsKey::USERNAME) + .unwrap_or_else(|e| { + log::warn!("error getting username from database: {}", e); + None + }); + + // TODO: This would be the point where we map from credential item to password. + let password: String = settings.get::(SettingsKey::CREDENTIAL_ITEM) + .unwrap_or_else(|e| { + log::warn!("error getting password from database: {}", e); + None + }) + .unwrap_or_else(|| { + log::warn!("warning: no password in database, [DEBUG] using default password"); + "test".to_string() + }); + + if username.is_none() { + log::warn!("Username not present in database"); + } + + match (username, password) { + (Some(username), password) => Some(Credentials { username, password }), + _ => None, + } + }).await + } + async fn get_token(&mut self) -> Option { - self.database.lock().await.get_token().await + self.database.lock().await + .with_settings(|settings| settings.get::(SettingsKey::TOKEN).unwrap_or_default()).await } async fn set_token(&mut self, token: JwtToken) { - self.database.lock().await.set_token(token).await; + self.database.lock().await + .with_settings(|settings| settings.put(SettingsKey::TOKEN, &token)).await.unwrap_or_else(|e| { + log::error!("Failed to set token: {}", e); + }); } } @@ -252,9 +287,7 @@ impl Daemon { } async fn get_settings(&mut self) -> Result { - let settings = self.database.with_settings(Settings::from_db - ).await?; - + let settings = self.database.with_settings(Settings::from_db).await?; Ok(settings) } @@ -262,30 +295,19 @@ impl Daemon { self.database.with_settings(|s| settings.save(s)).await } - async fn get_client(&mut self) -> Result> { + async fn get_client(&mut self) -> Result> { Self::get_client_impl(&mut self.database).await } - async fn get_client_impl(database: &mut Arc>) -> Result> { - let settings = database.with_settings(Settings::from_db - ).await?; + async fn get_client_impl(database: &mut Arc>) -> Result> { + let settings = database.with_settings(Settings::from_db).await?; let server_url = settings.server_url .ok_or(DaemonError::ClientNotConfigured)?; let client = HTTPAPIClient::new( server_url.parse().unwrap(), - - match (settings.username, settings.credential_item) { - (Some(username), Some(password)) => Some( - Credentials { - username, - password, - } - ), - _ => None, - }, - DatabaseTokenStore { database: database.clone() } + DatabaseAuthenticationStore { database: database.clone() } ); Ok(client) diff --git a/kordophoned/src/daemon/settings.rs b/kordophoned/src/daemon/settings.rs index 5a3d202..18e9482 100644 --- a/kordophoned/src/daemon/settings.rs +++ b/kordophoned/src/daemon/settings.rs @@ -1,10 +1,11 @@ use kordophone_db::settings::Settings as DbSettings; use anyhow::Result; -mod keys { +pub mod keys { pub static SERVER_URL: &str = "ServerURL"; pub static USERNAME: &str = "Username"; pub static CREDENTIAL_ITEM: &str = "CredentialItem"; + pub static TOKEN: &str = "Token"; } #[derive(Debug)] @@ -13,6 +14,7 @@ pub struct Settings { pub server_url: Option, pub username: Option, pub credential_item: Option, + pub token: Option, } impl Settings { @@ -20,11 +22,12 @@ impl Settings { let server_url: Option = db_settings.get(keys::SERVER_URL)?; let username: Option = db_settings.get(keys::USERNAME)?; let credential_item: Option = db_settings.get(keys::CREDENTIAL_ITEM)?; - + let token: Option = db_settings.get(keys::TOKEN)?; Ok(Self { server_url, username, credential_item, + token, }) } @@ -38,6 +41,9 @@ impl Settings { if let Some(credential_item) = &self.credential_item { db_settings.put(keys::CREDENTIAL_ITEM, &credential_item)?; } + if let Some(token) = &self.token { + db_settings.put(keys::TOKEN, &token)?; + } Ok(()) } } diff --git a/kordophoned/src/dbus/server_impl.rs b/kordophoned/src/dbus/server_impl.rs index 6d82bed..25a1f3c 100644 --- a/kordophoned/src/dbus/server_impl.rs +++ b/kordophoned/src/dbus/server_impl.rs @@ -108,6 +108,7 @@ impl DbusSettings for ServerImpl { server_url: Some(url), username: Some(user), credential_item: None, + token: None, }, r) ) } @@ -123,6 +124,7 @@ impl DbusSettings for ServerImpl { server_url: Some(value), username: None, credential_item: None, + token: None, }, r) ) } @@ -138,6 +140,7 @@ impl DbusSettings for ServerImpl { server_url: None, username: Some(value), credential_item: None, + token: None, }, r) ) } @@ -153,6 +156,7 @@ impl DbusSettings for ServerImpl { server_url: None, username: None, credential_item: Some(value.to_string()), + token: None, }, r) ) } diff --git a/kpcli/src/client/mod.rs b/kpcli/src/client/mod.rs index d683593..1057601 100644 --- a/kpcli/src/client/mod.rs +++ b/kpcli/src/client/mod.rs @@ -1,13 +1,13 @@ use kordophone::APIInterface; use kordophone::api::http_client::HTTPAPIClient; use kordophone::api::http_client::Credentials; -use kordophone::api::InMemoryTokenStore; +use kordophone::api::InMemoryAuthenticationStore; use anyhow::Result; use clap::Subcommand; use crate::printers::{ConversationPrinter, MessagePrinter}; -pub fn make_api_client_from_env() -> HTTPAPIClient { +pub fn make_api_client_from_env() -> HTTPAPIClient { dotenv::dotenv().ok(); // read from env @@ -22,7 +22,7 @@ pub fn make_api_client_from_env() -> HTTPAPIClient { .expect("KORDOPHONE_PASSWORD must be set"), }; - HTTPAPIClient::new(base_url.parse().unwrap(), credentials.into(), InMemoryTokenStore::new()) + HTTPAPIClient::new(base_url.parse().unwrap(), InMemoryAuthenticationStore::new(Some(credentials))) } #[derive(Subcommand)] @@ -51,7 +51,7 @@ impl Commands { } struct ClientCli { - api: HTTPAPIClient, + api: HTTPAPIClient, } impl ClientCli {