From 420bda5e386c1be10230f96d8aff7d747431faa3 Mon Sep 17 00:00:00 2001 From: IliesAllali Date: Fri, 16 Oct 2026 18:08:50 +0200 Subject: [PATCH] fix(arbre): bornes des positions et conflit de relecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Les coordonnées de disposition sont bornées à ±1 000 000 (PATCH personne, modification en session de contribution, OpenAPI). - Un déplacement par le propriétaire ne met plus en conflit les contributions en attente sur la même personne : layoutX/layoutY sont exclus de la comparaison de conflit. Co-Authored-By: Claude Opus 5.5 --- backend/openapi/v1.yaml | 4 + backend/src/routes/contributions.ts | 11 ++- backend/src/routes/trees.ts | 7 +- backend/src/utils/graph-payload.ts | 3 + backend/test/api-security.integration.test.ts | 89 +++++++++++++++++++ 5 files changed, 110 insertions(+), 4 deletions(-) diff --git a/backend/openapi/v1.yaml b/backend/openapi/v1.yaml index e6cabb2..6eb7257 100644 --- a/backend/openapi/v1.yaml +++ b/backend/openapi/v1.yaml @@ -356,9 +356,13 @@ components: layoutX: type: number nullable: true + minimum: -1000000 + maximum: 1000000 layoutY: type: number nullable: true + minimum: -1000000 + maximum: 1000000 sex: type: string nullable: true diff --git a/backend/src/routes/contributions.ts b/backend/src/routes/contributions.ts index b79a930..cb4712b 100644 --- a/backend/src/routes/contributions.ts +++ b/backend/src/routes/contributions.ts @@ -10,8 +10,11 @@ import { removeStoredFile } from '../lib/media-files.js' import { syncPersonMediaOrder } from '../lib/media-order.js' import type { Actor, MembershipRole } from '../types/auth.js' +import { LAYOUT_COORDINATE_LIMIT } from '../utils/graph-payload.js' import { findParentChildValidationError, findUnionValidationError } from '../utils/relationship-guards.js' +const layoutCoordinateSchema = z.number().finite().min(-LAYOUT_COORDINATE_LIMIT).max(LAYOUT_COORDINATE_LIMIT) + const isoDateRegex = /^\d{4}-\d{2}-\d{2}$/ const treeIdParamSchema = z.object({ @@ -86,8 +89,8 @@ const personUpdateSchema = z birthName: z.string().max(120).optional().nullable(), birthDate: z.string().regex(isoDateRegex).optional().nullable(), deathDate: z.string().regex(isoDateRegex).optional().nullable(), - layoutX: z.number().finite().optional().nullable(), - layoutY: z.number().finite().optional().nullable(), + layoutX: layoutCoordinateSchema.optional().nullable(), + layoutY: layoutCoordinateSchema.optional().nullable(), birthPlace: z.string().max(200).optional().nullable(), profession: z.string().max(200).optional().nullable(), region: z.string().max(120).optional().nullable(), @@ -781,6 +784,8 @@ async function applyContributionChange( } const SNAPSHOT_ENTITY_TYPES = new Set(['person', 'union', 'parent_child_link']) +// Champs qui ne créent jamais de conflit de relecture (positions dans l'arbre, déplacées à la main) +const NON_CONFLICTING_KEYS = new Set(['layoutX', 'layoutY']) // État actuel d'une personne, union ou lien, tel qu'on le montre à la relecture. null : introuvable dans l'arbre. async function snapshotEntity( @@ -853,6 +858,8 @@ async function detectConflict( const snap = await snapshotEntity(tx, treeId, change.entityType, change.entityId) if (!snap) return true for (const key of Object.keys(before)) { + // La place dans l'arbre bouge à chaque glisser du propriétaire : ce n'est pas une modification concurrente + if (NON_CONFLICTING_KEYS.has(key)) continue if (key in snap && String(snap[key] ?? '') !== String(before[key] ?? '')) return true } return false diff --git a/backend/src/routes/trees.ts b/backend/src/routes/trees.ts index cb89ea2..9baba17 100644 --- a/backend/src/routes/trees.ts +++ b/backend/src/routes/trees.ts @@ -13,8 +13,11 @@ import { decryptSharePassword, encryptSharePassword } from '../lib/share-passwor import { purgeTrees } from '../lib/tree-purge.js' import { reserveTreeUnlockAttempt, serializePasswordCheck } from '../lib/unlock-guard.js' import type { MembershipRole } from '../types/auth.js' +import { LAYOUT_COORDINATE_LIMIT } from '../utils/graph-payload.js' import { findParentChildValidationError, findUnionValidationError } from '../utils/relationship-guards.js' +const layoutCoordinateSchema = z.number().finite().min(-LAYOUT_COORDINATE_LIMIT).max(LAYOUT_COORDINATE_LIMIT) + const MAX_TREES_PER_ACCOUNT = 10 const treeIdParamSchema = z.object({ @@ -101,8 +104,8 @@ const patchPersonSchema = z birthName: z.string().max(120).optional().nullable(), birthDate: z.string().regex(isoDateRegex).optional().nullable(), deathDate: z.string().regex(isoDateRegex).optional().nullable(), - layoutX: z.number().finite().optional().nullable(), - layoutY: z.number().finite().optional().nullable(), + layoutX: layoutCoordinateSchema.optional().nullable(), + layoutY: layoutCoordinateSchema.optional().nullable(), birthPlace: z.string().max(200).optional().nullable(), profession: z.string().max(200).optional().nullable(), region: z.string().max(120).optional().nullable(), diff --git a/backend/src/utils/graph-payload.ts b/backend/src/utils/graph-payload.ts index a77f93c..9d912d9 100644 --- a/backend/src/utils/graph-payload.ts +++ b/backend/src/utils/graph-payload.ts @@ -2,6 +2,9 @@ import type { Annotation, MediaItem, ParentChildLink, Person, Union } from '@pri import { sanitizeAnnotationContentForRead } from '../lib/annotation-content.js' +// Espace des coordonnées de disposition manuelle (unités du graphe). Au-delà, rien de réel dans un arbre. +export const LAYOUT_COORDINATE_LIMIT = 1_000_000 + export type GraphPayload = { persons: Array<{ id: string diff --git a/backend/test/api-security.integration.test.ts b/backend/test/api-security.integration.test.ts index 621cd12..63cdb9b 100644 --- a/backend/test/api-security.integration.test.ts +++ b/backend/test/api-security.integration.test.ts @@ -541,6 +541,44 @@ describe('API security and authz', () => { await app.close() }) + it('refuses a manual layout coordinate outside the tree space', async () => { + const app = createApp() + const token = await createUserToken(app) + + const response = await app.inject({ + method: 'PATCH', + url: '/trees/tree-1/persons/person-1', + headers: { authorization: `Bearer ${token}` }, + payload: { layoutX: 2_000_000, layoutY: 10 }, + }) + + expect(response.statusCode).toBe(400) + expect(prismaMock.person.updateMany).not.toHaveBeenCalled() + await app.close() + }) + + it('forbids a member from moving a person by hand', async () => { + const app = createApp() + const token = await createUserToken(app) + prismaMock.treeMembership.findUnique.mockResolvedValue({ + treeId: 'tree-1', + userId: 'user-1', + role: 'member', + tree: { deletedAt: null, rootPersonId: null }, + }) + + const response = await app.inject({ + method: 'PATCH', + url: '/trees/tree-1/persons/person-1', + headers: { authorization: `Bearer ${token}` }, + payload: { layoutX: 120, layoutY: 240 }, + }) + + expect(response.statusCode).toBe(403) + expect(prismaMock.person.updateMany).not.toHaveBeenCalled() + await app.close() + }) + it('allows contributor access token to submit a pending contribution session', async () => { const app = createApp() await app.ready() @@ -930,6 +968,57 @@ describe('API security and authz', () => { await app.close() }) + it('does not flag a pending person change as a conflict when the owner only moved that person', async () => { + const app = createApp() + const token = await createUserToken(app) + + prismaMock.contributionSession.findFirst.mockResolvedValue({ + id: 'session-1', + treeId: 'tree-1', + status: 'pending', + changes: [ + { + id: 'change-1', + entityType: 'person', + action: 'update', + entityId: 'person-1', + beforeJson: { firstName: 'Alice', layoutX: null, layoutY: null }, + afterJson: { firstName: 'Alicia' }, + conflictState: 'none', + }, + ], + }) + // Le propriétaire a glissé la personne depuis l'envoi : seule la place a changé + prismaMock.person.findFirst.mockResolvedValue({ + firstName: 'Alice', + lastName: 'Martin', + birthName: null, + birthDate: null, + deathDate: null, + layoutX: 300, + layoutY: 400, + birthPlace: null, + profession: null, + region: null, + nationality: null, + sex: null, + notes: null, + }) + + const response = await app.inject({ + method: 'POST', + url: '/trees/tree-1/contributions/sessions/session-1/review', + headers: { authorization: `Bearer ${token}` }, + payload: { decision: 'approved' }, + }) + + expect(response.statusCode).toBe(200) + expect(prismaMock.contributionChange.update).not.toHaveBeenCalledWith(expect.objectContaining({ + data: { conflictState: 'needs_review' }, + })) + await app.close() + }) + it('allows admin to list pending contribution sessions', async () => { const app = createApp() const token = await createUserToken(app)