diff --git a/frontend/src/api.js b/frontend/src/api.js index aee167c..938b4dd 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -73,9 +73,15 @@ export function postDownload(url, ids = '') { form.remove() } -// SHA-256 of a File, matching the server's content hash — used to skip -// uploading bytes the album already has. +// Above this, hashing whole-file in memory (WebCrypto has no streaming digest) +// risks OOM / the ~2GiB ArrayBuffer cap, so we skip the client dedup check and +// just upload — the server still dedups on arrival. +const CLIENT_HASH_LIMIT = 512 * 1024 * 1024 + +// SHA-256 of a File (lowercase hex), matching the server's content hash, or +// null when the file is too large to hash safely in the browser. export async function sha256Hex(file) { + if (file.size > CLIENT_HASH_LIMIT) return null const digest = await crypto.subtle.digest('SHA-256', await file.arrayBuffer()) return [...new Uint8Array(digest)].map((b) => b.toString(16).padStart(2, '0')).join('') } diff --git a/frontend/src/components/Gallery.jsx b/frontend/src/components/Gallery.jsx index 0f9ba4c..f51be40 100644 --- a/frontend/src/components/Gallery.jsx +++ b/frontend/src/components/Gallery.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react' +import { useCallback, useRef, useState } from 'react' import { imgUrl } from '../api' // True justified layout: pack photos greedily into rows at their real aspect @@ -32,15 +32,19 @@ function layoutRows(photos, containerWidth, targetHeight, gap) { } export default function Gallery({ photos, onOpen, overlay, selected, onToggleSelect }) { - const containerRef = useRef(null) const [width, setWidth] = useState(0) + const observerRef = useRef(null) - useEffect(() => { - const el = containerRef.current - if (!el) return + // Callback ref: (re)attaches the observer whenever the container node + // mounts. A plain mount-effect misses the case where Gallery first renders + // empty (no container) and photos arrive later. + const containerRef = useCallback((node) => { + observerRef.current?.disconnect() + if (!node) return const observer = new ResizeObserver((entries) => setWidth(entries[0].contentRect.width)) - observer.observe(el) - return () => observer.disconnect() + observer.observe(node) + observerRef.current = observer + setWidth(node.getBoundingClientRect().width) }, []) if (photos.length === 0) return null diff --git a/frontend/src/pages/AlbumPage.jsx b/frontend/src/pages/AlbumPage.jsx index 816b742..94d3329 100644 --- a/frontend/src/pages/AlbumPage.jsx +++ b/frontend/src/pages/AlbumPage.jsx @@ -71,13 +71,16 @@ function UploadZone({ albumId, onUploaded }) { setQueue((q) => q.map((x) => (x.key === item.key ? { ...x, ...patch } : x))) const transfer = async () => { // Hash locally first: content the album already has is skipped - // without transferring a single byte. + // without transferring a single byte. Files too large to hash in the + // browser (null) fall straight through to a normal upload. update({ status: 'checking' }) try { const hash = await sha256Hex(item.file) - await api(`/api/albums/${albumId}/photos/by-hash/${hash}`) - update({ status: 'skipped', progress: 1 }) - return + if (hash) { + await api(`/api/albums/${albumId}/photos/by-hash/${hash}`) + update({ status: 'skipped', progress: 1 }) + return + } } catch { // 404 (not there yet) or hashing unavailable — upload normally. } diff --git a/migrations/0003_multi_tenant.sql b/migrations/0003_multi_tenant.sql index 6b0ff01..4d3868f 100644 --- a/migrations/0003_multi_tenant.sql +++ b/migrations/0003_multi_tenant.sql @@ -2,8 +2,25 @@ -- all hang off albums, so this single column scopes everything. alter table albums add column owner_id uuid references users(id); --- Existing albums belong to the instance's original photographer. -update albums set owner_id = (select id from users order by created_at limit 1); +-- Backfill: pre-tenancy albums had no owner. Assigning them to "the" original +-- photographer is only unambiguous when exactly one user exists. With several +-- (v0.1.0 let every allowed email share all albums) the correct owner is +-- unknowable, so refuse rather than silently transfer everyone's work to one +-- account — the operator must assign ownership manually before migrating. +do $$ +declare + n_users int; + n_albums int; +begin + select count(*) into n_users from users; + select count(*) into n_albums from albums; + if n_albums > 0 and n_users <> 1 then + raise exception + 'multi-tenant migration: % albums exist but there are % users (need exactly 1 to auto-assign ownership); set albums.owner_id manually first', + n_albums, n_users; + end if; + update albums set owner_id = (select id from users order by created_at limit 1); +end $$; alter table albums alter column owner_id set not null; diff --git a/src/auth.rs b/src/auth.rs index eb416ff..1fe2b17 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -114,11 +114,17 @@ pub(crate) fn base_cookie(name: &'static str, value: String, secure: bool) -> Co .build() } +/// The signed session-cookie payload, in one place so tests and the +/// mint_session dev tool can't drift from what user_from_jar parses. +pub fn session_payload(user_id: Uuid, email: &str, exp: i64) -> String { + format!("{user_id}|{exp}|{email}") +} + fn session_cookie(state: &AppState, user_id: Uuid, email: &str) -> Cookie<'static> { let exp = Utc::now().timestamp() + SESSION_DAYS * 86400; let mut cookie = base_cookie( SESSION_COOKIE, - format!("{user_id}|{exp}|{email}"), + session_payload(user_id, email, exp), state.config.cookie_secure(), ); cookie.set_max_age(time::Duration::days(SESSION_DAYS)); diff --git a/src/routes/images.rs b/src/routes/images.rs index e2bacea..6e20a77 100644 --- a/src/routes/images.rs +++ b/src/routes/images.rs @@ -28,8 +28,19 @@ async fn authorize_photo( .await?; let photo = photo.ok_or_else(ApiError::not_found)?; - if user_from_jar(state, jar).is_some() { - return Ok(photo); + // Photographers may only reach their OWN photos (scoped by album owner); + // otherwise fall through to share-cookie access. + if let Some(user) = user_from_jar(state, jar) { + let owns: Option<(Uuid,)> = sqlx::query_as( + "select a.id from albums a where a.id = $1 and a.owner_id = $2", + ) + .bind(photo.album_id) + .bind(user.id) + .fetch_optional(&state.db) + .await?; + if owns.is_some() { + return Ok(photo); + } } authorize_album_via_cookie(state, jar, photo.album_id, need_download).await?; // Clients may only reach photos the share listing exposes. diff --git a/src/routes/photos.rs b/src/routes/photos.rs index d6f5524..ee6af33 100644 --- a/src/routes/photos.rs +++ b/src/routes/photos.rs @@ -41,6 +41,25 @@ fn sanitize_filename(raw: &str) -> Result { Ok(cleaned.chars().take(150).collect()) } +/// A dedup hit on a photo that previously failed processing means the user is +/// re-uploading to fix it — reset it and re-enqueue instead of handing back a +/// broken row that the UI would report as "already uploaded". +async fn heal_if_errored(state: &AppState, photo: Photo) -> ApiResult { + if photo.status != PhotoStatus::Error { + return Ok(photo); + } + let mut tx = state.db.begin().await?; + let healed: Photo = + sqlx::query_as("update photos set status = $2, error = null where id = $1 returning *") + .bind(photo.id) + .bind(PhotoStatus::Uploaded.as_str()) + .fetch_one(&mut *tx) + .await?; + jobs::ensure_process_photo(&mut tx, photo.id).await?; + tx.commit().await?; + Ok(healed) +} + pub async fn upload( State(state): State, user: AuthUser, @@ -94,7 +113,7 @@ pub async fn upload( .fetch_optional(&state.db) .await?; if let Some(existing) = existing { - return Ok(Json(existing)); + return Ok(Json(heal_if_errored(&state, existing).await?)); } // Upload to S3 first, then create the row and enqueue processing in one @@ -153,7 +172,7 @@ pub async fn upload( .fetch_optional(&state.db) .await?; if let Some(winner) = winner { - return Ok(Json(winner)); + return Ok(Json(heal_if_errored(&state, winner).await?)); } } return Err(e.into()); diff --git a/src/routes/zip.rs b/src/routes/zip.rs index 8e654f0..aed5276 100644 --- a/src/routes/zip.rs +++ b/src/routes/zip.rs @@ -137,9 +137,6 @@ struct Entry { offset: u64, dos_time: u16, dos_date: u16, - /// Stored at upload/processing time; photos from before hashes existed - /// have None and take the slower spool path (which backfills it). - crc: Option, } struct ZipPlan { @@ -180,7 +177,6 @@ fn plan_zip(photos: &[Photo]) -> Result { offset: entry_offset, dos_time, dos_date, - crc: photo.crc32.map(|v| v as u32), }); } let cd_offset = offset; @@ -282,23 +278,26 @@ fn stream_zip(state: AppState, photos: Vec, album_name: &str) -> ApiResul .map_err(|e| anyhow::anyhow!("building response: {e}").into()) } -enum Fetched { - /// CRC already known — the body streams straight into the response. - Direct(Box), - /// Pre-hash photo: spooled to a temp file to compute the CRC first. - Spooled(tokio::fs::File, u32), +struct Fetched { + file: tokio::fs::File, + crc: u32, + /// sha256 hex — always computed so legacy photos (crc-only) get their + /// content hash backfilled, restoring upload dedup for them. + sha256: String, } -/// Start fetching an original. With a known CRC this only opens the S3 -/// response (the body is consumed later, straight into the zip stream); -/// otherwise the object is spooled to an anonymous temp file to compute the -/// CRC, verifying the byte count the zip plan promised. +/// Fetch an original and spool it to an anonymous temp file, computing crc32 +/// and sha256 and verifying the byte count the zip plan promised. Spooling +/// (rather than streaming the live S3 body straight through) is deliberate: +/// the prefetched entry is fully read immediately, so no S3 connection sits +/// idle across the previous entry's (possibly slow) client stream — which S3 +/// idle-timeouts would otherwise reset mid-download. fn fetch_entry( state: &AppState, key: String, expected_size: u64, - crc_known: bool, ) -> JoinHandle> { + use sha2::Digest; let state = state.clone(); tokio::spawn(async move { let object = state @@ -309,12 +308,10 @@ fn fetch_entry( .send() .await .map_err(|e| anyhow::anyhow!("fetching {key}: {e}"))?; - if crc_known { - return Ok(Fetched::Direct(Box::new(object))); - } let mut file = tokio::fs::File::from_std(tempfile::tempfile()?); let mut reader = object.body.into_async_read(); - let mut hasher = crc32fast::Hasher::new(); + let mut crc = crc32fast::Hasher::new(); + let mut sha = sha2::Sha256::new(); let mut written: u64 = 0; let mut buf = vec![0u8; 128 * 1024]; loop { @@ -322,7 +319,8 @@ fn fetch_entry( if n == 0 { break; } - hasher.update(&buf[..n]); + crc.update(&buf[..n]); + sha.update(&buf[..n]); file.write_all(&buf[..n]).await?; written += n as u64; } @@ -332,7 +330,11 @@ fn fetch_entry( ); file.flush().await?; file.seek(std::io::SeekFrom::Start(0)).await?; - Ok(Fetched::Spooled(file, hasher.finalize())) + Ok(Fetched { + file, + crc: crc.finalize(), + sha256: hex::encode(sha.finalize()), + }) }) } @@ -345,30 +347,23 @@ async fn write_zip( const FLAGS: u16 = 0x0800; let mut crcs = Vec::with_capacity(plan.entries.len()); - // Prefetch: start fetching the next object while streaming the current one. + // Prefetch: spool the next object to a temp file while streaming the + // current one — the prefetched body is drained immediately, never held + // open across the current entry's client stream. let mut pending: Option>> = None; for (i, entry) in plan.entries.iter().enumerate() { let current = match pending.take() { Some(handle) => handle, - None => fetch_entry(state, entry.s3_key.clone(), entry.size, entry.crc.is_some()), + None => fetch_entry(state, entry.s3_key.clone(), entry.size), }; if let Some(next) = plan.entries.get(i + 1) { - pending = Some(fetch_entry( - state, - next.s3_key.clone(), - next.size, - next.crc.is_some(), - )); + pending = Some(fetch_entry(state, next.s3_key.clone(), next.size)); } - let fetched = current + let mut fetched = current .await .map_err(|e| anyhow::anyhow!("fetch task failed: {e}"))? .map_err(|e| anyhow::anyhow!("photo {}: {e:#}", entry.photo_id))?; - let crc = match &fetched { - Fetched::Direct(_) => entry.crc.expect("direct fetch implies known crc"), - Fetched::Spooled(_, crc) => *crc, - }; - crcs.push(crc); + crcs.push(fetched.crc); let mut lfh = Vec::with_capacity(30 + entry.name.len()); lfh.extend_from_slice(&0x04034b50u32.to_le_bytes()); @@ -377,35 +372,26 @@ async fn write_zip( lfh.extend_from_slice(&0u16.to_le_bytes()); // method: stored lfh.extend_from_slice(&entry.dos_time.to_le_bytes()); lfh.extend_from_slice(&entry.dos_date.to_le_bytes()); - lfh.extend_from_slice(&crc.to_le_bytes()); + lfh.extend_from_slice(&fetched.crc.to_le_bytes()); lfh.extend_from_slice(&(entry.size as u32).to_le_bytes()); // compressed lfh.extend_from_slice(&(entry.size as u32).to_le_bytes()); // uncompressed lfh.extend_from_slice(&(entry.name.len() as u16).to_le_bytes()); lfh.extend_from_slice(&0u16.to_le_bytes()); // extra len lfh.extend_from_slice(&entry.name); out.write_all(&lfh).await?; - match fetched { - Fetched::Direct(object) => { - let mut reader = object.body.into_async_read(); - let copied = tokio::io::copy(&mut reader, &mut out).await?; - anyhow::ensure!( - copied == entry.size, - "{} is {copied} bytes in s3 but {} in the database", - entry.s3_key, - entry.size - ); - } - Fetched::Spooled(mut file, crc) => { - tokio::io::copy(&mut file, &mut out).await?; - // Self-heal: store the freshly computed crc so the next - // download of this photo streams directly. - let _ = sqlx::query("update photos set crc32 = coalesce(crc32, $2) where id = $1") - .bind(entry.photo_id) - .bind(i64::from(crc)) - .execute(&state.db) - .await; - } - } + tokio::io::copy(&mut fetched.file, &mut out).await?; + + // Self-heal legacy photos: backfill both hashes so future zips and + // upload dedup both work for them. + let _ = sqlx::query( + "update photos set crc32 = coalesce(crc32, $2), sha256 = coalesce(sha256, $3) + where id = $1", + ) + .bind(entry.photo_id) + .bind(i64::from(fetched.crc)) + .bind(&fetched.sha256) + .execute(&state.db) + .await; } // Central directory. diff --git a/tests/tenancy.rs b/tests/tenancy.rs index 3b6a953..82af14a 100644 --- a/tests/tenancy.rs +++ b/tests/tenancy.rs @@ -44,10 +44,12 @@ fn test_config(database_url: String) -> Config { fn session_for(user_id: Uuid, email: &str) -> String { let key = Key::from(&Sha512::digest(SECRET.as_bytes())); let exp = chrono::Utc::now().timestamp() + 3600; + // Use the production payload builder so the test can't drift from what + // user_from_jar parses. let mut jar = CookieJar::new(); jar.signed_mut(&key).add(Cookie::new( "photos_session", - format!("{user_id}|{exp}|{email}"), + photos::auth::session_payload(user_id, email, exp), )); format!("photos_session={}", jar.get("photos_session").unwrap().value()) } @@ -184,6 +186,23 @@ async fn tenant_isolation_matrix() { assert_eq!(status, StatusCode::OK); assert_eq!(list.as_array().unwrap().len(), 0, "bob must see no albums"); + // ---- Dual-auth image routes must ALSO reject a non-owning photographer. + // Bob owns no share and doesn't own the album, so authorize_photo falls + // through to the share-cookie path and 401s BEFORE any S3 access — the + // cross-tenant original leak these routes previously allowed. ---- + for path in [ + format!("/api/img/{photo_id}/thumb"), + format!("/api/img/{photo_id}/preview"), + format!("/api/photos/{photo_id}/original"), + ] { + let (status, _) = request(&router, "GET", &path, Some(&cookie_b), None).await; + assert_eq!( + status, + StatusCode::UNAUTHORIZED, + "cross-tenant image GET {path} must not authorize" + ); + } + // ---- Alice keeps full access to her own resources. ---- let (status, list) = request(&router, "GET", "/api/albums", Some(&cookie_a), None).await; assert_eq!(status, StatusCode::OK);