From 648921837b17630e547d9f5901cddf3f60fe0ef1 Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Tue, 4 Mar 2025 20:04:44 +0100 Subject: [PATCH] feat: improve authentication flow authentication now works either with sessionid or basic auth, with basic auth not creating a session --- Cargo.lock | 1 + Cargo.toml | 1 + src/db/repository/auth.rs | 24 ++++++++++++ src/gpodder/mod.rs | 4 ++ src/server/gpodder/mod.rs | 76 ++++++++++++++++++------------------ src/server/gpodder/models.rs | 2 +- 6 files changed, 69 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c8d1d6..d178dd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,6 +896,7 @@ dependencies = [ "axum-extra", "chrono", "clap", + "cookie", "diesel", "diesel_migrations", "libsqlite3-sys", diff --git a/Cargo.toml b/Cargo.toml index fee72f0..057325c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ axum = { version = "0.8.1", features = ["macros"] } axum-extra = { version = "0.10", features = ["cookie", "typed-header"] } chrono = { version = "0.4.39", features = ["serde"] } clap = { version = "4.5.30", features = ["derive", "env"] } +cookie = "0.18.1" diesel = { version = "2.2.7", features = ["r2d2", "sqlite", "returning_clauses_for_sqlite_3_35"] } diesel_migrations = { version = "2.2.0", features = ["sqlite"] } libsqlite3-sys = { version = "0.31.0", features = ["bundled"] } diff --git a/src/db/repository/auth.rs b/src/db/repository/auth.rs index 32cbf87..ff30f09 100644 --- a/src/db/repository/auth.rs +++ b/src/db/repository/auth.rs @@ -20,6 +20,30 @@ impl From for gpodder::AuthErr { } impl gpodder::AuthRepository for SqliteRepository { + fn validate_credentials( + &self, + username: &str, + password: &str, + ) -> Result { + if let Some(user) = users::table + .select(db::User::as_select()) + .filter(users::username.eq(username)) + .first(&mut self.pool.get()?) + .optional()? + { + if user.verify_password(password) { + Ok(gpodder::User { + id: user.id, + username: user.username, + }) + } else { + Err(gpodder::AuthErr::InvalidPassword) + } + } else { + Err(gpodder::AuthErr::UnknownUser) + } + } + fn validate_session(&self, session_id: i64) -> Result { match sessions::dsl::sessions .inner_join(users::table) diff --git a/src/gpodder/mod.rs b/src/gpodder/mod.rs index 4bad4fd..4ae79d3 100644 --- a/src/gpodder/mod.rs +++ b/src/gpodder/mod.rs @@ -13,6 +13,10 @@ pub trait AuthRepository { /// Validate the given session ID and return its user. fn validate_session(&self, session_id: i64) -> Result; + /// Validate the credentials, returning the user if the credentials are correct. + fn validate_credentials(&self, username: &str, password: &str) + -> Result; + /// Create a new session for the given user. fn create_session( &self, diff --git a/src/server/gpodder/mod.rs b/src/server/gpodder/mod.rs index 0b9dedd..7850dd3 100644 --- a/src/server/gpodder/mod.rs +++ b/src/server/gpodder/mod.rs @@ -44,44 +44,57 @@ pub fn router(ctx: Context) -> Router { /// This middleware accepts pub async fn auth_middleware(State(ctx): State, mut req: Request, next: Next) -> Response { - // SAFETY: this extractor's error type is Infallible - let jar: CookieJar = req.extract_parts().await.unwrap(); - let mut auth_user = None; - let mut new_session_id = None; + tracing::debug!("{:?}", req.headers()); + // SAFETY: this extractor's error type is Infallible + let mut jar: CookieJar = req.extract_parts().await.unwrap(); + let mut auth_user = None; + + // First try to validate the session if let Some(session_id) = jar .get(SESSION_ID_COOKIE) .and_then(|c| c.value().parse::().ok()) { - match tokio::task::spawn_blocking(move || ctx.repo.validate_session(session_id)) + let ctx_clone = ctx.clone(); + match tokio::task::spawn_blocking(move || ctx_clone.repo.validate_session(session_id)) .await .unwrap() - .map_err(AppError::from) { Ok(user) => { auth_user = Some(user); } + Err(gpodder::AuthErr::UnknownSession) => { + jar = jar.add( + Cookie::build((SESSION_ID_COOKIE, String::new())) + .max_age(cookie::time::Duration::ZERO), + ); + } Err(err) => { - return err.into_response(); + return AppError::from(err).into_response(); } }; - } else if let Ok(auth) = req - .extract_parts::>>() - .await - { - match tokio::task::spawn_blocking(move || { - ctx.repo.create_session(auth.username(), auth.password()) - }) - .await - .unwrap() - .map_err(AppError::from) + } + + // Only if the sessionid wasn't present or valid do we check the credentials. + if auth_user.is_none() { + if let Ok(auth) = req + .extract_parts::>>() + .await { - Ok((session_id, user)) => { - auth_user = Some(user); - new_session_id = Some(session_id); - } - Err(err) => { - return err.into_response(); + match tokio::task::spawn_blocking(move || { + ctx.repo + .validate_credentials(auth.username(), auth.password()) + }) + .await + .unwrap() + .map_err(AppError::from) + { + Ok(user) => { + auth_user = Some(user); + } + Err(err) => { + return err.into_response(); + } } } } @@ -89,22 +102,9 @@ pub async fn auth_middleware(State(ctx): State, mut req: Request, next: if let Some(user) = auth_user { req.extensions_mut().insert(user); - let res = next.run(req).await; - - if let Some(session_id) = new_session_id { - ( - jar.add( - Cookie::build((SESSION_ID_COOKIE, session_id.to_string())) - .expires(Expiration::Session), - ), - res, - ) - .into_response() - } else { - res - } + (jar, next.run(req).await).into_response() } else { - let mut res = StatusCode::UNAUTHORIZED.into_response(); + let mut res = (jar, StatusCode::UNAUTHORIZED).into_response(); // This is what the gpodder.net service returns, and some clients seem to depend on it res.headers_mut().insert( diff --git a/src/server/gpodder/models.rs b/src/server/gpodder/models.rs index 68f46e3..2be8f87 100644 --- a/src/server/gpodder/models.rs +++ b/src/server/gpodder/models.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -#[derive(Deserialize)] +#[derive(Deserialize, Debug)] pub struct SubscriptionDelta { pub add: Vec, pub remove: Vec,