From c703a34165dc83ee02457f9ba085c962b37bd12f Mon Sep 17 00:00:00 2001 From: Mistral Vibe Date: Mon, 12 Oct 2026 13:40:38 +0200 Subject: [PATCH] =?UTF-8?q?Immich=20:=20v=C3=A9rifier=20l'existence=20d'un?= =?UTF-8?q?e=20personne=20avant=20l'association?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le 404 de la vignette ne distingue pas une personne inexistante d'une vignette manquante : l'association pouvait donc être enregistrée pour une personne Immich qui n'existe pas. - ImmichClient.getPerson() : GET /api/people/{id} (200 | 404), qui établit l'existence de la personne et annonce sa vignette. - La route d'association rejette une personne inexistante (404 immich_person_not_found) ; un refus d'authentification (502) ou une erreur serveur (503) n'est jamais pris pour une absence. - La vignette n'est demandée que si Immich en annonce une ; une vignette manquante chez une personne existante associe toujours sans portrait. - Tests : personne inexistante refusée sans toucher base, audit ni fichiers ; vignette disparue entre les deux appels ; échec d'authentification pendant l'association sans changement en base. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe --- backend/src/lib/immich.ts | 16 +++++ backend/src/routes/immich.ts | 26 +++++++-- backend/test/immich.test.ts | 109 +++++++++++++++++++++++++++++++++-- 3 files changed, 141 insertions(+), 10 deletions(-) diff --git a/backend/src/lib/immich.ts b/backend/src/lib/immich.ts index 0edf9bb..5b1d288 100644 --- a/backend/src/lib/immich.ts +++ b/backend/src/lib/immich.ts @@ -134,6 +134,22 @@ export class ImmichClient { return people } + // Une personne précise : 200 avec ses données, 404 si elle n'existe pas. Le 404 de la + // vignette ne prouve rien (personne absente ou simple vignette manquante), l'existence + // se vérifie donc ici. + async getPerson(personId: string): Promise { + const response = await this.fetch(`/api/people/${encodeURIComponent(personId)}`) + const person = (await response.json()) as { id?: unknown; name?: unknown; thumbnailPath?: unknown } + if (typeof person?.id !== 'string' || typeof person?.name !== 'string') { + throw new ImmichUnavailableError('Réponse Immich inattendue sur /api/people/{id}') + } + return { + id: person.id, + name: person.name, + hasThumbnail: typeof person.thumbnailPath === 'string' && person.thumbnailPath.length > 0, + } + } + // Vignette d'une personne, en octets bruts (JPEG côté Immich). Absente -> ImmichNotFoundError. async getPersonThumbnail(personId: string): Promise { const response = await this.fetch(`/api/people/${encodeURIComponent(personId)}/thumbnail`) diff --git a/backend/src/routes/immich.ts b/backend/src/routes/immich.ts index b321b5a..91f3bd4 100644 --- a/backend/src/routes/immich.ts +++ b/backend/src/routes/immich.ts @@ -11,6 +11,7 @@ import { ImmichAuthError, ImmichNotFoundError, ImmichUnavailableError, + type ImmichPersonSummary, } from '../lib/immich.js' import { InvalidMediaError, removeStoredFile, sniffMimeType } from '../lib/media-files.js' import { exceedsStorageQuota, getStorageRoot, prepareUploadedFile, requireAdminMembership } from './media.js' @@ -184,15 +185,28 @@ export const immichRoutes: FastifyPluginAsync = async (app) => { return reply.code(404).send({ error: 'person_not_found' }) } - // Vignette de la personne Immich : absente -> on associe sans toucher au portrait existant. - let thumbnail: Buffer | null = null + // La personne Immich doit exister : le 404 de la vignette ne le prouve pas (personne + // absente ou simple vignette manquante), on la vérifie donc explicitement. Un refus + // d'authentification ou une erreur serveur ne sont jamais pris pour une absence. + let immichPerson: ImmichPersonSummary try { - thumbnail = await client.getPersonThumbnail(immichPersonId) + immichPerson = await client.getPerson(immichPersonId) } catch (error) { - if (!(error instanceof ImmichNotFoundError)) { - return replyImmichError(request, reply, error) + return replyImmichError(request, reply, error) + } + + // Vignette de la personne Immich : absente -> on associe sans toucher au portrait existant. + // Demandée seulement si Immich en annonce une : sans vignette, l'appel est inutile. + let thumbnail: Buffer | null = null + if (immichPerson.hasThumbnail) { + try { + thumbnail = await client.getPersonThumbnail(immichPersonId) + } catch (error) { + if (!(error instanceof ImmichNotFoundError)) { + return replyImmichError(request, reply, error) + } + // Vignette absente : l'association se fait sans portrait. } - // Vignette absente : l'association se fait sans portrait. } if (!thumbnail) { diff --git a/backend/test/immich.test.ts b/backend/test/immich.test.ts index 49f28b2..d18236f 100644 --- a/backend/test/immich.test.ts +++ b/backend/test/immich.test.ts @@ -46,6 +46,20 @@ async function jsonResponse(payload: unknown, status = 200): Promise { return new Response(JSON.stringify(payload), { status, headers: { 'content-type': 'application/json' } }) } +// Association : deux appels Immich possibles, la personne (JSON) puis sa vignette (image). +// Le 404 de /api/people/{id} dit que la personne n'existe pas ; celui de /thumbnail seul +// ne dit rien (personne absente ou vignette manquante). +function mockImmichPerson(options: { thumbnailPath?: string; thumbnailStatus?: number } = {}): void { + const { thumbnailPath = '/thumb/a.jpg', thumbnailStatus = 200 } = options + fetchMock.mockImplementation(async (input: RequestInfo | URL) => { + const url = String(input) + if (url.endsWith('/thumbnail')) { + return new Response(new Uint8Array(jpegBytes), { status: thumbnailStatus }) + } + return jsonResponse({ id: immichPersonId, name: 'Jean Dupont', thumbnailPath, isHidden: false, birthDate: null }) + }) +} + async function createUserToken(app: FastifyInstance, userId = 'user-1'): Promise { await app.ready() return (app as any).jwt.sign( @@ -305,7 +319,7 @@ describe('Immich integration: assign', () => { await writeFile(await absStoragePath(oldAvatarRelative), 'old avatar') prismaMock.person.findFirst.mockResolvedValue({ id: 'person-1', avatarPath: oldAvatarRelative }) - fetchMock.mockResolvedValue(new Response(new Uint8Array(jpegBytes), { status: 200 })) + mockImmichPerson() const response = await app.inject({ method: 'POST', @@ -346,7 +360,36 @@ describe('Immich integration: assign', () => { const token = await createUserToken(app) prismaMock.person.findFirst.mockResolvedValue({ id: 'person-1', avatarPath: 'tree-1/person-1/avatar-keep.jpg' }) - fetchMock.mockResolvedValue(new Response('', { status: 404 })) + mockImmichPerson({ thumbnailPath: '' }) + + const response = await app.inject({ + method: 'POST', + url: '/trees/tree-1/immich/assign', + headers: { authorization: `Bearer ${token}` }, + payload: { personId: 'person-1', immichPersonId }, + }) + + expect(response.statusCode).toBe(200) + expect(response.json()).toEqual({ ok: true, avatarUpdated: false, immichPersonId }) + + // Sans vignette annoncée, l'appel de vignette n'a même pas lieu. + expect(fetchMock).toHaveBeenCalledTimes(1) + + const updateData = prismaMock.person.update.mock.calls[0][0].data + expect(updateData.immichPersonId).toBe(immichPersonId) + expect(updateData).not.toHaveProperty('avatarPath') + expect(updateData).not.toHaveProperty('avatarMimeType') + await app.close() + }) + + it('still assigns without the avatar when the thumbnail vanishes after the person was found', async () => { + const app = createApp() + const token = await createUserToken(app) + + // Immich annonce une vignette qui répond pourtant 404 : la personne existe, + // l'association se fait sans portrait. + prismaMock.person.findFirst.mockResolvedValue({ id: 'person-1', avatarPath: null }) + mockImmichPerson({ thumbnailStatus: 404 }) const response = await app.inject({ method: 'POST', @@ -365,6 +408,64 @@ describe('Immich integration: assign', () => { await app.close() }) + it('refuses the assignment when the Immich person does not exist', async () => { + const app = createApp() + const token = await createUserToken(app) + + // Portrait en place : un refus ne doit rien changer. + // Dossier dédié : les autres tests laissent leurs portraits dans tree-1/person-1. + const avatarRelative = 'tree-1/person-missing/avatar-keep.jpg' + await mkdir(path.dirname(await absStoragePath(avatarRelative)), { recursive: true }) + await writeFile(await absStoragePath(avatarRelative), 'existing avatar') + + prismaMock.person.findFirst.mockResolvedValue({ id: 'person-missing', avatarPath: avatarRelative }) + // GET /api/people/{id} : la personne n'existe pas. Un 404 sur la vignette ne suffirait + // pas à le distinguer d'une simple vignette manquante. + fetchMock.mockResolvedValue(new Response('', { status: 404 })) + + const response = await app.inject({ + method: 'POST', + url: '/trees/tree-1/immich/assign', + headers: { authorization: `Bearer ${token}` }, + payload: { personId: 'person-missing', immichPersonId }, + }) + + expect(response.statusCode).toBe(404) + expect(response.json()).toEqual({ error: 'immich_person_not_found' }) + + // Base, journal d'audit et fichiers : rien n'a bougé. + expect(prismaMock.person.update).not.toHaveBeenCalled() + expect(prismaMock.auditLog.create).not.toHaveBeenCalled() + expect(prismaMock.$transaction).not.toHaveBeenCalled() + await expect(fsExists(await absStoragePath(avatarRelative))).resolves.toBe(true) + + const files = await fsListDir(path.join(storageRoot, 'tree-1', 'person-missing')) + expect(files.filter((name) => name.startsWith('immich-'))).toEqual([]) + await app.close() + }) + + it('maps an Immich auth failure during assignment to 502 without touching the database', async () => { + const app = createApp() + const token = await createUserToken(app) + + prismaMock.person.findFirst.mockResolvedValue({ id: 'person-1', avatarPath: null }) + fetchMock.mockResolvedValue(new Response(JSON.stringify({ message: 'Invalid API key' }), { status: 401 })) + + const response = await app.inject({ + method: 'POST', + url: '/trees/tree-1/immich/assign', + headers: { authorization: `Bearer ${token}` }, + payload: { personId: 'person-1', immichPersonId }, + }) + + // Un refus d'authentification n'est jamais pris pour une personne manquante. + expect(response.statusCode).toBe(502) + expect(response.json()).toEqual({ error: 'immich_auth_failed' }) + expect(prismaMock.person.update).not.toHaveBeenCalled() + expect(prismaMock.auditLog.create).not.toHaveBeenCalled() + await app.close() + }) + it('rejects invalid identifiers, unauthenticated, non-admin and unknown person', async () => { const app = createApp() const token = await createUserToken(app) @@ -426,7 +527,7 @@ describe('Immich integration: assign', () => { await writeFile(path.join(storageRoot, 'tree-quota', 'big.bin'), Buffer.alloc(1024 * 1024)) prismaMock.person.findFirst.mockResolvedValue({ id: 'person-quota', avatarPath: null }) - fetchMock.mockResolvedValue(new Response(new Uint8Array(jpegBytes), { status: 200 })) + mockImmichPerson() const response = await app.inject({ method: 'POST', @@ -449,7 +550,7 @@ describe('Immich integration: assign', () => { await writeFile(await absStoragePath(oldAvatarRelative), 'old avatar') prismaMock.person.findFirst.mockResolvedValue({ id: 'person-tx', avatarPath: oldAvatarRelative }) - fetchMock.mockResolvedValue(new Response(new Uint8Array(jpegBytes), { status: 200 })) + mockImmichPerson() prismaMock.$transaction.mockRejectedValue(new Error('db down')) const response = await app.inject({