From 3944688b12218b49a0a7487769d48d10562acc21 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 03:42:54 +0300 Subject: [PATCH 1/3] fix: check edit permission on parent note in relation routes POST and PATCH /note/:notePublicId/relation only checked that the user can edit the child note. The parent passed in parentNoteId was never checked, so a user could put their own note under anyone's note. Both handlers now require a Write role on the parent (same rule as the userCanEdit policy) and reply 403 otherwise. An unknown parent is still left to the note service, so its 400 responses are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/presentation/http/router/note.test.ts | 143 ++++++++++++++++++++++ src/presentation/http/router/note.ts | 41 +++++++ 2 files changed, 184 insertions(+) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 575932ac..520ab05e 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1664,6 +1664,83 @@ describe('Note API', () => { expect(response?.json().parentNote.id).toBe(newParentNote.publicId); }); + test.each([ + /** Returns 403 if user is not in the team of the new parent note */ + { + role: null, + }, + + /** Returns 403 if user is team member of the new parent note with a Read role */ + { + role: MemberRole.Read, + }, + ]) + ('Returns 403 when user can not edit the new parent note', async ({ role }) => { + /* create test user - creator of the new parent note */ + const owner = await global.db.insertUser(); + + /* create test child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /* create test parent note */ + const parentNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /* create test note of another user, that will be new parent for the child note */ + const newParentNote = await global.db.insertNote({ + creatorId: owner.id, + }); + + /* create note settings for child note */ + await global.db.insertNoteSetting({ + noteId: childNote.id, + isPublic: true, + }); + + /* create test relation */ + await global.db.insertNoteRelation({ + noteId: childNote.id, + parentId: parentNote.id, + }); + + /* create test team of the new parent note */ + if (role !== null) { + await global.db.insertNoteTeam({ + noteId: newParentNote.id, + userId: user.id, + role: role, + }); + } + + let response = await global.api?.fakeRequest({ + method: 'PATCH', + headers: { + authorization: `Bearer ${accessToken}`, + }, + body: { + parentNoteId: newParentNote.publicId, + }, + url: `/note/${childNote.publicId}/relation`, + }); + + expect(response?.statusCode).toBe(403); + + expect(response?.json().message).toStrictEqual('Permission denied'); + + response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${childNote.publicId}`, + }); + + expect(response?.json().parentNote.id).toBe(parentNote.publicId); + }); + test('Returns 400 when parent is the same as child', async () => { /* create test child note */ const childNote = await global.db.insertNote({ @@ -1802,6 +1879,72 @@ describe('Note API', () => { expect(response?.json().parentNote.id).toBe(parentNote.publicId); }); + test.each([ + /** Returns 403 if user is not in the team of the parent note */ + { + role: null, + }, + + /** Returns 403 if user is team member of the parent note with a Read role */ + { + role: MemberRole.Read, + }, + ]) + ('Returns 403 when user can not edit the parent note', async ({ role }) => { + /* create test user - creator of the parent note */ + const owner = await global.db.insertUser(); + + /* create test child note */ + const childNote = await global.db.insertNote({ + creatorId: user.id, + }); + + /* create test note of another user, that will be parent for the child note */ + const parentNote = await global.db.insertNote({ + creatorId: owner.id, + }); + + /* create note settings for child note */ + await global.db.insertNoteSetting({ + noteId: childNote.id, + isPublic: true, + }); + + /* create test team of the parent note */ + if (role !== null) { + await global.db.insertNoteTeam({ + noteId: parentNote.id, + userId: user.id, + role: role, + }); + } + + let response = await global.api?.fakeRequest({ + method: 'POST', + headers: { + authorization: `Bearer ${accessToken}`, + }, + body: { + parentNoteId: parentNote.publicId, + }, + url: `/note/${childNote.publicId}/relation`, + }); + + expect(response?.statusCode).toBe(403); + + expect(response?.json().message).toStrictEqual('Permission denied'); + + response = await global.api?.fakeRequest({ + method: 'GET', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note/${childNote.publicId}`, + }); + + expect(response?.json().parentNote).toBeUndefined(); + }); + test('Returns 400 when note already has parent note', async () => { /* create test child note */ const childNote = await global.db.insertNote({ diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index f6cb0492..d114aeb3 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -7,6 +7,7 @@ import useNoteResolver from '../middlewares/note/useNoteResolver.js'; import useNoteSettingsResolver from '../middlewares/noteSettings/useNoteSettingsResolver.js'; import useMemberRoleResolver from '../middlewares/noteSettings/useMemberRoleResolver.js'; import { MemberRole } from '@domain/entities/team.js'; +import { DomainError } from '@domain/entities/DomainError.js'; import { type NotePublic, definePublicNote } from '@domain/entities/notePublic.js'; import type NoteVisitsService from '@domain/service/noteVisits.js'; import type EditorToolsService from '@domain/service/editorTools.js'; @@ -72,6 +73,30 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const { memberRoleResolver } = useMemberRoleResolver(noteSettingsService); + /** + * Checks if user has a Write role in the note that is going to become a parent + * If parent note does not exist, returns true, so note service could reply with its own error + * @param userId - id of the user + * @param parentPublicId - public id of the parent note + */ + async function userCanEditParentNote(userId: number, parentPublicId: NotePublicId): Promise { + let parentNote: Note; + + try { + parentNote = await noteService.getNoteByPublicId(parentPublicId); + } catch (error) { + if (error instanceof DomainError) { + return true; + } + + throw error; + } + + const memberRole = await noteSettingsService.getUserRoleByUserIdAndNoteId(userId, parentNote.id); + + return memberRole === MemberRole.Write; + } + /** * Get note by id */ @@ -439,6 +464,14 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don }, async (request, reply) => { const noteId = request.note?.id as number; const parentNoteId = request.body.parentNoteId; + const { userId } = request; + + /** + * User should have a Write role in the parent note too + */ + if (!await userCanEditParentNote(userId!, parentNoteId)) { + return reply.forbidden(); + } const parentNote = await noteService.createNoteRelation(noteId, parentNoteId); @@ -494,6 +527,14 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don }, async (request, reply) => { const noteId = request.note?.id as number; const parentNoteId = request.body.parentNoteId; + const { userId } = request; + + /** + * User should have a Write role in the new parent note too + */ + if (!await userCanEditParentNote(userId!, parentNoteId)) { + return reply.forbidden(); + } const isUpdated = await noteService.updateNoteRelation(noteId, parentNoteId); From 975e1ce9931513c7ed1151ac7feba6dc8d162a1c Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 03:54:59 +0300 Subject: [PATCH 2/3] refactor: use parent note policy in relation routes Replace the router helper with parentNoteResolver and the userCanEditParentNote policy from #310. An unknown parent note now gets 406 "Parent note not found" from the resolver, as in POST /note. Co-Authored-By: Claude Opus 5.5 --- src/presentation/http/router/note.test.ts | 12 +++--- src/presentation/http/router/note.ts | 45 ++--------------------- 2 files changed, 10 insertions(+), 47 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 0392daa7..4c59d8ac 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1897,7 +1897,7 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(`Forbidden relation. Note can't be a child of own child`); }); - test('Return 400 when parent note does not exist', async () => { + test('Return 406 when parent note does not exist', async () => { const nonExistentParentId = '47L43yY7dp'; const childNote = await global.db.insertNote({ @@ -1915,9 +1915,9 @@ describe('Note API', () => { url: `/note/${childNote.publicId}/relation`, }); - expect(response?.statusCode).toBe(400); + expect(response?.statusCode).toBe(406); - expect(response?.json().message).toStrictEqual('Incorrect parent note'); + expect(response?.json().message).toStrictEqual('Parent note not found'); }); test('Return 400 when circular reference occurs', async () => { @@ -2128,7 +2128,7 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual(`Forbidden relation. Note can't be a child of own child`); }); - test('Return 400 when parent note does not exist', async () => { + test('Return 406 when parent note does not exist', async () => { const nonExistentParentId = '47L43yY7dp'; const childNote = await global.db.insertNote({ @@ -2146,9 +2146,9 @@ describe('Note API', () => { url: `/note/${childNote.publicId}/relation`, }); - expect(response?.statusCode).toBe(400); + expect(response?.statusCode).toBe(406); - expect(response?.json().message).toStrictEqual('Incorrect parent note Id'); + expect(response?.json().message).toStrictEqual('Parent note not found'); }); test('Return 400 when circular reference occurs', async () => { diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index 68f4d211..7dfc05fe 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -8,7 +8,6 @@ import useParentNoteResolver from '../middlewares/note/useParentNoteResolver.js' import useNoteSettingsResolver from '../middlewares/noteSettings/useNoteSettingsResolver.js'; import useMemberRoleResolver from '../middlewares/noteSettings/useMemberRoleResolver.js'; import { MemberRole } from '@domain/entities/team.js'; -import { DomainError } from '@domain/entities/DomainError.js'; import { type NotePublic, definePublicNote } from '@domain/entities/notePublic.js'; import type NoteVisitsService from '@domain/service/noteVisits.js'; import type EditorToolsService from '@domain/service/editorTools.js'; @@ -80,30 +79,6 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const { memberRoleResolver } = useMemberRoleResolver(noteSettingsService); - /** - * Checks if user has a Write role in the note that is going to become a parent - * If parent note does not exist, returns true, so note service could reply with its own error - * @param userId - id of the user - * @param parentPublicId - public id of the parent note - */ - async function userCanEditParentNote(userId: number, parentPublicId: NotePublicId): Promise { - let parentNote: Note; - - try { - parentNote = await noteService.getNoteByPublicId(parentPublicId); - } catch (error) { - if (error instanceof DomainError) { - return true; - } - - throw error; - } - - const memberRole = await noteSettingsService.getUserRoleByUserIdAndNoteId(userId, parentNote.id); - - return memberRole === MemberRole.Write; - } - /** * Get note by id */ @@ -467,22 +442,16 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don policy: [ 'authRequired', 'userCanEdit', + 'userCanEditParentNote', ], }, preHandler: [ noteResolver, + parentNoteResolver, ], }, async (request, reply) => { const noteId = request.note?.id as number; const parentNoteId = request.body.parentNoteId; - const { userId } = request; - - /** - * User should have a Write role in the parent note too - */ - if (!await userCanEditParentNote(userId!, parentNoteId)) { - return reply.forbidden(); - } const parentNote = await noteService.createNoteRelation(noteId, parentNoteId); @@ -530,22 +499,16 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don policy: [ 'authRequired', 'userCanEdit', + 'userCanEditParentNote', ], }, preHandler: [ noteResolver, + parentNoteResolver, ], }, async (request, reply) => { const noteId = request.note?.id as number; const parentNoteId = request.body.parentNoteId; - const { userId } = request; - - /** - * User should have a Write role in the new parent note too - */ - if (!await userCanEditParentNote(userId!, parentNoteId)) { - return reply.forbidden(); - } const isUpdated = await noteService.updateNoteRelation(noteId, parentNoteId); From e364bfdd3cea5b17a4e1692a682b0a42931e91f5 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 04:08:58 +0300 Subject: [PATCH 3/3] test: expect parent note 403 message in relation routes --- src/presentation/http/router/note.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 156a4960..e33fd540 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1851,7 +1851,7 @@ describe('Note API', () => { expect(response?.statusCode).toBe(403); - expect(response?.json().message).toStrictEqual('Permission denied'); + expect(response?.json().message).toStrictEqual('You don\'t have permission to edit the parent note'); response = await global.api?.fakeRequest({ method: 'GET', @@ -2055,7 +2055,7 @@ describe('Note API', () => { expect(response?.statusCode).toBe(403); - expect(response?.json().message).toStrictEqual('Permission denied'); + expect(response?.json().message).toStrictEqual('You don\'t have permission to edit the parent note'); response = await global.api?.fakeRequest({ method: 'GET',