Implement critical security improvements for authentication #45

Merged
cameron merged 6 commits from feature/security-improvements into master 2026-01-02 22:01:40 +00:00
8 changed files with 434 additions and 174 deletions

129
Cargo.lock generated
View File

@@ -82,6 +82,18 @@ dependencies = [
"v_htmlescape", "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]] [[package]]
name = "actix-http" name = "actix-http"
version = "3.11.1" version = "3.11.1"
@@ -874,6 +886,19 @@ dependencies = [
"syn", "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]] [[package]]
name = "deranged" name = "deranged"
version = "0.5.3" version = "0.5.3"
@@ -1218,6 +1243,12 @@ version = "0.3.31"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988"
[[package]]
name = "futures-timer"
version = "3.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24"
[[package]] [[package]]
name = "futures-util" name = "futures-util"
version = "0.3.31" version = "0.3.31"
@@ -1277,6 +1308,26 @@ version = "0.31.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" 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]] [[package]]
name = "h2" name = "h2"
version = "0.3.27" version = "0.3.27"
@@ -1315,6 +1366,12 @@ dependencies = [
"tracing", "tracing",
] ]
[[package]]
name = "hashbrown"
version = "0.14.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1"
[[package]] [[package]]
name = "hashbrown" name = "hashbrown"
version = "0.15.5" version = "0.15.5"
@@ -1605,11 +1662,12 @@ dependencies = [
[[package]] [[package]]
name = "image-api" name = "image-api"
version = "0.4.0" version = "0.4.1"
dependencies = [ dependencies = [
"actix", "actix",
"actix-cors", "actix-cors",
"actix-files", "actix-files",
"actix-governor",
"actix-multipart", "actix-multipart",
"actix-rt", "actix-rt",
"actix-web", "actix-web",
@@ -1666,7 +1724,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f2481980430f9f78649238835720ddccc57e52df14ffce1c6f37391d61b563e9" checksum = "f2481980430f9f78649238835720ddccc57e52df14ffce1c6f37391d61b563e9"
dependencies = [ dependencies = [
"equivalent", "equivalent",
"hashbrown", "hashbrown 0.15.5",
] ]
[[package]] [[package]]
@@ -2018,6 +2076,12 @@ version = "1.0.6"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086" checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086"
[[package]]
name = "no-std-compat"
version = "0.4.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b93853da6d84c2e3c7d730d6473e8817692dd89be387eb01b94d7f108ecb5b8c"
[[package]] [[package]]
name = "nom" name = "nom"
version = "7.1.3" version = "7.1.3"
@@ -2028,6 +2092,12 @@ dependencies = [
"minimal-lexical", "minimal-lexical",
] ]
[[package]]
name = "nonzero_ext"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "38bf9645c8b145698bb0b18a4637dcacbc421ea49bef2317e4fd8065a387cf21"
[[package]] [[package]]
name = "noop_proc_macro" name = "noop_proc_macro"
version = "0.3.0" version = "0.3.0"
@@ -2448,6 +2518,21 @@ dependencies = [
"num-traits", "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]] [[package]]
name = "quick-error" name = "quick-error"
version = "2.0.1" version = "2.0.1"
@@ -2578,6 +2663,15 @@ dependencies = [
"rgb", "rgb",
] ]
[[package]]
name = "raw-cpuid"
version = "11.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "498cd0dc59d73224351ee52a95fee0f1a617a2eae0e7d9d720cc622c73a54186"
dependencies = [
"bitflags",
]
[[package]] [[package]]
name = "rayon" name = "rayon"
version = "1.11.0" version = "1.11.0"
@@ -2919,6 +3013,15 @@ dependencies = [
"windows-sys 0.59.0", "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]] [[package]]
name = "stable_deref_trait" name = "stable_deref_trait"
version = "1.2.0" version = "1.2.0"
@@ -3546,6 +3649,22 @@ dependencies = [
"wasm-bindgen", "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]] [[package]]
name = "winapi-util" name = "winapi-util"
version = "0.1.10" version = "0.1.10"
@@ -3555,6 +3674,12 @@ dependencies = [
"windows-sys 0.60.2", "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]] [[package]]
name = "windows-core" name = "windows-core"
version = "0.61.2" version = "0.61.2"

View File

@@ -1,6 +1,6 @@
[package] [package]
name = "image-api" name = "image-api"
version = "0.4.0" version = "0.4.1"
authors = ["Cameron Cordes <cameronc.dev@gmail.com>"] authors = ["Cameron Cordes <cameronc.dev@gmail.com>"]
edition = "2024" edition = "2024"
@@ -17,6 +17,7 @@ tokio = { version = "1.42.0", features = ["default", "process", "sync"] }
actix-files = "0.6" actix-files = "0.6"
actix-cors = "0.7" actix-cors = "0.7"
actix-multipart = "0.7.2" actix-multipart = "0.7.2"
actix-governor = "0.5"
futures = "0.3.5" futures = "0.3.5"
jsonwebtoken = "9.3.0" jsonwebtoken = "9.3.0"
serde = "1" serde = "1"
@@ -47,4 +48,4 @@ opentelemetry-stdout = "0.31.0"
opentelemetry-appender-log = "0.31.0" opentelemetry-appender-log = "0.31.0"
tempfile = "3.20.0" tempfile = "3.20.0"
regex = "1.11.1" regex = "1.11.1"
exif = { package = "kamadak-exif", version = "0.6.1" } exif = { package = "kamadak-exif", version = "0.6.1" }

View File

@@ -13,22 +13,47 @@ use crate::{
database::UserDao, 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)] #[allow(dead_code)]
async fn register<D: UserDao>( async fn register<D: UserDao>(
user: Json<CreateAccountRequest>, user: Json<CreateAccountRequest>,
user_dao: web::Data<Mutex<D>>, user_dao: web::Data<Mutex<D>>,
) -> impl Responder { ) -> 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"); let mut dao = user_dao.lock().expect("Unable to get UserDao");
if dao.user_exists(&user.username) { if dao.user_exists(&user.username) {
HttpResponse::BadRequest() HttpResponse::BadRequest().finish()
} else if let Some(_user) = dao.create_user(&user.username, &user.password) { } else if let Some(_user) = dao.create_user(&user.username, &user.password) {
HttpResponse::Ok() HttpResponse::Ok().finish()
} else { } else {
HttpResponse::InternalServerError() HttpResponse::InternalServerError().finish()
} }
} else { } else {
HttpResponse::BadRequest() HttpResponse::BadRequest().finish()
} }
} }
@@ -45,19 +70,21 @@ pub async fn login<D: UserDao>(
sub: user.id.to_string(), sub: user.id.to_string(),
exp: (Utc::now() + Duration::days(5)).timestamp(), exp: (Utc::now() + Duration::days(5)).timestamp(),
}; };
let token = encode( let token = match encode(
&Header::default(), &Header::default(),
&claims, &claims,
&EncodingKey::from_secret(secret_key().as_bytes()), &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 }) HttpResponse::Ok().json(Token { token: &token })
} else { } else {
error!( error!("Failed login attempt for user: '{}'", creds.username);
"User not found during login or incorrect password: '{}'",
creds.username
);
HttpResponse::NotFound().finish() HttpResponse::NotFound().finish()
} }
} }

View File

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

View File

@@ -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!() todo!()
} }
} }
@@ -997,7 +997,6 @@ mod tests {
testhelpers::BodyReader, testhelpers::BodyReader,
}; };
use crate::database::SqliteExifDao;
use crate::database::test::in_memory_db_connection; use crate::database::test::in_memory_db_connection;
use crate::tags::SqliteTagDao; use crate::tags::SqliteTagDao;
use actix_web::test::TestRequest; use actix_web::test::TestRequest;

View File

@@ -21,6 +21,7 @@ use walkdir::{DirEntry, WalkDir};
use actix_cors::Cors; use actix_cors::Cors;
use actix_files::NamedFile; use actix_files::NamedFile;
use actix_governor::{Governor, GovernorConfigBuilder};
use actix_multipart as mp; use actix_multipart as mp;
use actix_web::{ use actix_web::{
App, HttpRequest, HttpResponse, HttpServer, Responder, delete, get, middleware, post, put, App, HttpRequest, HttpResponse, HttpServer, Responder, delete, get, middleware, post, put,
@@ -171,6 +172,10 @@ async fn get_file_metadata(
Ok(metadata) => { Ok(metadata) => {
let mut response: MetadataResponse = metadata.into(); 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 // Query EXIF data if available
if let Ok(mut dao) = exif_dao.lock() if let Ok(mut dao) = exif_dao.lock()
&& let Ok(Some(exif)) = dao.get_exif(&span_context, &path.path) && let Ok(Some(exif)) = dao.get_exif(&span_context, &path.path)
@@ -760,10 +765,21 @@ fn main() -> std::io::Result<()> {
.supports_credentials() .supports_credentials()
.max_age(3600); .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() App::new()
.wrap(middleware::Logger::default()) .wrap(middleware::Logger::default())
.wrap(cors) .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( .service(
web::resource("/photos") web::resource("/photos")
.route(web::get().to(files::list_photos::<SqliteTagDao, RealFileSystem>)), .route(web::get().to(files::list_photos::<SqliteTagDao, RealFileSystem>)),

View File

@@ -1,9 +1,7 @@
use actix_web::web::Data; use actix_web::web::Data;
use actix_web::{HttpRequest, HttpResponse, Responder, get, web}; use actix_web::{HttpRequest, HttpResponse, Responder, get, web};
use chrono::LocalResult::{Ambiguous, Single}; 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 log::{debug, trace, warn};
use opentelemetry::KeyValue; use opentelemetry::KeyValue;
use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer}; use opentelemetry::trace::{Span, Status, TraceContextExt, Tracer};
@@ -123,63 +121,6 @@ pub struct MemoriesResponse {
pub items: Vec<MemoryItem>, 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 /// Convert Unix timestamp to NaiveDate in client timezone
fn timestamp_to_naive_date( fn timestamp_to_naive_date(
timestamp: i64, timestamp: i64,
@@ -196,37 +137,6 @@ fn timestamp_to_naive_date(
Some(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>> { pub fn extract_date_from_filename(filename: &str) -> Option<DateTime<FixedOffset>> {
let build_date_from_ymd_capture = let build_date_from_ymd_capture =
|captures: &regex::Captures| -> Option<DateTime<FixedOffset>> { |captures: &regex::Captures| -> Option<DateTime<FixedOffset>> {
@@ -327,6 +237,99 @@ pub fn extract_date_from_filename(filename: &str) -> Option<DateTime<FixedOffset
None 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 /// Collect memories from EXIF database
fn collect_exif_memories( fn collect_exif_memories(
exif_dao: &Data<Mutex<Box<dyn ExifDao>>>, exif_dao: &Data<Mutex<Box<dyn ExifDao>>>,
@@ -371,27 +374,16 @@ fn collect_exif_memories(
return None; 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 // Check if matches memory criteria
if !is_memories_match(file_path, file_date, now, span_mode, years_back) { if !is_memories_match(file_path, file_date, now, span_mode, years_back) {
return None; 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(( Some((
MemoryItem { MemoryItem {
path: file_path.clone(), path: file_path.clone(),
@@ -440,8 +432,9 @@ fn collect_filesystem_memories(
entries entries
.par_iter() .par_iter()
.filter_map(|entry| { .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( if is_memories_match(
entry.path().to_str().unwrap_or("Unknown"), entry.path().to_str().unwrap_or("Unknown"),
@@ -546,7 +539,7 @@ pub async fn list_memories(
match span_mode { match span_mode {
// Sort by absolute time for a more 'overview' // Sort by absolute time for a more 'overview'
MemoriesSpan::Month => memories_with_dates.sort_by(|a, b| a.1.cmp(&b.1)), 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 => { MemoriesSpan::Week => {
memories_with_dates.sort_by(|a, b| { memories_with_dates.sort_by(|a, b| {
// First, sort by day of month // First, sort by day of month
@@ -555,45 +548,12 @@ pub async fn list_memories(
return day_cmp; 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) { 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, (Some(_), None) => std::cmp::Ordering::Less,
(None, Some(_)) => std::cmp::Ordering::Greater, (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] #[test]
fn test_get_file_date_info_from_filename() { fn test_memory_date_priority_filename() {
let temp_dir = tempdir().unwrap(); let temp_dir = tempdir().unwrap();
let temp_file = temp_dir.path().join("Screenshot_2014-06-01-20-44-50.png"); let temp_file = temp_dir.path().join("Screenshot_2014-06-01-20-44-50.png");
File::create(&temp_file).unwrap(); 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.year(), 2014);
assert_eq!(date.month(), 6); assert_eq!(date.month(), 6);
assert_eq!(date.day(), 1); assert_eq!(date.day(), 1);
@@ -820,12 +789,14 @@ mod tests {
} }
#[test] #[test]
fn test_get_file_date_info_from_metadata() { fn test_memory_date_priority_metadata_fallback() {
let temp_dir = tempdir().unwrap(); let temp_dir = tempdir().unwrap();
let temp_file = temp_dir.path().join("regular_image.jpg"); let temp_file = temp_dir.path().join("regular_image.jpg");
File::create(&temp_file).unwrap(); 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) // Both date and timestamps should be from metadata (recent)
let today = Local::now().date_naive(); let today = Local::now().date_naive();
@@ -844,6 +815,37 @@ mod tests {
assert_eq!(dt_modified.year(), today.year()); 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] #[test]
fn test_path_excluder_absolute_under_base() { fn test_path_excluder_absolute_under_base() {
let tmp = tempdir().unwrap(); let tmp = tempdir().unwrap();
@@ -980,4 +982,90 @@ mod tests {
// keep.jpg doesn't match any rule // keep.jpg doesn't match any rule
assert!(!excluder.is_excluded(&keep)); 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);
}
} }

View File

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