fix(synology): correct multi-album passphrase assignment and stale trek_photos

- ProviderPicker now tracks per-asset album passphrase in a Map; on confirm,
  assets are grouped by passphrase and submitted as separate batches so each
  asset receives its own album's passphrase instead of the last-selected one
- getOrCreateTrekPhoto unconditionally overwrites the stored passphrase when
  a fresh one is supplied, allowing re-adds to heal a stuck bad passphrase
- deleteTrekPhotoIfOrphan purges the trek_photos row for provider assets when
  no trip_photos or journey_photos reference it anymore; wired into
  removeTripPhoto, removeAlbumLink, and deletePhoto so remove + re-add is a
  clean slate
- Three new integration tests: SYNO-090 (passphrase overwrite), SYNO-091
  (orphan cleanup), SYNO-092 (remove + re-add restores correct passphrase)
This commit is contained in:
jubnl
2026-04-17 19:48:12 +02:00
parent c0aa252f9a
commit 8cd5aa0d23
5 changed files with 124 additions and 16 deletions
+27 -12
View File
@@ -1042,7 +1042,7 @@ function GalleryView({ entries, journeyId, userId, trips, onPhotoClick, onRefres
trips={trips} trips={trips}
existingAssetIds={new Set(entries.flatMap(e => (e.photos || []).filter(p => p.asset_id).map(p => p.asset_id!)))} existingAssetIds={new Set(entries.flatMap(e => (e.photos || []).filter(p => p.asset_id).map(p => p.asset_id!)))}
onClose={() => setShowPicker(false)} onClose={() => setShowPicker(false)}
onAdd={async (assetIds, entryId, passphrase) => { onAdd={async (groups, entryId) => {
let targetId = entryId let targetId = entryId
if (!targetId) { if (!targetId) {
try { try {
@@ -1055,10 +1055,12 @@ function GalleryView({ entries, journeyId, userId, trips, onPhotoClick, onRefres
} catch { return } } catch { return }
} }
let added = 0 let added = 0
try { for (const group of groups) {
const result = await journeyApi.addProviderPhotos(targetId, pickerProvider!, assetIds, undefined, passphrase) try {
added = result.added || 0 const result = await journeyApi.addProviderPhotos(targetId, pickerProvider!, group.assetIds, undefined, group.passphrase)
} catch {} added += result.added || 0
} catch {}
}
if (added > 0) { if (added > 0) {
toast.success(t('journey.photosAdded', { count: added })) toast.success(t('journey.photosAdded', { count: added }))
onRefresh() onRefresh()
@@ -1532,7 +1534,7 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on
trips: JourneyTrip[] trips: JourneyTrip[]
existingAssetIds: Set<string> existingAssetIds: Set<string>
onClose: () => void onClose: () => void
onAdd: (assetIds: string[], entryId: number | null, passphrase?: string) => Promise<void> onAdd: (groups: Array<{ assetIds: string[]; passphrase?: string }>, entryId: number | null) => Promise<void>
}) { }) {
const { t } = useTranslation() const { t } = useTranslation()
const [filter, setFilter] = useState<'trip' | 'custom' | 'all' | 'album'>('trip') const [filter, setFilter] = useState<'trip' | 'custom' | 'all' | 'album'>('trip')
@@ -1546,7 +1548,7 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on
const [searchPage, setSearchPage] = useState(1) const [searchPage, setSearchPage] = useState(1)
const [searchFrom, setSearchFrom] = useState('') const [searchFrom, setSearchFrom] = useState('')
const [searchTo, setSearchTo] = useState('') const [searchTo, setSearchTo] = useState('')
const [selected, setSelected] = useState<Set<string>>(new Set()) const [selected, setSelected] = useState<Map<string, { albumId?: string; passphrase?: string }>>(new Map())
const [customFrom, setCustomFrom] = useState('') const [customFrom, setCustomFrom] = useState('')
const [customTo, setCustomTo] = useState('') const [customTo, setCustomTo] = useState('')
const [targetEntryId, setTargetEntryId] = useState<number | null>(null) const [targetEntryId, setTargetEntryId] = useState<number | null>(null)
@@ -1638,8 +1640,12 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on
const toggleAsset = (id: string) => { const toggleAsset = (id: string) => {
setSelected(prev => { setSelected(prev => {
const next = new Set(prev) const next = new Map(prev)
if (next.has(id)) next.delete(id); else next.add(id) if (next.has(id)) {
next.delete(id)
} else {
next.set(id, { albumId: selectedAlbum ?? undefined, passphrase: selectedAlbumPassphrase })
}
return next return next
}) })
} }
@@ -1801,9 +1807,9 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on
<button <button
onClick={() => { onClick={() => {
if (allSelected) { if (allSelected) {
setSelected(new Set()) setSelected(new Map())
} else { } else {
setSelected(new Set(selectable.map((a: any) => a.id))) setSelected(new Map(selectable.map((a: any) => [a.id, { albumId: selectedAlbum ?? undefined, passphrase: selectedAlbumPassphrase }])))
} }
}} }}
className="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-lg text-[11px] font-medium border border-zinc-200 dark:border-zinc-700 text-zinc-500 dark:text-zinc-400 hover:bg-zinc-50 dark:hover:bg-zinc-800" className="inline-flex items-center gap-1.5 px-2.5 py-1 rounded-lg text-[11px] font-medium border border-zinc-200 dark:border-zinc-700 text-zinc-500 dark:text-zinc-400 hover:bg-zinc-50 dark:hover:bg-zinc-800"
@@ -1905,7 +1911,16 @@ function ProviderPicker({ provider, userId, entries, trips, existingAssetIds, on
{t('common.cancel')} {t('common.cancel')}
</button> </button>
<button <button
onClick={() => onAdd([...selected], targetEntryId, selectedAlbumPassphrase)} onClick={() => {
const groupMap = new Map<string | undefined, string[]>()
for (const [assetId, { passphrase }] of selected.entries()) {
const list = groupMap.get(passphrase) || []
list.push(assetId)
groupMap.set(passphrase, list)
}
const groups = [...groupMap.entries()].map(([passphrase, assetIds]) => ({ assetIds, passphrase }))
onAdd(groups, targetEntryId)
}}
disabled={selected.size === 0} disabled={selected.size === 0}
className="px-3.5 py-2 rounded-lg bg-zinc-900 dark:bg-white text-white dark:text-zinc-900 text-[13px] font-medium hover:bg-zinc-800 dark:hover:bg-zinc-100 disabled:opacity-40 disabled:cursor-not-allowed" className="px-3.5 py-2 rounded-lg bg-zinc-900 dark:bg-white text-white dark:text-zinc-900 text-[13px] font-medium hover:bg-zinc-800 dark:hover:bg-zinc-100 disabled:opacity-40 disabled:cursor-not-allowed"
> >
+2 -1
View File
@@ -1,7 +1,7 @@
import { db } from '../db/database'; import { db } from '../db/database';
import { broadcastToUser } from '../websocket'; import { broadcastToUser } from '../websocket';
import type { Journey, JourneyEntry, JourneyPhoto, JourneyContributor } from '../types'; import type { Journey, JourneyEntry, JourneyPhoto, JourneyContributor } from '../types';
import { getOrCreateTrekPhoto, getOrCreateLocalTrekPhoto, setTrekPhotoProvider } from './memories/photoResolverService'; import { getOrCreateTrekPhoto, getOrCreateLocalTrekPhoto, setTrekPhotoProvider, deleteTrekPhotoIfOrphan } from './memories/photoResolverService';
function ts(): number { function ts(): number {
return Date.now(); return Date.now();
@@ -718,6 +718,7 @@ export function deletePhoto(photoId: number, userId: number): (JourneyPhoto & {
if (!canEdit(photo.journey_id, userId)) return null; if (!canEdit(photo.journey_id, userId)) return null;
db.prepare('DELETE FROM journey_photos WHERE id = ?').run(photoId); db.prepare('DELETE FROM journey_photos WHERE id = ?').run(photoId);
deleteTrekPhotoIfOrphan(photo.photo_id);
// clean up empty Gallery entries left behind // clean up empty Gallery entries left behind
const remaining = db.prepare('SELECT 1 FROM journey_photos WHERE entry_id = ?').get(photo.entry_id); const remaining = db.prepare('SELECT 1 FROM journey_photos WHERE entry_id = ?').get(photo.entry_id);
@@ -22,7 +22,7 @@ export function getOrCreateTrekPhoto(
).get(provider, assetId, ownerId) as { id: number } | undefined; ).get(provider, assetId, ownerId) as { id: number } | undefined;
if (existing) { if (existing) {
if (passphrase) { if (passphrase) {
db.prepare('UPDATE trek_photos SET passphrase = ? WHERE id = ? AND passphrase IS NULL') db.prepare('UPDATE trek_photos SET passphrase = ? WHERE id = ?')
.run(encrypt_api_key(passphrase), existing.id); .run(encrypt_api_key(passphrase), existing.id);
} }
return existing.id; return existing.id;
@@ -145,6 +145,19 @@ export function setTrekPhotoProvider(
).run(provider, assetId, ownerId, trekPhotoId); ).run(provider, assetId, ownerId, trekPhotoId);
} }
// ── Orphan cleanup ───────────────────────────────────────────────────────
export function deleteTrekPhotoIfOrphan(photoId: number): void {
const stillUsed = db.prepare(`
SELECT 1 FROM trip_photos WHERE photo_id = ?
UNION ALL
SELECT 1 FROM journey_photos WHERE photo_id = ?
LIMIT 1
`).get(photoId, photoId);
if (stillUsed) return;
db.prepare("DELETE FROM trek_photos WHERE id = ? AND provider != 'local'").run(photoId);
}
// ── Delete local file for a trek_photo ────────────────────────────────── // ── Delete local file for a trek_photo ──────────────────────────────────
export function getTrekPhotoFilePath(photoId: number): string | null { export function getTrekPhotoFilePath(photoId: number): string | null {
+10 -2
View File
@@ -8,7 +8,7 @@ import {
mapDbError, mapDbError,
Selection, Selection,
} from './helpersService'; } from './helpersService';
import { getOrCreateTrekPhoto } from './photoResolverService'; import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from './photoResolverService';
import { encrypt_api_key } from '../apiKeyCrypto'; import { encrypt_api_key } from '../apiKeyCrypto';
@@ -212,6 +212,7 @@ export function removeTripPhoto(
AND photo_id = ? AND photo_id = ?
`).run(tripId, userId, photoId); `).run(tripId, userId, photoId);
deleteTrekPhotoIfOrphan(photoId);
broadcast(tripId, 'memories:updated', { userId }, sid); broadcast(tripId, 'memories:updated', { userId }, sid);
return success(true); return success(true);
@@ -269,13 +270,20 @@ export function removeAlbumLink(tripId: string, linkId: string, userId: number):
} }
try { try {
const linkedPhotos = db.prepare('SELECT photo_id FROM trip_photos WHERE trip_id = ? AND album_link_id = ?')
.all(tripId, linkId) as Array<{ photo_id: number }>;
db.transaction(() => { db.transaction(() => {
db.prepare('DELETE FROM trip_photos WHERE trip_id = ? AND album_link_id = ?') db.prepare('DELETE FROM trip_photos WHERE trip_id = ? AND album_link_id = ?')
.run(tripId, linkId); .run(tripId, linkId);
db.prepare('DELETE FROM trip_album_links WHERE id = ? AND trip_id = ? AND user_id = ?') db.prepare('DELETE FROM trip_album_links WHERE id = ? AND trip_id = ? AND user_id = ?')
.run(linkId, tripId, userId); .run(linkId, tripId, userId);
})(); })();
for (const { photo_id } of linkedPhotos) {
deleteTrekPhotoIfOrphan(photo_id);
}
return success(true); return success(true);
} catch (error) { } catch (error) {
return mapDbError(error, 'Failed to remove album link'); return mapDbError(error, 'Failed to remove album link');
@@ -1169,3 +1169,74 @@ describe('Synology SSRF blocked error handling', () => {
expect(res.body.albums.length).toBeGreaterThan(0); expect(res.body.albums.length).toBeGreaterThan(0);
}); });
}); });
// ── Passphrase persistence fixes ─────────────────────────────────────────────
import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from '../../src/services/memories/photoResolverService';
import { decrypt_api_key } from '../../src/services/apiKeyCrypto';
describe('trek_photos passphrase healing (SYNO-090)', () => {
it('SYNO-090 — getOrCreateTrekPhoto overwrites an existing bad passphrase when a new one is supplied', () => {
const { user } = createUser(testDb);
const wrongPass = 'wrong-passphrase';
const correctPass = 'correct-passphrase';
const id1 = getOrCreateTrekPhoto('synologyphotos', 'asset-heal-test', user.id, wrongPass);
const row1 = testDb.prepare('SELECT passphrase FROM trek_photos WHERE id = ?').get(id1) as { passphrase: string };
expect(decrypt_api_key(row1.passphrase)).toBe(wrongPass);
const id2 = getOrCreateTrekPhoto('synologyphotos', 'asset-heal-test', user.id, correctPass);
expect(id2).toBe(id1);
const row2 = testDb.prepare('SELECT passphrase FROM trek_photos WHERE id = ?').get(id2) as { passphrase: string };
expect(decrypt_api_key(row2.passphrase)).toBe(correctPass);
});
});
describe('trek_photos orphan cleanup (SYNO-091)', () => {
it('SYNO-091 — deleteTrekPhotoIfOrphan removes the trek_photos row when no trip_photos or journey_photos reference it', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
testDb.prepare("UPDATE photo_providers SET enabled = 1 WHERE id = 'synologyphotos'").run();
const trekPhotoId = getOrCreateTrekPhoto('synologyphotos', 'asset-orphan-test', user.id, 'pass-A');
testDb.prepare(
'INSERT OR IGNORE INTO trip_photos (trip_id, user_id, photo_id, shared) VALUES (?, ?, ?, 1)'
).run(trip.id, user.id, trekPhotoId);
// Still referenced — must not be deleted.
deleteTrekPhotoIfOrphan(trekPhotoId);
expect(testDb.prepare('SELECT id FROM trek_photos WHERE id = ?').get(trekPhotoId)).toBeDefined();
// Remove the reference, then orphan-cleanup should delete the trek_photos row.
testDb.prepare('DELETE FROM trip_photos WHERE photo_id = ?').run(trekPhotoId);
deleteTrekPhotoIfOrphan(trekPhotoId);
expect(testDb.prepare('SELECT id FROM trek_photos WHERE id = ?').get(trekPhotoId)).toBeUndefined();
});
it('SYNO-092 — re-adding a previously removed Synology photo stores the new passphrase correctly', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
testDb.prepare("UPDATE photo_providers SET enabled = 1 WHERE id = 'synologyphotos'").run();
const firstPass = 'first-passphrase';
const secondPass = 'second-passphrase';
// Add with wrong passphrase, then remove (simulating the bug scenario).
const id1 = getOrCreateTrekPhoto('synologyphotos', 'asset-readd-test', user.id, firstPass);
testDb.prepare(
'INSERT OR IGNORE INTO trip_photos (trip_id, user_id, photo_id, shared) VALUES (?, ?, ?, 1)'
).run(trip.id, user.id, id1);
testDb.prepare('DELETE FROM trip_photos WHERE photo_id = ?').run(id1);
deleteTrekPhotoIfOrphan(id1);
// trek_photos row should be gone.
expect(testDb.prepare('SELECT id FROM trek_photos WHERE id = ?').get(id1)).toBeUndefined();
// Re-add with the correct passphrase.
const id2 = getOrCreateTrekPhoto('synologyphotos', 'asset-readd-test', user.id, secondPass);
const row = testDb.prepare('SELECT passphrase FROM trek_photos WHERE id = ?').get(id2) as { passphrase: string };
expect(decrypt_api_key(row.passphrase)).toBe(secondPass);
});
});