From 54a723f803150d1060bac6636d8d4c6f7318b25a Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 15 Mar 2025 18:28:40 +0100 Subject: [PATCH] refactor: moved auth business logic outside of db using store abstraction --- src/cli/serve.rs | 4 +- src/db/repository/auth.rs | 60 +++++++++++++++++++++++ src/gpodder/mod.rs | 16 +++++++ src/gpodder/models.rs | 9 +++- src/gpodder/repository.rs | 74 +++++++++++++++++++++++++++++ src/server/gpodder/advanced/auth.rs | 23 ++++----- src/server/gpodder/mod.rs | 13 ++--- src/server/mod.rs | 1 + 8 files changed, 179 insertions(+), 21 deletions(-) create mode 100644 src/gpodder/repository.rs diff --git a/src/cli/serve.rs b/src/cli/serve.rs index a0ff0fe..1183814 100644 --- a/src/cli/serve.rs +++ b/src/cli/serve.rs @@ -6,9 +6,11 @@ pub fn serve(config: &crate::config::Config) -> u8 { tracing::info!("Initializing database and running migrations"); let pool = db::initialize_db(config.data_dir.join(crate::DB_FILENAME), true).unwrap(); + let repo = db::SqliteRepository::from(pool); let ctx = server::Context { - repo: db::SqliteRepository::from(pool), + repo: repo.clone(), + store: crate::gpodder::GpodderRepository::new(Box::new(repo)), }; let app = server::app(ctx); diff --git a/src/db/repository/auth.rs b/src/db/repository/auth.rs index 453c1f5..e65f046 100644 --- a/src/db/repository/auth.rs +++ b/src/db/repository/auth.rs @@ -1,3 +1,4 @@ +use chrono::DateTime; use diesel::prelude::*; use rand::Rng; @@ -19,6 +20,16 @@ impl From for gpodder::AuthErr { } } +impl From for gpodder::User { + fn from(value: db::User) -> Self { + Self { + id: value.id, + username: value.username, + password_hash: value.password_hash, + } + } +} + impl gpodder::AuthRepository for SqliteRepository { fn validate_credentials( &self, @@ -35,6 +46,7 @@ impl gpodder::AuthRepository for SqliteRepository { Ok(gpodder::User { id: user.id, username: user.username, + password_hash: user.password_hash, }) } else { Err(gpodder::AuthErr::InvalidPassword) @@ -54,6 +66,7 @@ impl gpodder::AuthRepository for SqliteRepository { Ok(user) => Ok(gpodder::User { id: user.id, username: user.username, + password_hash: user.password_hash, }), Err(diesel::result::Error::NotFound) => Err(gpodder::AuthErr::UnknownSession), Err(err) => Err(gpodder::AuthErr::Other(Box::new(err))), @@ -88,6 +101,7 @@ impl gpodder::AuthRepository for SqliteRepository { gpodder::User { id: user.id, username: user.username, + password_hash: user.password_hash, }, )) } else { @@ -122,3 +136,49 @@ impl gpodder::AuthRepository for SqliteRepository { } } } + +impl gpodder::AuthStore for SqliteRepository { + fn get_user(&self, username: &str) -> Result, AuthErr> { + Ok(users::table + .select(db::User::as_select()) + .filter(users::username.eq(username)) + .first(&mut self.pool.get()?) + .optional()? + .map(gpodder::User::from)) + } + + fn get_session(&self, session_id: i64) -> Result, AuthErr> { + match sessions::table + .inner_join(users::table) + .filter(sessions::id.eq(session_id)) + .select((db::Session::as_select(), db::User::as_select())) + .get_result(&mut self.pool.get()?) + { + Ok((session, user)) => Ok(Some(gpodder::Session { + id: session.id, + last_seen: DateTime::from_timestamp(session.last_seen, 0).unwrap(), + user: user.into(), + })), + Err(err) => Err(AuthErr::from(err)), + } + } + + fn remove_session(&self, session_id: i64) -> Result<(), AuthErr> { + Ok( + diesel::delete(sessions::table.filter(sessions::id.eq(session_id))) + .execute(&mut self.pool.get()?) + .map(|_| ())?, + ) + } + + fn insert_session(&self, session: &gpodder::Session) -> Result<(), AuthErr> { + Ok(db::Session { + id: session.id, + user_id: session.user.id, + last_seen: session.last_seen.timestamp(), + } + .insert_into(sessions::table) + .execute(&mut self.pool.get()?) + .map(|_| ())?) + } +} diff --git a/src/gpodder/mod.rs b/src/gpodder/mod.rs index 4ae79d3..567badf 100644 --- a/src/gpodder/mod.rs +++ b/src/gpodder/mod.rs @@ -1,6 +1,8 @@ pub mod models; +mod repository; pub use models::*; +pub use repository::GpodderRepository; pub enum AuthErr { UnknownSession, @@ -27,6 +29,20 @@ pub trait AuthRepository { fn remove_session(&self, username: &str, session_id: i64) -> Result<(), AuthErr>; } +pub trait AuthStore { + /// Retrieve the session with the given session ID + fn get_session(&self, session_id: i64) -> Result, AuthErr>; + + /// Retrieve the user with the given username + fn get_user(&self, username: &str) -> Result, AuthErr>; + + /// Create a new session for a user with the given session ID + fn insert_session(&self, session: &Session) -> Result<(), AuthErr>; + + /// Remove the session with the given session ID + fn remove_session(&self, session_id: i64) -> Result<(), AuthErr>; +} + pub trait DeviceRepository { /// Return all devices associated with the user fn devices_for_user(&self, user: &User) -> Result, AuthErr>; diff --git a/src/gpodder/models.rs b/src/gpodder/models.rs index 39a615c..2ecb004 100644 --- a/src/gpodder/models.rs +++ b/src/gpodder/models.rs @@ -1,10 +1,11 @@ -use chrono::NaiveDateTime; +use chrono::{DateTime, NaiveDateTime, Utc}; use serde::{Deserialize, Serialize}; #[derive(Clone)] pub struct User { pub id: i64, pub username: String, + pub password_hash: String, } #[derive(Serialize, Deserialize)] @@ -57,3 +58,9 @@ pub struct EpisodeAction { #[serde(flatten)] pub action: EpisodeActionType, } + +pub struct Session { + pub id: i64, + pub last_seen: DateTime, + pub user: User, +} diff --git a/src/gpodder/repository.rs b/src/gpodder/repository.rs new file mode 100644 index 0000000..a6fb0b1 --- /dev/null +++ b/src/gpodder/repository.rs @@ -0,0 +1,74 @@ +use std::sync::Arc; + +use argon2::{Argon2, PasswordHash, PasswordVerifier}; +use rand::Rng; + +use super::{models, AuthErr, AuthStore}; + +const MAX_SESSION_AGE: i64 = 60 * 60 * 24 * 7; + +type Store = dyn AuthStore + Send + Sync; + +#[derive(Clone)] +pub struct GpodderRepository { + store: Arc, +} + +impl GpodderRepository { + pub fn new(store: Box) -> Self { + Self { + store: Arc::from(store), + } + } + + pub fn validate_session(&self, session_id: i64) -> Result { + let session = self + .store + .get_session(session_id)? + .ok_or(AuthErr::UnknownSession)?; + + // Expired sessions still in the database are considered removed + if chrono::Utc::now() - session.last_seen + > chrono::TimeDelta::new(MAX_SESSION_AGE, 0).unwrap() + { + Err(AuthErr::UnknownSession) + } else { + Ok(session) + } + } + + pub fn validate_credentials( + &self, + username: &str, + password: &str, + ) -> Result { + let user = self.store.get_user(username)?.ok_or(AuthErr::UnknownUser)?; + + let password_hash = PasswordHash::new(&user.password_hash).unwrap(); + + if Argon2::default() + .verify_password(password.as_bytes(), &password_hash) + .is_ok() + { + Ok(user) + } else { + Err(AuthErr::InvalidPassword) + } + } + + pub fn create_session(&self, user: &models::User) -> Result { + let session = models::Session { + id: rand::thread_rng().gen(), + last_seen: chrono::Utc::now(), + user: user.clone(), + }; + + self.store.insert_session(&session)?; + + Ok(session) + } + + pub fn remove_session(&self, session_id: i64) -> Result<(), AuthErr> { + self.store.remove_session(session_id) + } +} diff --git a/src/server/gpodder/advanced/auth.rs b/src/server/gpodder/advanced/auth.rs index abdb0d6..ac5cbe6 100644 --- a/src/server/gpodder/advanced/auth.rs +++ b/src/server/gpodder/advanced/auth.rs @@ -12,13 +12,10 @@ use axum_extra::{ TypedHeader, }; -use crate::{ - gpodder::AuthRepository, - server::{ - error::{AppError, AppResult}, - gpodder::SESSION_ID_COOKIE, - Context, - }, +use crate::server::{ + error::{AppError, AppResult}, + gpodder::SESSION_ID_COOKIE, + Context, }; pub fn router() -> Router { @@ -38,14 +35,17 @@ async fn post_login( return Err(AppError::BadRequest); } - let (session_id, _) = tokio::task::spawn_blocking(move || { - ctx.repo.create_session(auth.username(), auth.password()) + let session = tokio::task::spawn_blocking(move || { + let user = ctx + .store + .validate_credentials(auth.username(), auth.password())?; + ctx.store.create_session(&user) }) .await .unwrap()?; Ok(jar.add( - Cookie::build((SESSION_ID_COOKIE, session_id.to_string())).expires(Expiration::Session), + Cookie::build((SESSION_ID_COOKIE, session.id.to_string())).expires(Expiration::Session), )) } @@ -60,7 +60,8 @@ async fn post_logout( .parse() .map_err(|_| AppError::BadRequest)?; - tokio::task::spawn_blocking(move || ctx.repo.remove_session(&username, session_id)) + // TODO reintroduce username check + tokio::task::spawn_blocking(move || ctx.store.remove_session(session_id)) .await .unwrap()?; diff --git a/src/server/gpodder/mod.rs b/src/server/gpodder/mod.rs index 5483935..776100e 100644 --- a/src/server/gpodder/mod.rs +++ b/src/server/gpodder/mod.rs @@ -17,10 +17,7 @@ use axum_extra::{ }; use tower_http::set_header::SetResponseHeaderLayer; -use crate::{ - gpodder::{self, AuthRepository}, - server::error::AppError, -}; +use crate::{gpodder, server::error::AppError}; use super::Context; @@ -51,12 +48,12 @@ pub async fn auth_middleware(State(ctx): State, mut req: Request, next: .and_then(|c| c.value().parse::().ok()) { let ctx_clone = ctx.clone(); - match tokio::task::spawn_blocking(move || ctx_clone.repo.validate_session(session_id)) + match tokio::task::spawn_blocking(move || ctx_clone.store.validate_session(session_id)) .await .unwrap() { - Ok(user) => { - auth_user = Some(user); + Ok(session) => { + auth_user = Some(session.user); } Err(gpodder::AuthErr::UnknownSession) => { jar = jar.add( @@ -77,7 +74,7 @@ pub async fn auth_middleware(State(ctx): State, mut req: Request, next: .await { match tokio::task::spawn_blocking(move || { - ctx.repo + ctx.store .validate_credentials(auth.username(), auth.password()) }) .await diff --git a/src/server/mod.rs b/src/server/mod.rs index 3e7a0e1..0a88fd6 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -7,6 +7,7 @@ use tower_http::trace::TraceLayer; #[derive(Clone)] pub struct Context { pub repo: crate::db::SqliteRepository, + pub store: crate::gpodder::GpodderRepository, } pub fn app(ctx: Context) -> Router {