user.write_to_database

Cargo.lock

@@ -2663,10 +2663,12 @@ version = "0.1.0"
 dependencies = [
  "argon2",
  "atom_syndication",
+ "base64 0.21.7",
  "bcrypt",
  "chrono",
  "clap",
  "feed-rs",
+ "getrandom 0.2.15",
  "reqwest",
  "rocket",
  "rocket_db_pools",

Cargo.toml

@@ -20,3 +20,5 @@ feed-rs = "2.3.1"
 reqwest = { version = "0.12.12", features = ["json"] }
 tokio = "1.43.0"
 time = "0.3.37"
+base64 = "0.21"
+getrandom = "0.2"

TODO.md

@@ -1,24 +1,10 @@
 # TODO List
 
-## Security Improvements
-
-### Make Server Secret Configurable
-Currently, the server secret used for cookie encryption is not configurable and uses Rocket's default. We should:
-- Add a configuration option for the server secret
-- Allow it to be set via environment variable or config file
-- Generate and persist a random secret on first run if none is provided
-- Add documentation about the security implications of the secret
-
 ### Improve Session Management
 Current session management is basic and needs improvement:
-- Replace simple user_id cookie with a proper session system
 - Add session expiry and renewal logic
 - Store sessions in the database with proper cleanup
 - Add ability to revoke sessions
 - Consider adding "remember me" functionality
 - Add session tracking (last used, IP, user agent, etc.)
 
-Reference: [Current basic implementation in user.rs](src/user.rs) with the comment:
-```rust
-// TODO there should be a more complicated notion of a session
-``` 

src/demo.rs

@@ -1,51 +1,32 @@
-use chrono;
-use sqlx;
-use rocket::serde;
-use uuid::Uuid;
-
 use crate::feeds::Feed;
+use crate::user::User;
 
 pub async fn setup_demo_data(pool: &sqlx::SqlitePool) {
     // Create admin user
-    let admin_id = Uuid::new_v4();
-    let admin_id_str = admin_id.to_string();
-    let admin_hash = bcrypt::hash("admin", bcrypt::DEFAULT_COST).unwrap();
-    let now = chrono::Utc::now().to_rfc3339();
-
-    sqlx::query(
-        "INSERT INTO users (id, username, password_hash, email, display_name, created_at, admin) 
-         VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
-    )
-    .bind(&admin_id_str)
-    .bind("admin")
-    .bind(&admin_hash)
-    .bind(Option::<String>::None)
-    .bind(Option::<String>::None)
-    .bind(&now)
-    .bind(true)
-    .execute(pool)
-    .await
-    .expect("Failed to create admin user");
+    let mut admin = User::new(
+        "admin".to_string(),
+        bcrypt::hash("admin", bcrypt::DEFAULT_COST).unwrap(),
+        None,
+        None,
+    );
+    admin.admin = true;
+    admin
+        .write_to_database(pool)
+        .await
+        .expect("Failed to create admin user");
 
     // Create demo user
-    let demo_id = Uuid::new_v4();
-    let demo_id_str = demo_id.to_string();
-    let demo_hash = bcrypt::hash("demo", bcrypt::DEFAULT_COST).unwrap();
+    let demo = User::new(
+        "demo".to_string(),
+        bcrypt::hash("demo", bcrypt::DEFAULT_COST).unwrap(),
+        None,
+        None,
+    );
+    demo.write_to_database(pool)
+        .await
+        .expect("Failed to create demo user");
 
-    sqlx::query(
-        "INSERT INTO users (id, username, password_hash, email, display_name, created_at, admin) 
-         VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
-    )
-    .bind(&demo_id_str)
-    .bind("demo")
-    .bind(&demo_hash)
-    .bind(Option::<String>::None)
-    .bind(Option::<String>::None)
-    .bind(&now)
-    .bind(false)
-    .execute(pool)
-    .await
-    .expect("Failed to create demo user");
+    let demo_id = demo.id;
 
     let mut bbc_news = Feed::new(
         "BBC News".to_string(),
@@ -77,30 +58,10 @@ pub async fn setup_demo_data(pool: &sqlx::SqlitePool) {
     );
     acx.categorization = vec!["Substack".to_string()];
 
-
     let feeds = [bbc_news, xkcd, isidore, acx];
 
     for feed in feeds {
-        // TODO: This insert logic is substantially the same as Feed::write_to_database.
-        // Should find a way to unify these two code paths to avoid duplication.
-        let categorization_json = serde::json::to_value(feed.categorization).map_err(|e| {
-            eprintln!("Failed to serialize categorization: {}", e);
-            sqlx::Error::Decode(Box::new(e))
-        }).unwrap();
-        println!("{}", categorization_json);
-
-        sqlx::query(
-            "INSERT INTO feeds (feed_id, name, url, user_id, added_time, last_checked_time, categorization) 
-            VALUES (?1, ?2, ?3, ?4, ?5, ?6, json(?7))",
-        )
-            .bind(feed.feed_id.to_string())
-            .bind(&feed.name)
-            .bind(feed.url.as_str())
-            .bind(feed.user_id.to_string())
-            .bind(feed.added_time.to_rfc3339())
-            .bind(feed.last_checked_time.to_rfc3339())
-            .bind(categorization_json.to_string())
-            .execute(pool)
+        feed.write_to_database(pool)
             .await
             .expect("Failed to create demo feed");
     }

src/feeds.rs

@@ -2,6 +2,7 @@ use rocket::http::Status;
 use rocket::serde::{self, json::Json, Deserialize, Serialize};
 use rocket_db_pools::Connection;
 use sqlx::types::JsonValue;
+use sqlx::Executor;
 use url::Url;
 use uuid::Uuid;
 
@@ -35,7 +36,10 @@ impl Feed {
         }
     }
 
-    pub async fn write_to_database(&self, mut db: Connection<Db>) -> Result<(), sqlx::Error> {
+    pub async fn write_to_database<'a, E>(&self, executor: E) -> sqlx::Result<()>
+    where
+        E: Executor<'a, Database = sqlx::Sqlite>,
+    {
         // Convert categorization to JSON value
         let categorization_json = serde::json::to_value(&self.categorization).map_err(|e| {
             eprintln!("Failed to serialize categorization: {}", e);
@@ -53,7 +57,7 @@ impl Feed {
         .bind(self.added_time.to_rfc3339())
         .bind(self.last_checked_time.to_rfc3339())
         .bind(categorization_json.to_string())
-        .execute(&mut **db)
+        .execute(executor)
         .await?;
 
         Ok(())
@@ -70,7 +74,7 @@ pub struct NewFeed {
 
 #[post("/feeds", data = "<new_feed>")]
 pub async fn create_feed(
-    db: Connection<Db>,
+    mut db: Connection<Db>,
     new_feed: Json<NewFeed>,
     user: AuthenticatedUser,
 ) -> Result<Json<Feed>, Status> {
@@ -97,7 +101,7 @@ pub async fn create_feed(
     let mut feed = Feed::new(name, new_feed.url, user.user_id);
     feed.categorization = new_feed.categorization;
 
-    match feed.write_to_database(db).await {
+    match feed.write_to_database(&mut **db).await {
         Ok(_) => Ok(Json(feed)),
         Err(e) => {
             eprintln!("Database error: {}", e);

src/main.rs

@@ -101,7 +101,10 @@ fn rocket() -> _ {
 
     let figment = rocket::Config::figment()
         .merge(("databases.rss_data.url", db_url))
-        .merge(("secret_key", std::env::var("SECRET_KEY").expect("SECRET_KEY environment variable must be set")));
+        .merge((
+            "secret_key",
+            std::env::var("SECRET_KEY").expect("SECRET_KEY environment variable must be set"),
+        ));
 
     rocket::custom(figment)
         .mount(
@@ -128,6 +131,7 @@ fn rocket() -> _ {
         .attach(Template::fairing())
         .attach(Db::init())
         .manage(args.demo)
+        .manage(user::SessionStore::new())
         .attach(AdHoc::try_on_ignite("DB Setup", move |rocket| async move {
             setup_database(args.demo, rocket).await
         }))

src/user.rs

@@ -1,13 +1,57 @@
 use time::Duration;
 
+use base64::{engine::general_purpose::STANDARD as BASE64, Engine as _};
 use rocket::http::{Cookie, CookieJar, Status};
 use rocket::serde::{json::Json, Deserialize, Serialize};
+use rocket::State;
 use rocket_db_pools::Connection;
 use rocket_dyn_templates::{context, Template};
+use std::collections::{HashMap, HashSet};
+use std::sync::RwLock;
 use uuid::Uuid;
 
 use crate::Db;
 
+pub struct SessionStore(RwLock<HashMap<Uuid, HashSet<String>>>);
+
+impl SessionStore {
+    pub fn new() -> Self {
+        SessionStore(RwLock::new(HashMap::new()))
+    }
+
+    fn generate_secret() -> String {
+        let mut bytes = [0u8; 32];
+        getrandom::getrandom(&mut bytes).expect("Failed to generate random bytes");
+        BASE64.encode(bytes)
+    }
+
+    fn store(&self, user_id: Uuid, secret: String) {
+        let mut store = self.0.write().unwrap();
+        store
+            .entry(user_id)
+            .or_insert_with(HashSet::new)
+            .insert(secret);
+    }
+
+    fn verify(&self, user_id: Uuid, secret: &str) -> bool {
+        let store = self.0.read().unwrap();
+        store
+            .get(&user_id)
+            .map_or(false, |secrets| secrets.contains(secret))
+    }
+
+    fn remove(&self, user_id: Uuid, secret: &str) {
+        let mut store = self.0.write().unwrap();
+        if let Some(secrets) = store.get_mut(&user_id) {
+            secrets.remove(secret);
+            // Clean up the user entry if no sessions remain
+            if secrets.is_empty() {
+                store.remove(&user_id);
+            }
+        }
+    }
+}
+
 #[derive(Debug, Serialize)]
 #[serde(crate = "rocket::serde")]
 pub struct User {
@@ -37,6 +81,29 @@ impl User {
             admin: false,
         }
     }
+
+    pub async fn write_to_database<'a, E>(&self, executor: E) -> sqlx::Result<()>
+    where
+        E: sqlx::Executor<'a, Database = sqlx::Sqlite>,
+    {
+        sqlx::query(
+            r#"
+            INSERT INTO users (id, username, password_hash, email, display_name, created_at, admin) 
+            VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)
+            "#,
+        )
+        .bind(self.id.to_string())
+        .bind(self.username.clone())
+        .bind(self.password_hash.clone())
+        .bind(self.email.clone())
+        .bind(self.display_name.clone())
+        .bind(self.created_at.to_rfc3339())
+        .bind(self.admin)
+        .execute(executor)
+        .await?;
+
+        Ok(())
+    }
 }
 
 #[derive(Debug, Deserialize)]
@@ -86,19 +153,7 @@ pub async fn create_user(
         new_user.display_name,
     );
 
-    let query = sqlx::query(
-        "INSERT INTO users (id, username, password_hash, email, display_name, created_at, admin) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)"
-    )
-        .bind(user.id.to_string())
-        .bind(user.username.as_str())
-        .bind(user.password_hash.as_str())
-        .bind(user.email.as_ref())
-        .bind(user.display_name.as_ref())
-        .bind(user.created_at.to_rfc3339())
-        .bind(false)
-        .execute(&mut **db).await;
-
-    match query {
+    match user.write_to_database(&mut **db).await {
         Ok(_) => Ok(Json(user)),
         Err(e) => {
             eprintln!("Database error: {}", e);
@@ -184,6 +239,7 @@ pub async fn login(
     mut db: Connection<Db>,
     credentials: Json<LoginCredentials>,
     cookies: &CookieJar<'_>,
+    sessions: &State<SessionStore>,
 ) -> Result<Json<LoginResponse>, Status> {
     let creds = credentials.into_inner();
 
@@ -219,13 +275,15 @@ pub async fn login(
         return Err(Status::Unauthorized);
     }
 
-    // Set session cookie
-    // TODO tehre should be a more complicated notion of a session
+    // Generate and store session
     let user_id = Uuid::parse_str(&user.id).map_err(|_| Status::InternalServerError)?;
+    let session_secret = SessionStore::generate_secret();
+    sessions.store(user_id, session_secret.clone());
 
-    //TODO make this user-configurable
+    // Set session cookie with both user_id and secret
     let max_age = Duration::days(6);
-    let mut cookie = Cookie::new("user_id", user_id.to_string());
+    let cookie_value = format!("{}:{}", user_id, session_secret);
+    let mut cookie = Cookie::new("user_id", cookie_value);
     cookie.set_max_age(max_age);
     cookies.add_private(cookie);
 
@@ -236,8 +294,15 @@ pub async fn login(
 }
 
 #[post("/logout")]
-pub fn logout(cookies: &CookieJar<'_>) -> Status {
-    cookies.remove_private(Cookie::build("user_id"));
+pub fn logout(cookies: &CookieJar<'_>, sessions: &State<SessionStore>) -> Status {
+    if let Some(cookie) = cookies.get_private("user_id") {
+        if let Some((user_id, secret)) = cookie.value().split_once(':') {
+            if let Ok(user_id) = Uuid::parse_str(user_id) {
+                sessions.remove(user_id, secret);
+            }
+        }
+        cookies.remove_private(Cookie::build("user_id"));
+    }
     Status::NoContent
 }
 
@@ -255,13 +320,18 @@ impl<'r> rocket::request::FromRequest<'r> for AuthenticatedUser {
     ) -> rocket::request::Outcome<Self, Self::Error> {
         use rocket::request::Outcome;
 
+        let sessions = request.rocket().state::<SessionStore>().unwrap();
+
         match request.cookies().get_private("user_id") {
             Some(cookie) => {
-                if let Ok(user_id) = Uuid::parse_str(cookie.value()) {
-                    Outcome::Success(AuthenticatedUser { user_id })
-                } else {
-                    Outcome::Forward(Status::Unauthorized)
+                if let Some((user_id, secret)) = cookie.value().split_once(':') {
+                    if let Ok(user_id) = Uuid::parse_str(user_id) {
+                        if sessions.verify(user_id, secret) {
+                            return Outcome::Success(AuthenticatedUser { user_id });
+                        }
+                    }
                 }
+                Outcome::Forward(Status::Unauthorized)
             }
             None => Outcome::Forward(Status::Unauthorized),
         }
@@ -291,6 +361,8 @@ pub async fn setup(
     mut db: Connection<Db>,
     new_user: Json<NewUser>,
 ) -> Result<Status, Json<SetupError>> {
+    let new_user = new_user.into_inner();
+
     // Check if any users exist
     let count = sqlx::query!("SELECT COUNT(*) as count FROM users")
         .fetch_one(&mut **db)
@@ -309,34 +381,25 @@ pub async fn setup(
         }));
     }
 
+    let password = new_user.password.as_bytes();
+
     // Hash the password
-    let password_hash =
-        bcrypt::hash(new_user.password.as_bytes(), bcrypt::DEFAULT_COST).map_err(|e| {
-            eprintln!("Password hashing error: {}", e);
-            Json(SetupError {
-                error: "Failed to process password".to_string(),
-            })
-        })?;
+    let password_hash = bcrypt::hash(password, bcrypt::DEFAULT_COST).map_err(|e| {
+        eprintln!("Password hashing error: {}", e);
+        Json(SetupError {
+            error: "Failed to process password".to_string(),
+        })
+    })?;
 
-    // Create admin user
-    let user_id = Uuid::new_v4().to_string();
-    let now = chrono::Utc::now().to_rfc3339();
+    let mut user = User::new(
+        new_user.username,
+        password_hash,
+        new_user.email,
+        new_user.display_name,
+    );
+    user.admin = true; // This is an admin user
 
-    let result = sqlx::query(
-        "INSERT INTO users (id, username, password_hash, email, display_name, created_at, admin) 
-         VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
-    )
-    .bind(&user_id)
-    .bind(&new_user.username)
-    .bind(&password_hash)
-    .bind(&new_user.email)
-    .bind(&new_user.display_name)
-    .bind(&now)
-    .bind(true) // This is an admin user
-    .execute(&mut **db)
-    .await;
-
-    match result {
+    match user.write_to_database(&mut **db).await {
         Ok(_) => Ok(Status::Created),
         Err(e) => {
             eprintln!("Database error: {}", e);