Merge pull request 'Implement critical security improvements for authentication' (#45) from feature/security-improvements into master

Reviewed-on: #45

Cargo.lock

@@ -82,6 +82,18 @@ dependencies = [
  "v_htmlescape",
 ]
 
+[[package]]
+name = "actix-governor"
+version = "0.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a2e7b88f3804e01bd4191fdb08650430bbfcb43d3d9b2890064df3551ec7d25b"
+dependencies = [
+ "actix-http",
+ "actix-web",
+ "futures",
+ "governor",
+]
+
 [[package]]
 name = "actix-http"
 version = "3.11.1"
@@ -874,6 +886,19 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "dashmap"
+version = "5.5.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856"
+dependencies = [
+ "cfg-if",
+ "hashbrown 0.14.5",
+ "lock_api",
+ "once_cell",
+ "parking_lot_core",
+]
+
 [[package]]
 name = "deranged"
 version = "0.5.3"
@@ -1218,6 +1243,12 @@ version = "0.3.31"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988"
 
+[[package]]
+name = "futures-timer"
+version = "3.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24"
+
 [[package]]
 name = "futures-util"
 version = "0.3.31"
@@ -1277,6 +1308,26 @@ version = "0.31.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f"
 
+[[package]]
+name = "governor"
+version = "0.6.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "68a7f542ee6b35af73b06abc0dad1c1bae89964e4e253bc4b587b91c9637867b"
+dependencies = [
+ "cfg-if",
+ "dashmap",
+ "futures",
+ "futures-timer",
+ "no-std-compat",
+ "nonzero_ext",
+ "parking_lot",
+ "portable-atomic",
+ "quanta",
+ "rand 0.8.5",
+ "smallvec",
+ "spinning_top",
+]
+
 [[package]]
 name = "h2"
 version = "0.3.27"
@@ -1315,6 +1366,12 @@ dependencies = [
  "tracing",
 ]
 
+[[package]]
+name = "hashbrown"
+version = "0.14.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1"
+
 [[package]]
 name = "hashbrown"
 version = "0.15.5"
@@ -1605,11 +1662,12 @@ dependencies = [
 
 [[package]]
 name = "image-api"
-version = "0.4.0"
+version = "0.4.1"
 dependencies = [
  "actix",
  "actix-cors",
  "actix-files",
+ "actix-governor",
  "actix-multipart",
  "actix-rt",
  "actix-web",
@@ -1666,7 +1724,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f2481980430f9f78649238835720ddccc57e52df14ffce1c6f37391d61b563e9"
 dependencies = [
  "equivalent",
- "hashbrown",
+ "hashbrown 0.15.5",
 ]
 
 [[package]]
@@ -2018,6 +2076,12 @@ version = "1.0.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086"
 
+[[package]]
+name = "no-std-compat"
+version = "0.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b93853da6d84c2e3c7d730d6473e8817692dd89be387eb01b94d7f108ecb5b8c"
+
 [[package]]
 name = "nom"
 version = "7.1.3"
@@ -2028,6 +2092,12 @@ dependencies = [
  "minimal-lexical",
 ]
 
+[[package]]
+name = "nonzero_ext"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "38bf9645c8b145698bb0b18a4637dcacbc421ea49bef2317e4fd8065a387cf21"
+
 [[package]]
 name = "noop_proc_macro"
 version = "0.3.0"
@@ -2448,6 +2518,21 @@ dependencies = [
  "num-traits",
 ]
 
+[[package]]
+name = "quanta"
+version = "0.12.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f3ab5a9d756f0d97bdc89019bd2e4ea098cf9cde50ee7564dde6b81ccc8f06c7"
+dependencies = [
+ "crossbeam-utils",
+ "libc",
+ "once_cell",
+ "raw-cpuid",
+ "wasi 0.11.1+wasi-snapshot-preview1",
+ "web-sys",
+ "winapi",
+]
+
 [[package]]
 name = "quick-error"
 version = "2.0.1"
@@ -2578,6 +2663,15 @@ dependencies = [
  "rgb",
 ]
 
+[[package]]
+name = "raw-cpuid"
+version = "11.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "498cd0dc59d73224351ee52a95fee0f1a617a2eae0e7d9d720cc622c73a54186"
+dependencies = [
+ "bitflags",
+]
+
 [[package]]
 name = "rayon"
 version = "1.11.0"
@@ -2919,6 +3013,15 @@ dependencies = [
  "windows-sys 0.59.0",
 ]
 
+[[package]]
+name = "spinning_top"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d96d2d1d716fb500937168cc09353ffdc7a012be8475ac7308e1bdf0e3923300"
+dependencies = [
+ "lock_api",
+]
+
 [[package]]
 name = "stable_deref_trait"
 version = "1.2.0"
@@ -3546,6 +3649,22 @@ dependencies = [
  "wasm-bindgen",
 ]
 
+[[package]]
+name = "winapi"
+version = "0.3.9"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419"
+dependencies = [
+ "winapi-i686-pc-windows-gnu",
+ "winapi-x86_64-pc-windows-gnu",
+]
+
+[[package]]
+name = "winapi-i686-pc-windows-gnu"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
+
 [[package]]
 name = "winapi-util"
 version = "0.1.10"
@@ -3555,6 +3674,12 @@ dependencies = [
  "windows-sys 0.60.2",
 ]
 
+[[package]]
+name = "winapi-x86_64-pc-windows-gnu"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
+
 [[package]]
 name = "windows-core"
 version = "0.61.2"

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "image-api"
-version = "0.4.0"
+version = "0.4.1"
 authors = ["Cameron Cordes <cameronc.dev@gmail.com>"]
 edition = "2024"
 
@@ -17,6 +17,7 @@ tokio = { version = "1.42.0", features = ["default", "process", "sync"] }
 actix-files = "0.6"
 actix-cors = "0.7"
 actix-multipart = "0.7.2"
+actix-governor = "0.5"
 futures = "0.3.5"
 jsonwebtoken = "9.3.0"
 serde = "1"

src/auth.rs

@@ -13,22 +13,47 @@ use crate::{
     database::UserDao,
 };
 
+/// Validate password meets security requirements
+fn validate_password(password: &str) -> Result<(), String> {
+    if password.len() < 12 {
+        return Err("Password must be at least 12 characters".into());
+    }
+    if !password.chars().any(|c| c.is_uppercase()) {
+        return Err("Password must contain at least one uppercase letter".into());
+    }
+    if !password.chars().any(|c| c.is_lowercase()) {
+        return Err("Password must contain at least one lowercase letter".into());
+    }
+    if !password.chars().any(|c| c.is_numeric()) {
+        return Err("Password must contain at least one number".into());
+    }
+    if !password.chars().any(|c| !c.is_alphanumeric()) {
+        return Err("Password must contain at least one special character".into());
+    }
+    Ok(())
+}
+
 #[allow(dead_code)]
 async fn register<D: UserDao>(
     user: Json<CreateAccountRequest>,
     user_dao: web::Data<Mutex<D>>,
 ) -> impl Responder {
-    if !user.username.is_empty() && user.password.len() > 5 && user.password == user.confirmation {
+    // Validate password strength
+    if let Err(msg) = validate_password(&user.password) {
+        return HttpResponse::BadRequest().body(msg);
+    }
+
+    if !user.username.is_empty() && user.password == user.confirmation {
         let mut dao = user_dao.lock().expect("Unable to get UserDao");
         if dao.user_exists(&user.username) {
-            HttpResponse::BadRequest()
+            HttpResponse::BadRequest().finish()
         } else if let Some(_user) = dao.create_user(&user.username, &user.password) {
-            HttpResponse::Ok()
+            HttpResponse::Ok().finish()
         } else {
-            HttpResponse::InternalServerError()
+            HttpResponse::InternalServerError().finish()
         }
     } else {
-        HttpResponse::BadRequest()
+        HttpResponse::BadRequest().finish()
     }
 }
 
@@ -45,19 +70,21 @@ pub async fn login<D: UserDao>(
             sub: user.id.to_string(),
             exp: (Utc::now() + Duration::days(5)).timestamp(),
         };
-        let token = encode(
+        let token = match encode(
             &Header::default(),
             &claims,
             &EncodingKey::from_secret(secret_key().as_bytes()),
-        )
+        ) {
-        .unwrap();
+            Ok(t) => t,
+            Err(e) => {
+                error!("Failed to encode JWT: {}", e);
+                return HttpResponse::InternalServerError().finish();
+            }
+        };
 
         HttpResponse::Ok().json(Token { token: &token })
     } else {
-        error!(
+        error!("Failed login attempt for user: '{}'", creds.username);
-            "User not found during login or incorrect password: '{}'",
-            creds.username
-        );
         HttpResponse::NotFound().finish()
     }
 }

src/data/mod.rs

@@ -50,7 +50,9 @@ impl FromStr for Claims {
     type Err = jsonwebtoken::errors::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let token = *(s.split("Bearer ").collect::<Vec<_>>().last().unwrap_or(&""));
+        let token = s.strip_prefix("Bearer ").ok_or_else(|| {
+            jsonwebtoken::errors::Error::from(jsonwebtoken::errors::ErrorKind::InvalidToken)
+        })?;
 
         match decode::<Claims>(
             token,
@@ -202,6 +204,7 @@ pub struct MetadataResponse {
     pub modified: Option<i64>,
     pub size: u64,
     pub exif: Option<ExifMetadata>,
+    pub filename_date: Option<i64>, // Date extracted from filename
 }
 
 impl From<fs::Metadata> for MetadataResponse {
@@ -217,6 +220,7 @@ impl From<fs::Metadata> for MetadataResponse {
             }),
             size: metadata.len(),
             exif: None,
+            filename_date: None, // Will be set in endpoint handler
         }
     }
 }
@@ -341,7 +345,7 @@ mod tests {
         };
 
         let c = Claims::from_str(
-            "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI5IiwiZXhwIjoxNjEzNjE2NDc5MH0.9wwK4l8vhvq55YoueEljMbN_5uVTaAsGLLRPr0AuymE")
+            "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI5IiwiZXhwIjoxNjEzNjE2NDc5MH0.9wwK4l8vhvq55YoueEljMbN_5uVTaAsGLLRPr0AuymE")
             .unwrap();
 
         assert_eq!(claims.sub, c.sub);
@@ -351,7 +355,7 @@ mod tests {
     #[test]
     fn test_expired_token() {
         let err = Claims::from_str(
-            "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI5IiwiZXhwIjoxNn0.eZnfaNfiD54VMbphIqeBICeG9SzAtwNXntLwtTBihjY",
+            "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI5IiwiZXhwIjoxNn0.eZnfaNfiD54VMbphIqeBICeG9SzAtwNXntLwtTBihjY",
         );
 
         match err.unwrap_err().into_kind() {

src/files.rs

@@ -855,7 +855,7 @@ mod tests {
             }
         }
 
-        fn move_file<P: AsRef<Path>>(&self, from: P, destination: P) -> anyhow::Result<()> {
+        fn move_file<P: AsRef<Path>>(&self, _from: P, _destination: P) -> anyhow::Result<()> {
             todo!()
         }
     }
@@ -997,7 +997,6 @@ mod tests {
             testhelpers::BodyReader,
         };
 
-        use crate::database::SqliteExifDao;
         use crate::database::test::in_memory_db_connection;
         use crate::tags::SqliteTagDao;
         use actix_web::test::TestRequest;

src/main.rs

@@ -21,6 +21,7 @@ use walkdir::{DirEntry, WalkDir};
 
 use actix_cors::Cors;
 use actix_files::NamedFile;
+use actix_governor::{Governor, GovernorConfigBuilder};
 use actix_multipart as mp;
 use actix_web::{
     App, HttpRequest, HttpResponse, HttpServer, Responder, delete, get, middleware, post, put,
@@ -171,6 +172,10 @@ async fn get_file_metadata(
         Ok(metadata) => {
             let mut response: MetadataResponse = metadata.into();
 
+            // Extract date from filename if possible
+            response.filename_date =
+                memories::extract_date_from_filename(&path.path).map(|dt| dt.timestamp());
+
             // Query EXIF data if available
             if let Ok(mut dao) = exif_dao.lock()
                 && let Ok(Some(exif)) = dao.get_exif(&span_context, &path.path)
@@ -760,10 +765,21 @@ fn main() -> std::io::Result<()> {
                 .supports_credentials()
                 .max_age(3600);
 
+            // Configure rate limiting for login endpoint (2 requests/sec, burst of 5)
+            let governor_conf = GovernorConfigBuilder::default()
+                .per_second(2)
+                .burst_size(5)
+                .finish()
+                .unwrap();
+
             App::new()
                 .wrap(middleware::Logger::default())
                 .wrap(cors)
-                .service(web::resource("/login").route(web::post().to(login::<SqliteUserDao>)))
+                .service(
+                    web::resource("/login")
+                        .wrap(Governor::new(&governor_conf))
+                        .route(web::post().to(login::<SqliteUserDao>)),
+                )
                 .service(
                     web::resource("/photos")
                         .route(web::get().to(files::list_photos::<SqliteTagDao, RealFileSystem>)),

src/memories.rs

@@ -1,9 +1,7 @@
 use actix_web::web::Data;
 use actix_web::{HttpRequest, HttpResponse, Responder, get, web};
 use chrono::LocalResult::{Ambiguous, Single};
-use chrono::{
+use chrono::{DateTime, Datelike, FixedOffset, Local, LocalResult, NaiveDate, TimeZone, Utc};
-    DateTime, Datelike, FixedOffset, Local, LocalResult, NaiveDate, TimeZone, Timelike, Utc,
-};
 use log::{debug, trace, warn};
 use opentelemetry::KeyValue;
 use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer};
@@ -123,63 +121,6 @@ pub struct MemoriesResponse {
     pub items: Vec<MemoryItem>,
 }
 
-fn get_file_date_info(
-    path: &Path,
-    client_timezone: &Option<FixedOffset>,
-) -> Option<(NaiveDate, Option<i64>, Option<i64>)> {
-    // Read file metadata once
-    let meta = std::fs::metadata(path).ok()?;
-
-    // Get created timestamp (tries filename first, then metadata)
-    let path_str = path.to_str()?;
-    let created = get_created_timestamp_with_fallback(path_str, &meta, client_timezone);
-
-    // Get modified timestamp from metadata
-    let modified = meta.modified().ok().map(|t| {
-        let utc: DateTime<Utc> = t.into();
-        if let Some(tz) = client_timezone {
-            utc.with_timezone(tz).timestamp()
-        } else {
-            utc.timestamp()
-        }
-    });
-
-    // Try to get date from filename for the NaiveDate
-    if let Some(date_time) = path
-        .file_name()
-        .and_then(|filename| filename.to_str())
-        .and_then(extract_date_from_filename)
-    {
-        // Convert to client timezone if specified
-        let date_in_timezone = if let Some(tz) = client_timezone {
-            date_time.with_timezone(tz)
-        } else {
-            date_time.with_timezone(&Local).fixed_offset()
-        };
-
-        debug!(
-            "File date from file {:?} > {:?} = {:?}",
-            path.file_name(),
-            date_time,
-            date_in_timezone
-        );
-        return Some((date_in_timezone.date_naive(), created, modified));
-    }
-
-    // Fall back to metadata if no date in filename
-    let system_time = meta.created().ok().or_else(|| meta.modified().ok())?;
-    let dt_utc: DateTime<Utc> = system_time.into();
-
-    let date_in_timezone = if let Some(tz) = client_timezone {
-        dt_utc.with_timezone(tz).date_naive()
-    } else {
-        dt_utc.with_timezone(&Local).date_naive()
-    };
-
-    trace!("Fallback metadata create date = {:?}", date_in_timezone);
-    Some((date_in_timezone, created, modified))
-}
-
 /// Convert Unix timestamp to NaiveDate in client timezone
 fn timestamp_to_naive_date(
     timestamp: i64,
@@ -196,37 +137,6 @@ fn timestamp_to_naive_date(
     Some(date)
 }
 
-/// Get created timestamp, trying filename parsing first, then falling back to metadata
-fn get_created_timestamp_with_fallback(
-    file_path: &str,
-    metadata: &std::fs::Metadata,
-    client_timezone: &Option<FixedOffset>,
-) -> Option<i64> {
-    // Try to extract date from filename first
-    if let Some(filename_date) = Path::new(file_path)
-        .file_name()
-        .and_then(|f| f.to_str())
-        .and_then(extract_date_from_filename)
-    {
-        let timestamp = if let Some(tz) = client_timezone {
-            filename_date.with_timezone(tz).timestamp()
-        } else {
-            filename_date.timestamp()
-        };
-        return Some(timestamp);
-    }
-
-    // Fall back to metadata
-    metadata.created().ok().map(|t| {
-        let utc: DateTime<Utc> = t.into();
-        if let Some(tz) = client_timezone {
-            utc.with_timezone(tz).timestamp()
-        } else {
-            utc.timestamp()
-        }
-    })
-}
-
 pub fn extract_date_from_filename(filename: &str) -> Option<DateTime<FixedOffset>> {
     let build_date_from_ymd_capture =
         |captures: &regex::Captures| -> Option<DateTime<FixedOffset>> {
@@ -327,6 +237,99 @@ pub fn extract_date_from_filename(filename: &str) -> Option<DateTime<FixedOffset
     None
 }
 
+/// Get the canonical date for a memory with priority: filename → EXIF → metadata
+/// Returns (NaiveDate for matching, timestamp for display, modified timestamp)
+fn get_memory_date_with_priority(
+    path: &Path,
+    exif_date_taken: Option<i64>,
+    client_timezone: &Option<FixedOffset>,
+) -> Option<(NaiveDate, Option<i64>, Option<i64>)> {
+    // Read file metadata once
+    let meta = std::fs::metadata(path).ok()?;
+
+    // Priority 1: Try to extract date from filename
+    if let Some(filename_date) = path
+        .file_name()
+        .and_then(|f| f.to_str())
+        .and_then(extract_date_from_filename)
+    {
+        // Convert to client timezone if specified
+        let date_in_timezone = if let Some(tz) = client_timezone {
+            filename_date.with_timezone(tz)
+        } else {
+            filename_date.with_timezone(&Local).fixed_offset()
+        };
+
+        let timestamp = if let Some(tz) = client_timezone {
+            filename_date.with_timezone(tz).timestamp()
+        } else {
+            filename_date.timestamp()
+        };
+
+        let modified = meta.modified().ok().map(|t| {
+            let utc: DateTime<Utc> = t.into();
+            if let Some(tz) = client_timezone {
+                utc.with_timezone(tz).timestamp()
+            } else {
+                utc.timestamp()
+            }
+        });
+
+        debug!(
+            "Memory date from filename {:?} > {:?} = {:?}",
+            path.file_name(),
+            filename_date,
+            date_in_timezone
+        );
+        return Some((date_in_timezone.date_naive(), Some(timestamp), modified));
+    }
+
+    // Priority 2: Use EXIF date_taken if available
+    if let Some(exif_timestamp) = exif_date_taken {
+        let date = timestamp_to_naive_date(exif_timestamp, client_timezone)?;
+
+        let modified = meta.modified().ok().map(|t| {
+            let utc: DateTime<Utc> = t.into();
+            if let Some(tz) = client_timezone {
+                utc.with_timezone(tz).timestamp()
+            } else {
+                utc.timestamp()
+            }
+        });
+
+        debug!("Memory date from EXIF {:?} = {:?}", path.file_name(), date);
+        return Some((date, Some(exif_timestamp), modified));
+    }
+
+    // Priority 3: Fall back to metadata
+    let system_time = meta.created().ok().or_else(|| meta.modified().ok())?;
+    let dt_utc: DateTime<Utc> = system_time.into();
+
+    let date_in_timezone = if let Some(tz) = client_timezone {
+        dt_utc.with_timezone(tz).date_naive()
+    } else {
+        dt_utc.with_timezone(&Local).date_naive()
+    };
+
+    let created_timestamp = if let Some(tz) = client_timezone {
+        dt_utc.with_timezone(tz).timestamp()
+    } else {
+        dt_utc.timestamp()
+    };
+
+    let modified = meta.modified().ok().map(|t| {
+        let utc: DateTime<Utc> = t.into();
+        if let Some(tz) = client_timezone {
+            utc.with_timezone(tz).timestamp()
+        } else {
+            utc.timestamp()
+        }
+    });
+
+    trace!("Fallback metadata create date = {:?}", date_in_timezone);
+    Some((date_in_timezone, Some(created_timestamp), modified))
+}
+
 /// Collect memories from EXIF database
 fn collect_exif_memories(
     exif_dao: &Data<Mutex<Box<dyn ExifDao>>>,
@@ -371,27 +374,16 @@ fn collect_exif_memories(
                 return None;
             }
 
-            // Convert timestamp to NaiveDate in client timezone
+            // Get date with priority: filename → EXIF → metadata
-            let file_date = timestamp_to_naive_date(*date_taken_ts, client_timezone)?;
+            // This ensures sorting and display use the same date source
+            let (file_date, created, modified) =
+                get_memory_date_with_priority(&full_path, Some(*date_taken_ts), client_timezone)?;
 
             // Check if matches memory criteria
             if !is_memories_match(file_path, file_date, now, span_mode, years_back) {
                 return None;
             }
 
-            // Get file metadata for created/modified timestamps
-            let metadata = std::fs::metadata(&full_path).ok()?;
-            let created =
-                get_created_timestamp_with_fallback(file_path, &metadata, client_timezone);
-            let modified = metadata.modified().ok().map(|t| {
-                let utc: DateTime<Utc> = t.into();
-                if let Some(tz) = client_timezone {
-                    utc.with_timezone(tz).timestamp()
-                } else {
-                    utc.timestamp()
-                }
-            });
-
             Some((
                 MemoryItem {
                     path: file_path.clone(),
@@ -440,8 +432,9 @@ fn collect_filesystem_memories(
     entries
         .par_iter()
         .filter_map(|entry| {
-            // Use existing get_file_date_info() for filename/metadata fallback
+            // Use unified date priority function (no EXIF for filesystem scan)
-            let (file_date, created, modified) = get_file_date_info(entry.path(), client_timezone)?;
+            let (file_date, created, modified) =
+                get_memory_date_with_priority(entry.path(), None, client_timezone)?;
 
             if is_memories_match(
                 entry.path().to_str().unwrap_or("Unknown"),
@@ -546,7 +539,7 @@ pub async fn list_memories(
     match span_mode {
         // Sort by absolute time for a more 'overview'
         MemoriesSpan::Month => memories_with_dates.sort_by(|a, b| a.1.cmp(&b.1)),
-        // For week span, sort by day of month, then time of day, then year (oldest first)
+        // For week span, sort by day of month, then by full timestamp (oldest first)
         MemoriesSpan::Week => {
             memories_with_dates.sort_by(|a, b| {
                 // First, sort by day of month
@@ -555,45 +548,12 @@ pub async fn list_memories(
                     return day_cmp;
                 }
 
-                // Then sort by time of day
+                // Then sort by full created timestamp (oldest to newest)
                 match (a.0.created, b.0.created) {
-                    (Some(a_time), Some(b_time)) => {
+                    (Some(a_time), Some(b_time)) => a_time.cmp(&b_time),
-                        // Convert timestamps to DateTime
-                        let a_dt_utc = DateTime::<Utc>::from_timestamp(a_time, 0).unwrap();
-                        let b_dt_utc = DateTime::<Utc>::from_timestamp(b_time, 0).unwrap();
-
-                        // Extract time of day in the appropriate timezone
-                        let a_time_of_day = if let Some(ref tz) = client_timezone {
-                            let dt = a_dt_utc.with_timezone(tz);
-                            (dt.hour(), dt.minute(), dt.second())
-                        } else {
-                            let dt = a_dt_utc.with_timezone(&Local);
-                            (dt.hour(), dt.minute(), dt.second())
-                        };
-
-                        let b_time_of_day = if let Some(ref tz) = client_timezone {
-                            let dt = b_dt_utc.with_timezone(tz);
-                            (dt.hour(), dt.minute(), dt.second())
-                        } else {
-                            let dt = b_dt_utc.with_timezone(&Local);
-                            (dt.hour(), dt.minute(), dt.second())
-                        };
-
-                        // Compare time of day
-                        let time_cmp = a_time_of_day.cmp(&b_time_of_day);
-                        if time_cmp != std::cmp::Ordering::Equal {
-                            return time_cmp;
-                        }
-
-                        // Finally, sort by year (oldest first)
-                        a.1.year().cmp(&b.1.year())
-                    }
                     (Some(_), None) => std::cmp::Ordering::Less,
                     (None, Some(_)) => std::cmp::Ordering::Greater,
-                    (None, None) => {
+                    (None, None) => std::cmp::Ordering::Equal,
-                        // If no timestamps, just sort by year (oldest first)
-                        a.1.year().cmp(&b.1.year())
-                    }
                 }
             });
         }
@@ -793,15 +753,24 @@ mod tests {
     }
 
     #[test]
-    fn test_get_file_date_info_from_filename() {
+    fn test_memory_date_priority_filename() {
         let temp_dir = tempdir().unwrap();
         let temp_file = temp_dir.path().join("Screenshot_2014-06-01-20-44-50.png");
         File::create(&temp_file).unwrap();
 
-        let (date, created, _) =
+        // Test that filename takes priority (even with EXIF data available)
-            get_file_date_info(&temp_file, &Some(*Local::now().fixed_offset().offset())).unwrap();
+        let exif_date = DateTime::<Utc>::from_timestamp(1609459200, 0) // 2021-01-01
+            .unwrap()
+            .timestamp();
 
-        // Check that date is from filename
+        let (date, created, _) = get_memory_date_with_priority(
+            &temp_file,
+            Some(exif_date),
+            &Some(*Local::now().fixed_offset().offset()),
+        )
+        .unwrap();
+
+        // Check that date is from filename (2014), NOT EXIF (2021)
         assert_eq!(date.year(), 2014);
         assert_eq!(date.month(), 6);
         assert_eq!(date.day(), 1);
@@ -820,12 +789,14 @@ mod tests {
     }
 
     #[test]
-    fn test_get_file_date_info_from_metadata() {
+    fn test_memory_date_priority_metadata_fallback() {
         let temp_dir = tempdir().unwrap();
         let temp_file = temp_dir.path().join("regular_image.jpg");
         File::create(&temp_file).unwrap();
 
-        let (date, created, modified) = get_file_date_info(&temp_file, &None).unwrap();
+        // Test metadata fallback when no filename date or EXIF
+        let (date, created, modified) =
+            get_memory_date_with_priority(&temp_file, None, &None).unwrap();
 
         // Both date and timestamps should be from metadata (recent)
         let today = Local::now().date_naive();
@@ -844,6 +815,37 @@ mod tests {
         assert_eq!(dt_modified.year(), today.year());
     }
 
+    #[test]
+    fn test_memory_date_priority_exif_over_metadata() {
+        let temp_dir = tempdir().unwrap();
+        let temp_file = temp_dir.path().join("regular_image.jpg");
+        File::create(&temp_file).unwrap();
+
+        // Test that EXIF takes priority over metadata (but not filename)
+        // EXIF date: June 15, 2020 12:00:00 UTC (safe from timezone edge cases)
+        let exif_date = DateTime::<Utc>::from_timestamp(1592222400, 0) // 2020-06-15 12:00:00 UTC
+            .unwrap()
+            .timestamp();
+
+        let (date, created, modified) =
+            get_memory_date_with_priority(&temp_file, Some(exif_date), &None).unwrap();
+
+        // Date should be from EXIF (2020), not metadata (today)
+        assert_eq!(date.year(), 2020);
+        assert_eq!(date.month(), 6);
+        assert_eq!(date.day(), 15);
+
+        // Created timestamp should also be from EXIF
+        assert!(created.is_some());
+        assert_eq!(created.unwrap(), exif_date);
+
+        // Modified should still be from metadata
+        assert!(modified.is_some());
+        let today = Local::now().date_naive();
+        let dt_modified = DateTime::<Utc>::from_timestamp(modified.unwrap(), 0).unwrap();
+        assert_eq!(dt_modified.year(), today.year());
+    }
+
     #[test]
     fn test_path_excluder_absolute_under_base() {
         let tmp = tempdir().unwrap();
@@ -980,4 +982,90 @@ mod tests {
         // keep.jpg doesn't match any rule
         assert!(!excluder.is_excluded(&keep));
     }
+
+    #[test]
+    fn test_week_span_sorting_chronological_by_day() {
+        // Test that Week span sorts by:
+        // 1. Day of month (ascending)
+        // 2. Full timestamp oldest to newest (year + time combined)
+
+        // Create test data:
+        // - Jan 15, 2024 at 9:00 AM
+        // - Jan 15, 2020 at 10:00 AM
+        // - Jan 16, 2021 at 8:00 AM
+
+        let jan_15_2024_9am = NaiveDate::from_ymd_opt(2024, 1, 15)
+            .unwrap()
+            .and_hms_opt(9, 0, 0)
+            .unwrap()
+            .and_utc()
+            .timestamp();
+
+        let jan_15_2020_10am = NaiveDate::from_ymd_opt(2020, 1, 15)
+            .unwrap()
+            .and_hms_opt(10, 0, 0)
+            .unwrap()
+            .and_utc()
+            .timestamp();
+
+        let jan_16_2021_8am = NaiveDate::from_ymd_opt(2021, 1, 16)
+            .unwrap()
+            .and_hms_opt(8, 0, 0)
+            .unwrap()
+            .and_utc()
+            .timestamp();
+
+        let mut memories_with_dates = vec![
+            (
+                MemoryItem {
+                    path: "photo1.jpg".to_string(),
+                    created: Some(jan_15_2024_9am),
+                    modified: Some(jan_15_2024_9am),
+                },
+                NaiveDate::from_ymd_opt(2024, 1, 15).unwrap(),
+            ),
+            (
+                MemoryItem {
+                    path: "photo2.jpg".to_string(),
+                    created: Some(jan_15_2020_10am),
+                    modified: Some(jan_15_2020_10am),
+                },
+                NaiveDate::from_ymd_opt(2020, 1, 15).unwrap(),
+            ),
+            (
+                MemoryItem {
+                    path: "photo3.jpg".to_string(),
+                    created: Some(jan_16_2021_8am),
+                    modified: Some(jan_16_2021_8am),
+                },
+                NaiveDate::from_ymd_opt(2021, 1, 16).unwrap(),
+            ),
+        ];
+
+        // Sort using Week span logic
+        memories_with_dates.sort_by(|a, b| {
+            // First, sort by day of month
+            let day_cmp = a.1.day().cmp(&b.1.day());
+            if day_cmp != std::cmp::Ordering::Equal {
+                return day_cmp;
+            }
+
+            // Then sort by full created timestamp (oldest to newest)
+            match (a.0.created, b.0.created) {
+                (Some(a_time), Some(b_time)) => a_time.cmp(&b_time),
+                (Some(_), None) => std::cmp::Ordering::Less,
+                (None, Some(_)) => std::cmp::Ordering::Greater,
+                (None, None) => std::cmp::Ordering::Equal,
+            }
+        });
+
+        // Expected order:
+        // 1. Jan 15, 2020 at 10:00 AM (oldest Jan 15 photo)
+        // 2. Jan 15, 2024 at 9:00 AM (newer Jan 15 photo)
+        // 3. Jan 16, 2021 at 8:00 AM (all Jan 16 photos after Jan 15)
+
+        assert_eq!(memories_with_dates[0].0.created.unwrap(), jan_15_2020_10am);
+        assert_eq!(memories_with_dates[1].0.created.unwrap(), jan_15_2024_9am);
+        assert_eq!(memories_with_dates[2].0.created.unwrap(), jan_16_2021_8am);
+    }
 }

src/tags.rs

@@ -918,7 +918,7 @@ mod tests {
     }
 }
 #[derive(QueryableByName, Debug, Clone)]
-pub(crate) struct FileWithTagCount {
+pub struct FileWithTagCount {
     #[diesel(sql_type = Text)]
     pub(crate) file_name: String,
     #[diesel(sql_type = BigInt)]