From 4c040f2c58b0da41de586ae1d05e93ee841ec906 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 14:54:30 +0000 Subject: [PATCH] fix(security): validate session tokens against database - Add sessions table to store session tokens with expiration - Create Session domain model and SessionRepository trait - Implement SqlSessionRepository for session persistence - Update is_authenticated() to validate tokens against database - Sessions expire after 30 days - Session tokens hashed with SHA-256 before storage - Delete sessions from database on logout - Update all page handlers to properly validate sessions This prevents session hijacking by ensuring only valid, unexpired tokens stored in the database can authenticate requests. Co-authored-by: jnsgruk <668505+jnsgruk@users.noreply.github.com> --- migrations/0003_sessions.sql | 13 ++ src/domain/mod.rs | 1 + src/domain/repositories.rs | 10 ++ src/domain/sessions.rs | 35 +++++ src/infrastructure/repositories/mod.rs | 1 + src/infrastructure/repositories/sessions.rs | 155 ++++++++++++++++++++ src/server/routes/auth.rs | 59 +++++++- src/server/routes/roasters.rs | 4 +- src/server/routes/roasts.rs | 4 +- src/server/routes/timeline.rs | 2 +- src/server/server.rs | 10 +- tests/server/helpers.rs | 7 +- 12 files changed, 287 insertions(+), 14 deletions(-) create mode 100644 migrations/0003_sessions.sql create mode 100644 src/domain/sessions.rs create mode 100644 src/infrastructure/repositories/sessions.rs diff --git a/migrations/0003_sessions.sql b/migrations/0003_sessions.sql new file mode 100644 index 0000000..82dff26 --- /dev/null +++ b/migrations/0003_sessions.sql @@ -0,0 +1,13 @@ +-- Add sessions table for web authentication +CREATE TABLE IF NOT EXISTS sessions ( + id TEXT PRIMARY KEY NOT NULL, + user_id TEXT NOT NULL, + session_token_hash TEXT NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS idx_sessions_token_hash ON sessions(session_token_hash); +CREATE INDEX IF NOT EXISTS idx_sessions_user_id ON sessions(user_id); +CREATE INDEX IF NOT EXISTS idx_sessions_expires_at ON sessions(expires_at); diff --git a/src/domain/mod.rs b/src/domain/mod.rs index 34318ea..cd59d28 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -4,6 +4,7 @@ pub mod origins; pub mod repositories; pub mod roasters; pub mod roasts; +pub mod sessions; pub mod timeline; pub mod tokens; pub mod users; diff --git a/src/domain/repositories.rs b/src/domain/repositories.rs index 4aedbef..1389d5f 100644 --- a/src/domain/repositories.rs +++ b/src/domain/repositories.rs @@ -5,6 +5,7 @@ use crate::domain::roasters::RoasterSortKey; use crate::domain::roasters::{Roaster, UpdateRoaster}; use crate::domain::roasts::RoastSortKey; use crate::domain::roasts::{Roast, RoastWithRoaster, UpdateRoast}; +use crate::domain::sessions::{Session, SessionId}; use crate::domain::timeline::{TimelineEvent, TimelineSortKey}; use crate::domain::tokens::{Token, TokenId}; use crate::domain::users::{User, UserId}; @@ -96,3 +97,12 @@ pub trait TokenRepository: Send + Sync { async fn revoke(&self, id: TokenId) -> Result; async fn update_last_used(&self, id: TokenId) -> Result<(), RepositoryError>; } + +#[async_trait] +pub trait SessionRepository: Send + Sync { + async fn insert(&self, session: Session) -> Result; + async fn get(&self, id: SessionId) -> Result; + async fn get_by_token_hash(&self, token_hash: &str) -> Result; + async fn delete(&self, id: SessionId) -> Result<(), RepositoryError>; + async fn delete_expired(&self) -> Result<(), RepositoryError>; +} diff --git a/src/domain/sessions.rs b/src/domain/sessions.rs new file mode 100644 index 0000000..5d9041c --- /dev/null +++ b/src/domain/sessions.rs @@ -0,0 +1,35 @@ +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +pub type SessionId = String; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Session { + pub id: SessionId, + pub user_id: String, + pub session_token_hash: String, + pub created_at: DateTime, + pub expires_at: DateTime, +} + +impl Session { + pub fn new( + id: SessionId, + user_id: String, + session_token_hash: String, + created_at: DateTime, + expires_at: DateTime, + ) -> Self { + Self { + id, + user_id, + session_token_hash, + created_at, + expires_at, + } + } + + pub fn is_expired(&self) -> bool { + Utc::now() > self.expires_at + } +} diff --git a/src/infrastructure/repositories/mod.rs b/src/infrastructure/repositories/mod.rs index f23b2af..e502d78 100644 --- a/src/infrastructure/repositories/mod.rs +++ b/src/infrastructure/repositories/mod.rs @@ -1,5 +1,6 @@ pub mod roasters; pub mod roasts; +pub mod sessions; pub mod timeline_events; pub mod tokens; pub mod users; diff --git a/src/infrastructure/repositories/sessions.rs b/src/infrastructure/repositories/sessions.rs new file mode 100644 index 0000000..d0cb487 --- /dev/null +++ b/src/infrastructure/repositories/sessions.rs @@ -0,0 +1,155 @@ +use async_trait::async_trait; +use chrono::{DateTime, Utc}; +use sqlx::{Pool, Row, Sqlite}; + +use crate::domain::sessions::{Session, SessionId}; +use crate::domain::{RepositoryError, repositories::SessionRepository}; + +pub struct SqlSessionRepository { + pool: Pool, +} + +impl SqlSessionRepository { + pub fn new(pool: Pool) -> Self { + Self { pool } + } +} + +#[async_trait] +impl SessionRepository for SqlSessionRepository { + async fn insert(&self, session: Session) -> Result { + sqlx::query( + r#" + INSERT INTO sessions (id, user_id, session_token_hash, created_at, expires_at) + VALUES (?, ?, ?, ?, ?) + "#, + ) + .bind(&session.id) + .bind(&session.user_id) + .bind(&session.session_token_hash) + .bind(session.created_at.to_rfc3339()) + .bind(session.expires_at.to_rfc3339()) + .execute(&self.pool) + .await + .map_err(|e| RepositoryError::unexpected(format!("failed to insert session: {}", e)))?; + + Ok(session) + } + + async fn get(&self, id: SessionId) -> Result { + let row = sqlx::query( + r#" + SELECT id, user_id, session_token_hash, created_at, expires_at + FROM sessions + WHERE id = ? + "#, + ) + .bind(&id) + .fetch_one(&self.pool) + .await + .map_err(|e| match e { + sqlx::Error::RowNotFound => RepositoryError::NotFound, + _ => RepositoryError::unexpected(format!("failed to get session: {}", e)), + })?; + + let created_at: String = row.try_get("created_at").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse created_at: {}", e)) + })?; + let expires_at: String = row.try_get("expires_at").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse expires_at: {}", e)) + })?; + + Ok(Session { + id: row.try_get("id").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse id: {}", e)) + })?, + user_id: row.try_get("user_id").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse user_id: {}", e)) + })?, + session_token_hash: row.try_get("session_token_hash").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse session_token_hash: {}", e)) + })?, + created_at: DateTime::parse_from_rfc3339(&created_at) + .map_err(|e| { + RepositoryError::unexpected(format!("failed to parse created_at: {}", e)) + })? + .with_timezone(&Utc), + expires_at: DateTime::parse_from_rfc3339(&expires_at) + .map_err(|e| { + RepositoryError::unexpected(format!("failed to parse expires_at: {}", e)) + })? + .with_timezone(&Utc), + }) + } + + async fn get_by_token_hash(&self, token_hash: &str) -> Result { + let row = sqlx::query( + r#" + SELECT id, user_id, session_token_hash, created_at, expires_at + FROM sessions + WHERE session_token_hash = ? + "#, + ) + .bind(token_hash) + .fetch_one(&self.pool) + .await + .map_err(|e| match e { + sqlx::Error::RowNotFound => RepositoryError::NotFound, + _ => RepositoryError::unexpected(format!("failed to get session by token: {}", e)), + })?; + + let created_at: String = row.try_get("created_at").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse created_at: {}", e)) + })?; + let expires_at: String = row.try_get("expires_at").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse expires_at: {}", e)) + })?; + + Ok(Session { + id: row.try_get("id").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse id: {}", e)) + })?, + user_id: row.try_get("user_id").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse user_id: {}", e)) + })?, + session_token_hash: row.try_get("session_token_hash").map_err(|e| { + RepositoryError::unexpected(format!("failed to parse session_token_hash: {}", e)) + })?, + created_at: DateTime::parse_from_rfc3339(&created_at) + .map_err(|e| { + RepositoryError::unexpected(format!("failed to parse created_at: {}", e)) + })? + .with_timezone(&Utc), + expires_at: DateTime::parse_from_rfc3339(&expires_at) + .map_err(|e| { + RepositoryError::unexpected(format!("failed to parse expires_at: {}", e)) + })? + .with_timezone(&Utc), + }) + } + + async fn delete(&self, id: SessionId) -> Result<(), RepositoryError> { + sqlx::query("DELETE FROM sessions WHERE id = ?") + .bind(&id) + .execute(&self.pool) + .await + .map_err(|e| { + RepositoryError::unexpected(format!("failed to delete session: {}", e)) + })?; + + Ok(()) + } + + async fn delete_expired(&self) -> Result<(), RepositoryError> { + let now = Utc::now().to_rfc3339(); + sqlx::query("DELETE FROM sessions WHERE expires_at < ?") + .bind(&now) + .execute(&self.pool) + .await + .map_err(|e| { + RepositoryError::unexpected(format!("failed to delete expired sessions: {}", e)) + })?; + + Ok(()) + } +} diff --git a/src/server/routes/auth.rs b/src/server/routes/auth.rs index 9132ff0..75eba66 100644 --- a/src/server/routes/auth.rs +++ b/src/server/routes/auth.rs @@ -3,10 +3,14 @@ use axum::extract::State; use axum::http::StatusCode; use axum::response::{IntoResponse, Redirect, Response}; use axum::Form; +use chrono::{Duration, Utc}; use serde::Deserialize; use tower_cookies::{Cookie, Cookies}; -use tracing::warn; -use crate::infrastructure::auth::{generate_session_token, verify_password}; +use tracing::{error, warn}; + +use crate::domain::ids::generate_id; +use crate::domain::sessions::Session; +use crate::infrastructure::auth::{generate_session_token, hash_token, verify_password}; use crate::server::routes::render_html; use crate::server::server::AppState; @@ -26,9 +30,12 @@ pub struct LoginForm { password: String, } -pub(crate) async fn login_page(cookies: Cookies) -> Result { +pub(crate) async fn login_page( + State(state): State, + cookies: Cookies, +) -> Result { // Check if already authenticated - if cookies.get(SESSION_COOKIE_NAME).is_some() { + if is_authenticated(&state, &cookies).await { return Ok(Redirect::to("/timeline").into_response()); } @@ -65,6 +72,21 @@ pub(crate) async fn login_submit( // Create session token let session_token = generate_session_token(); + let session_token_hash = hash_token(&session_token); + + // Create session in database (valid for 30 days) + let session = Session::new( + generate_id(), + user.id.clone(), + session_token_hash, + Utc::now(), + Utc::now() + Duration::days(30), + ); + + if let Err(err) = state.session_repo.insert(session).await { + error!(error = %err, "failed to create session"); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } // Set secure cookie let mut cookie = Cookie::new(SESSION_COOKIE_NAME, session_token); @@ -78,7 +100,18 @@ pub(crate) async fn login_submit( Ok(Redirect::to("/timeline").into_response()) } -pub(crate) async fn logout(cookies: Cookies) -> Redirect { +pub(crate) async fn logout(State(state): State, cookies: Cookies) -> Redirect { + // Try to delete session from database if cookie exists + if let Some(cookie) = cookies.get(SESSION_COOKIE_NAME) { + let session_token = cookie.value(); + let session_token_hash = hash_token(session_token); + + // Try to find and delete the session + if let Ok(session) = state.session_repo.get_by_token_hash(&session_token_hash).await { + let _ = state.session_repo.delete(session.id).await; + } + } + cookies.remove(Cookie::from(SESSION_COOKIE_NAME)); Redirect::to("/timeline") } @@ -94,6 +127,18 @@ fn show_login_error(message: &str) -> Result { } /// Check if user is authenticated based on session cookie -pub fn is_authenticated(cookies: &Cookies) -> bool { - cookies.get(SESSION_COOKIE_NAME).is_some() +/// Validates the session token against the database +pub async fn is_authenticated(state: &AppState, cookies: &Cookies) -> bool { + let Some(cookie) = cookies.get(SESSION_COOKIE_NAME) else { + return false; + }; + + let session_token = cookie.value(); + let session_token_hash = hash_token(session_token); + + // Check if session exists and is valid + match state.session_repo.get_by_token_hash(&session_token_hash).await { + Ok(session) => !session.is_expired(), + Err(_) => false, + } } diff --git a/src/server/routes/roasters.rs b/src/server/routes/roasters.rs index 558c2b1..fa169d3 100644 --- a/src/server/routes/roasters.rs +++ b/src/server/routes/roasters.rs @@ -56,7 +56,7 @@ pub(crate) async fn roasters_page( .await .map_err(|err| map_app_error(err))?; - let is_authenticated = crate::server::routes::auth::is_authenticated(&cookies); + let is_authenticated = crate::server::routes::auth::is_authenticated(&state, &cookies).await; let template = RoastersTemplate { nav_active: "roasters", @@ -85,7 +85,7 @@ pub(crate) async fn roaster_page( .map_err(|err| map_app_error(AppError::from(err)))?; let roaster_view = RoasterView::from(roaster); - let is_authenticated = crate::server::routes::auth::is_authenticated(&cookies); + let is_authenticated = crate::server::routes::auth::is_authenticated(&state, &cookies).await; let template = RoasterDetailTemplate { nav_active: "roasters", diff --git a/src/server/routes/roasts.rs b/src/server/routes/roasts.rs index bcc21f9..3beb2ca 100644 --- a/src/server/routes/roasts.rs +++ b/src/server/routes/roasts.rs @@ -63,7 +63,7 @@ pub(crate) async fn roasts_page( .await .map_err(|err| map_app_error(err))?; - let is_authenticated = crate::server::routes::auth::is_authenticated(&cookies); + let is_authenticated = crate::server::routes::auth::is_authenticated(&state, &cookies).await; let template = RoastsTemplate { nav_active: "roasts", @@ -92,7 +92,7 @@ pub(crate) async fn roast_page( .await .map_err(|err| map_app_error(AppError::from(err)))?; - let is_authenticated = crate::server::routes::auth::is_authenticated(&cookies); + let is_authenticated = crate::server::routes::auth::is_authenticated(&state, &cookies).await; let template = RoastDetailTemplate { nav_active: "roasts", diff --git a/src/server/routes/timeline.rs b/src/server/routes/timeline.rs index c3614bd..384280f 100644 --- a/src/server/routes/timeline.rs +++ b/src/server/routes/timeline.rs @@ -35,7 +35,7 @@ pub(crate) async fn timeline_page( .await .map_err(|err| map_app_error(err))?; - let is_authenticated = crate::server::routes::auth::is_authenticated(&cookies); + let is_authenticated = crate::server::routes::auth::is_authenticated(&state, &cookies).await; let template = TimelineTemplate { nav_active: "timeline", diff --git a/src/server/server.rs b/src/server/server.rs index ae38a92..255d758 100644 --- a/src/server/server.rs +++ b/src/server/server.rs @@ -10,13 +10,15 @@ use tracing::info; use crate::domain::ids::generate_id; use crate::domain::repositories::{ - RoastRepository, RoasterRepository, TimelineEventRepository, TokenRepository, UserRepository, + RoastRepository, RoasterRepository, SessionRepository, TimelineEventRepository, + TokenRepository, UserRepository, }; use crate::domain::users::User; use crate::infrastructure::auth::hash_password; use crate::infrastructure::database::Database; use crate::infrastructure::repositories::roasters::SqlRoasterRepository; use crate::infrastructure::repositories::roasts::SqlRoastRepository; +use crate::infrastructure::repositories::sessions::SqlSessionRepository; use crate::infrastructure::repositories::timeline_events::SqlTimelineEventRepository; use crate::infrastructure::repositories::tokens::SqlTokenRepository; use crate::infrastructure::repositories::users::SqlUserRepository; @@ -35,6 +37,7 @@ pub struct AppState { pub timeline_repo: Arc, pub user_repo: Arc, pub token_repo: Arc, + pub session_repo: Arc, } impl AppState { @@ -44,6 +47,7 @@ impl AppState { timeline_repo: Arc, user_repo: Arc, token_repo: Arc, + session_repo: Arc, ) -> Self { Self { roaster_repo, @@ -51,6 +55,7 @@ impl AppState { timeline_repo, user_repo, token_repo, + session_repo, } } } @@ -68,6 +73,8 @@ pub async fn serve(config: ServerConfig) -> anyhow::Result<()> { Arc::new(SqlUserRepository::new(database.clone_pool())); let token_repo: Arc = Arc::new(SqlTokenRepository::new(database.clone_pool())); + let session_repo: Arc = + Arc::new(SqlSessionRepository::new(database.clone_pool())); // Bootstrap admin user if no users exist bootstrap_admin_user(&user_repo, config.admin_password).await?; @@ -78,6 +85,7 @@ pub async fn serve(config: ServerConfig) -> anyhow::Result<()> { timeline_repo, user_repo, token_repo, + session_repo, ); let listener = TcpListener::bind(config.bind_address) diff --git a/tests/server/helpers.rs b/tests/server/helpers.rs index 23db6cf..1e82104 100644 --- a/tests/server/helpers.rs +++ b/tests/server/helpers.rs @@ -1,7 +1,8 @@ use std::sync::Arc; use brewlog::domain::repositories::{ - RoastRepository, RoasterRepository, TimelineEventRepository, TokenRepository, UserRepository, + RoastRepository, RoasterRepository, SessionRepository, TimelineEventRepository, + TokenRepository, UserRepository, }; use brewlog::domain::roasters::{NewRoaster, Roaster}; use brewlog::domain::users::User; @@ -9,6 +10,7 @@ use brewlog::infrastructure::auth::hash_password; use brewlog::infrastructure::database::Database; use brewlog::infrastructure::repositories::roasters::SqlRoasterRepository; use brewlog::infrastructure::repositories::roasts::SqlRoastRepository; +use brewlog::infrastructure::repositories::sessions::SqlSessionRepository; use brewlog::infrastructure::repositories::timeline_events::SqlTimelineEventRepository; use brewlog::infrastructure::repositories::tokens::SqlTokenRepository; use brewlog::infrastructure::repositories::users::SqlUserRepository; @@ -57,6 +59,8 @@ pub async fn spawn_app() -> TestApp { Arc::new(SqlUserRepository::new(database.clone_pool())); let token_repo: Arc = Arc::new(SqlTokenRepository::new(database.clone_pool())); + let session_repo: Arc = + Arc::new(SqlSessionRepository::new(database.clone_pool())); // Create application state let state = AppState::new( @@ -65,6 +69,7 @@ pub async fn spawn_app() -> TestApp { timeline_repo.clone(), user_repo.clone(), token_repo.clone(), + session_repo, ); // Create router