From 8246144b396e73c61da463e9860b92afa6bbc151 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 02:56:48 +0300 Subject: [PATCH 1/5] fix: check edit permission on parent note in POST /note Creating a note with parentId only checked that the parent exists, and only after the child note was already saved. Any logged-in user could attach a note to someone else's note tree. Now the handler resolves the parent first and requires a Write role on it (same rule as the userCanEdit policy), replying 403 otherwise. Co-Authored-By: Claude Opus 5.5 --- src/presentation/http/router/note.test.ts | 75 +++++++++++++++++++++++ src/presentation/http/router/note.ts | 12 ++++ 2 files changed, 87 insertions(+) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 575932ac..6e8e4150 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1019,6 +1019,81 @@ describe('Note API', () => { id: 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, + }, + ]) + ('Should not create note with parentId if user can not edit the parent note', async ({ role }) => { + /** Create test user - creator of a parent note */ + const owner = await global.db.insertUser(); + + /** Create test user */ + const user = await global.db.insertUser(); + + /** Create test parent note */ + const parentNote = await global.db.insertNote({ + creatorId: owner.id, + }); + + /** Create test note settings */ + await global.db.insertNoteSetting({ + noteId: parentNote.id, + isPublic: true, + }); + + /** Create test team */ + if (role !== null) { + await global.db.insertNoteTeam({ + noteId: parentNote.id, + userId: user.id, + role: role, + }); + } + + const accessToken = global.auth(user.id); + + const response = await global.api?.fakeRequest({ + method: 'POST', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note`, + body: { + parentId: parentNote.publicId, + content: { + blocks: [ + { + id: 'qxnjUh9muR', + type: 'header', + data: { + text: 'sample text', + level: 1, + }, + }, + ], + }, + tools: [ + { + name: headerTool.name, + id: headerTool.id, + }, + ], + }, + }); + + expect(response?.statusCode).toBe(403); + + expect(response?.json().message).toStrictEqual('Permission denied'); + }); + test.each([ /** * Specified extra tools diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index f6cb0492..f0432dad 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -297,6 +297,18 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don const parentId = request.body.parentId; const noteTools = request.body.tools; + /** + * If note is created as a child, user should have a Write role in the parent note + */ + if (parentId !== undefined) { + const parentNote = await noteService.getNoteByPublicId(parentId); + const memberRole = await noteSettingsService.getUserRoleByUserIdAndNoteId(userId!, parentNote.id); + + if (memberRole !== MemberRole.Write) { + return reply.forbidden(); + } + } + await noteService.validateNoteTools(noteTools, content); const addedNote = await noteService.addNote(content as Note['content'], userId as number, parentId, noteTools); // "authRequired" policy ensures that userId is not null From aa80dfc1acba041cc124ce1dfc765e79ca40640b Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 03:36:28 +0300 Subject: [PATCH 2/5] test: check POST /note with unknown parentId saves nothing Co-Authored-By: Claude Opus 5.5 --- src/presentation/http/router/note.test.ts | 48 +++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index 6e8e4150..c34a2e6c 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1094,6 +1094,54 @@ describe('Note API', () => { expect(response?.json().message).toStrictEqual('Permission denied'); }); + test('Should not create note if parent note does not exist', async () => { + const nonExistentParentId = '47L43yY7dp'; + + const user = await global.db.insertUser(); + + const accessToken = global.auth(user.id); + + const response = await global.api?.fakeRequest({ + method: 'POST', + headers: { + authorization: `Bearer ${accessToken}`, + }, + url: `/note`, + body: { + parentId: nonExistentParentId, + content: { + blocks: [ + { + id: 'qxnjUh9muR', + type: 'header', + data: { + text: 'sample text', + level: 1, + }, + }, + ], + }, + tools: [ + { + name: headerTool.name, + id: headerTool.id, + }, + ], + }, + }); + + expect(response?.statusCode).toBe(400); + + expect(response?.json().message).toStrictEqual(`Note with public id ${nonExistentParentId} was not found`); + + /** Check that neither the note nor its history record was saved */ + const [notes] = await global.db.query(`SELECT id FROM public.notes WHERE creator_id = ${user.id}`) as [unknown[], unknown]; + const [historyRecords] = await global.db.query(`SELECT id FROM public.note_history WHERE user_id = ${user.id}`) as [unknown[], unknown]; + + expect(notes).toHaveLength(0); + expect(historyRecords).toHaveLength(0); + }); + test.each([ /** * Specified extra tools From 7b3c55109152bf64e1006a73a8b096f30f314b5f Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 03:52:24 +0300 Subject: [PATCH 3/5] refactor: move parent note permission check to a policy Add parentNoteResolver middleware, which resolves the parent note from "parentId" or "parentNoteId" in the request body, and the userCanEditParentNote policy, which requires a Write role on it. POST /note uses them instead of the inline check. An unknown parent note now gets 406 "Parent note not found" from the resolver, the same way noteResolver handles an unknown note. Co-Authored-By: Claude Opus 5.5 --- src/presentation/http/fastify.d.ts | 2 +- .../middlewares/note/useParentNoteResolver.ts | 56 +++++++++++++++++++ src/presentation/http/policies/index.ts | 2 + .../http/policies/userCanEditParentNote.ts | 46 +++++++++++++++ src/presentation/http/router/note.test.ts | 4 +- src/presentation/http/router/note.ts | 23 ++++---- 6 files changed, 118 insertions(+), 15 deletions(-) create mode 100644 src/presentation/http/middlewares/note/useParentNoteResolver.ts create mode 100644 src/presentation/http/policies/userCanEditParentNote.ts diff --git a/src/presentation/http/fastify.d.ts b/src/presentation/http/fastify.d.ts index db57f946..fa03d1a6 100644 --- a/src/presentation/http/fastify.d.ts +++ b/src/presentation/http/fastify.d.ts @@ -69,7 +69,7 @@ declare module 'fastify' { memberRole?: MemberRole; /** - * This property added by noteRelationshipResolver middleware + * This property added by parentNoteResolver middleware */ parentNote: Note | null; } diff --git a/src/presentation/http/middlewares/note/useParentNoteResolver.ts b/src/presentation/http/middlewares/note/useParentNoteResolver.ts new file mode 100644 index 00000000..e84a1404 --- /dev/null +++ b/src/presentation/http/middlewares/note/useParentNoteResolver.ts @@ -0,0 +1,56 @@ +import type { FastifyRequest, preHandlerHookHandler } from 'fastify'; +import type NoteService from '@domain/service/note.js'; +import { notEmpty } from '@infrastructure/utils/empty.js'; +import hasProperty from '@infrastructure/utils/hasProperty.js'; +import { getRequestLogger } from '@infrastructure/logging/index.js'; +import type { NotePublicId } from '@domain/entities/note.js'; + +/** + * Add middleware for resolve parent note passed in request body and add it to request + * @param noteService - note domain service + */ +export default function useParentNoteResolver(noteService: NoteService): { + /** + * Resolve parent note by public id passed in request body and add it to request + * If request body has no parent note id, nothing is resolved + * + * Use this middleware as "preHandler" hook with a particular route + */ + parentNoteResolver: preHandlerHookHandler; +} { + /** + * Get parent note public id from request body + * POST /note accepts it as "parentId", relation routes accept it as "parentNoteId" + * @param requestData - request body + */ + function getParentNotePublicId(requestData: FastifyRequest['body']): NotePublicId | undefined { + if (hasProperty(requestData, 'parentId') && notEmpty(requestData.parentId)) { + return requestData.parentId as NotePublicId; + } + + if (hasProperty(requestData, 'parentNoteId') && notEmpty(requestData.parentNoteId)) { + return requestData.parentNoteId as NotePublicId; + } + } + + return { + parentNoteResolver: async function parentNoteResolver(request, reply) { + const logger = getRequestLogger('middlewares').child({ middleware: 'parentNoteResolver' }); + const parentPublicId = getParentNotePublicId(request.body); + + if (parentPublicId === undefined) { + return; + } + + try { + request.parentNote = await noteService.getNoteByPublicId(parentPublicId); + logger.debug('Parent note resolved by public ID'); + } catch (error) { + logger.error('Invalid parent note public id passed'); + logger.error(error); + + await reply.notAcceptable('Parent note not found'); + } + }, + }; +} diff --git a/src/presentation/http/policies/index.ts b/src/presentation/http/policies/index.ts index 0e2abf01..f8894411 100644 --- a/src/presentation/http/policies/index.ts +++ b/src/presentation/http/policies/index.ts @@ -1,9 +1,11 @@ import authRequired from './authRequired.js'; import notePublicOrUserInTeam from './notePublicOrUserInTeam.js'; import userCanEdit from './userCanEdit.js'; +import userCanEditParentNote from './userCanEditParentNote.js'; export default { authRequired, notePublicOrUserInTeam, userCanEdit, + userCanEditParentNote, }; diff --git a/src/presentation/http/policies/userCanEditParentNote.ts b/src/presentation/http/policies/userCanEditParentNote.ts new file mode 100644 index 00000000..0736a241 --- /dev/null +++ b/src/presentation/http/policies/userCanEditParentNote.ts @@ -0,0 +1,46 @@ +import { isEmpty } from '@infrastructure/utils/empty.js'; +import { MemberRole } from '@domain/entities/team.js'; +import type { PolicyContext } from '@presentation/http/types/PolicyContext.js'; +import { getRequestLogger } from '@infrastructure/logging/index.js'; + +/** + * Policy to check whether a user has permission to edit the parent note passed in request body + * Parent note should be resolved by parentNoteResolver middleware + * @param context - Context object, containing Fatify request, Fastify reply and domain services + */ +export default async function userCanEditParentNote(context: PolicyContext): Promise { + const { request, reply, domainServices } = context; + const logger = getRequestLogger('policies').child({ policy: 'userCanEditParentNote' }); + + const { userId } = request; + + /** + * If user is not authorized, we can't check his permissions + */ + if (isEmpty(userId)) { + logger.warn('User not authenticated for parent note edit access'); + + return await reply.unauthorized(); + }; + + /** + * If parent note is not passed, there is nothing to check + */ + if (isEmpty(request.parentNote)) { + return; + }; + + const memberRole = await domainServices.noteSettingsService.getUserRoleByUserIdAndNoteId(request.userId!, request.parentNote.id); + + /** + * If user has a Read Role or is not in team at all, + * he doesn't have permission to add child notes to the parent note + */ + if (memberRole !== MemberRole.Write) { + logger.warn('User does not have write permission for parent note'); + + return await reply.forbidden(); + } + + logger.debug('User parent note edit permission check completed successfully'); +} diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index c34a2e6c..fac2517e 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1130,9 +1130,9 @@ describe('Note API', () => { }, }); - expect(response?.statusCode).toBe(400); + expect(response?.statusCode).toBe(406); - expect(response?.json().message).toStrictEqual(`Note with public id ${nonExistentParentId} was not found`); + expect(response?.json().message).toStrictEqual('Parent note not found'); /** Check that neither the note nor its history record was saved */ const [notes] = await global.db.query(`SELECT id FROM public.notes WHERE creator_id = ${user.id}`) as [unknown[], unknown]; diff --git a/src/presentation/http/router/note.ts b/src/presentation/http/router/note.ts index f0432dad..383b86e3 100644 --- a/src/presentation/http/router/note.ts +++ b/src/presentation/http/router/note.ts @@ -4,6 +4,7 @@ import type NoteSettingsService from '@domain/service/noteSettings.js'; import type { ErrorResponse } from '@presentation/http/types/HttpResponse.js'; import type { Note, NotePublicId } from '@domain/entities/note.js'; import useNoteResolver from '../middlewares/note/useNoteResolver.js'; +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'; @@ -60,6 +61,12 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don */ const { noteResolver } = useNoteResolver(noteService); + /** + * Prepare parent note resolver middleware + * It should be used in routes that accepts parent note public id in request body + */ + const { parentNoteResolver } = useParentNoteResolver(noteService); + /** * Prepare note settings resolver middleware * It should be used to use note settings in middlewares @@ -262,6 +269,7 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don config: { policy: [ 'authRequired', + 'userCanEditParentNote', ], }, schema: { @@ -288,6 +296,9 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don }, }, }, + preHandler: [ + parentNoteResolver, + ], }, async (request, reply) => { /** * @todo Validate request query @@ -297,18 +308,6 @@ const NoteRouter: FastifyPluginCallback = (fastify, opts, don const parentId = request.body.parentId; const noteTools = request.body.tools; - /** - * If note is created as a child, user should have a Write role in the parent note - */ - if (parentId !== undefined) { - const parentNote = await noteService.getNoteByPublicId(parentId); - const memberRole = await noteSettingsService.getUserRoleByUserIdAndNoteId(userId!, parentNote.id); - - if (memberRole !== MemberRole.Write) { - return reply.forbidden(); - } - } - await noteService.validateNoteTools(noteTools, content); const addedNote = await noteService.addNote(content as Note['content'], userId as number, parentId, noteTools); // "authRequired" policy ensures that userId is not null From 1067b125e899612bbdb53f1e15443f57cf666baa Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 04:07:28 +0300 Subject: [PATCH 4/5] refactor: drop debug logs and add message to parent note 403 --- .../http/middlewares/note/useParentNoteResolver.ts | 1 - src/presentation/http/policies/userCanEditParentNote.ts | 4 +--- src/presentation/http/router/note.test.ts | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/presentation/http/middlewares/note/useParentNoteResolver.ts b/src/presentation/http/middlewares/note/useParentNoteResolver.ts index e84a1404..8af33bc8 100644 --- a/src/presentation/http/middlewares/note/useParentNoteResolver.ts +++ b/src/presentation/http/middlewares/note/useParentNoteResolver.ts @@ -44,7 +44,6 @@ export default function useParentNoteResolver(noteService: NoteService): { try { request.parentNote = await noteService.getNoteByPublicId(parentPublicId); - logger.debug('Parent note resolved by public ID'); } catch (error) { logger.error('Invalid parent note public id passed'); logger.error(error); diff --git a/src/presentation/http/policies/userCanEditParentNote.ts b/src/presentation/http/policies/userCanEditParentNote.ts index 0736a241..0d78b490 100644 --- a/src/presentation/http/policies/userCanEditParentNote.ts +++ b/src/presentation/http/policies/userCanEditParentNote.ts @@ -39,8 +39,6 @@ export default async function userCanEditParentNote(context: PolicyContext): Pro if (memberRole !== MemberRole.Write) { logger.warn('User does not have write permission for parent note'); - return await reply.forbidden(); + return await reply.forbidden('You don\'t have permission to edit the parent note'); } - - logger.debug('User parent note edit permission check completed successfully'); } diff --git a/src/presentation/http/router/note.test.ts b/src/presentation/http/router/note.test.ts index fac2517e..2f475ac5 100644 --- a/src/presentation/http/router/note.test.ts +++ b/src/presentation/http/router/note.test.ts @@ -1091,7 +1091,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'); }); test('Should not create note if parent note does not exist', async () => { From 03aa8a79aad33051f1ba82d20fa00f7fe0bdc1c0 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Wed, 21 Oct 2026 04:13:58 +0300 Subject: [PATCH 5/5] fix: reject parent note request if parent note is not resolved userCanEditParentNote skipped the check whenever request.parentNote was empty. If a route used the policy without parentNoteResolver, every request with a parent note id would pass unchecked. Now the policy checks the body for "parentId" or "parentNoteId" itself and replies 406 "Parent note not found" if the id is there but the parent note is not resolved. --- .../policies/userCanEditParentNote.test.ts | 63 +++++++++++++++++++ .../http/policies/userCanEditParentNote.ts | 23 ++++++- 2 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 src/presentation/http/policies/userCanEditParentNote.test.ts diff --git a/src/presentation/http/policies/userCanEditParentNote.test.ts b/src/presentation/http/policies/userCanEditParentNote.test.ts new file mode 100644 index 00000000..23851c3a --- /dev/null +++ b/src/presentation/http/policies/userCanEditParentNote.test.ts @@ -0,0 +1,63 @@ +import { describe, test, expect, vi } from 'vitest'; +import type { PolicyContext } from '@presentation/http/types/PolicyContext.js'; +import userCanEditParentNote from './userCanEditParentNote.js'; + +describe('userCanEditParentNote policy', () => { + test.each([ + /** POST /note passes parent note as "parentId" */ + { + body: { parentId: '47L43yY7dp' }, + }, + + /** Relation routes pass parent note as "parentNoteId" */ + { + body: { parentNoteId: '47L43yY7dp' }, + }, + ]) + ('Replies 406 if parent note id is passed but parent note is not resolved', async ({ body }) => { + const reply = { + notAcceptable: vi.fn(), + forbidden: vi.fn(), + }; + const getUserRoleByUserIdAndNoteId = vi.fn(); + + /** Request of a route without parentNoteResolver: request.parentNote is not set */ + await userCanEditParentNote({ + request: { + userId: 1, + body, + }, + reply, + domainServices: { + noteSettingsService: { getUserRoleByUserIdAndNoteId }, + }, + } as unknown as PolicyContext); + + expect(reply.notAcceptable).toHaveBeenCalledWith('Parent note not found'); + expect(reply.forbidden).not.toHaveBeenCalled(); + expect(getUserRoleByUserIdAndNoteId).not.toHaveBeenCalled(); + }); + + test('Does nothing if parent note id is not passed', async () => { + const reply = { + notAcceptable: vi.fn(), + forbidden: vi.fn(), + }; + const getUserRoleByUserIdAndNoteId = vi.fn(); + + await userCanEditParentNote({ + request: { + userId: 1, + body: {}, + }, + reply, + domainServices: { + noteSettingsService: { getUserRoleByUserIdAndNoteId }, + }, + } as unknown as PolicyContext); + + expect(reply.notAcceptable).not.toHaveBeenCalled(); + expect(reply.forbidden).not.toHaveBeenCalled(); + expect(getUserRoleByUserIdAndNoteId).not.toHaveBeenCalled(); + }); +}); diff --git a/src/presentation/http/policies/userCanEditParentNote.ts b/src/presentation/http/policies/userCanEditParentNote.ts index 0d78b490..ab067dd2 100644 --- a/src/presentation/http/policies/userCanEditParentNote.ts +++ b/src/presentation/http/policies/userCanEditParentNote.ts @@ -1,4 +1,5 @@ -import { isEmpty } from '@infrastructure/utils/empty.js'; +import { isEmpty, notEmpty } from '@infrastructure/utils/empty.js'; +import hasProperty from '@infrastructure/utils/hasProperty.js'; import { MemberRole } from '@domain/entities/team.js'; import type { PolicyContext } from '@presentation/http/types/PolicyContext.js'; import { getRequestLogger } from '@infrastructure/logging/index.js'; @@ -12,7 +13,13 @@ export default async function userCanEditParentNote(context: PolicyContext): Pro const { request, reply, domainServices } = context; const logger = getRequestLogger('policies').child({ policy: 'userCanEditParentNote' }); - const { userId } = request; + const { userId, body } = request; + + /** + * POST /note passes parent note as "parentId", relation routes pass it as "parentNoteId" + */ + const hasParentNoteId = (hasProperty(body, 'parentId') && notEmpty(body.parentId)) + || (hasProperty(body, 'parentNoteId') && notEmpty(body.parentNoteId)); /** * If user is not authorized, we can't check his permissions @@ -26,10 +33,20 @@ export default async function userCanEditParentNote(context: PolicyContext): Pro /** * If parent note is not passed, there is nothing to check */ - if (isEmpty(request.parentNote)) { + if (!hasParentNoteId) { return; }; + /** + * If parent note is passed but not resolved, we can't check permissions + * It happens if the route uses this policy without parentNoteResolver middleware + */ + if (isEmpty(request.parentNote)) { + logger.warn('Parent note not found for parent note edit permission check'); + + return await reply.notAcceptable('Parent note not found'); + }; + const memberRole = await domainServices.noteSettingsService.getUserRoleByUserIdAndNoteId(request.userId!, request.parentNote.id); /**