Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/presentation/http/fastify.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ declare module 'fastify' {
memberRole?: MemberRole;

/**
* This property added by noteRelationshipResolver middleware
* This property added by parentNoteResolver middleware
*/
parentNote: Note | null;
}
Expand Down
55 changes: 55 additions & 0 deletions src/presentation/http/middlewares/note/useParentNoteResolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
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);
} catch (error) {
logger.error('Invalid parent note public id passed');
logger.error(error);

await reply.notAcceptable('Parent note not found');
}
},
};
}
2 changes: 2 additions & 0 deletions src/presentation/http/policies/index.ts
Original file line number Diff line number Diff line change
@@ -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,
};
63 changes: 63 additions & 0 deletions src/presentation/http/policies/userCanEditParentNote.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
61 changes: 61 additions & 0 deletions src/presentation/http/policies/userCanEditParentNote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
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';

/**
* 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<void> {
const { request, reply, domainServices } = context;
const logger = getRequestLogger('policies').child({ policy: 'userCanEditParentNote' });

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
*/
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 (!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);

/**
* 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('You don\'t have permission to edit the parent note');
}
}
123 changes: 123 additions & 0 deletions src/presentation/http/router/note.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,129 @@ 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('You don\'t have permission to edit the parent note');
});

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(406);

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];
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
Expand Down
11 changes: 11 additions & 0 deletions src/presentation/http/router/note.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -60,6 +61,12 @@ const NoteRouter: FastifyPluginCallback<NoteRouterOptions> = (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
Expand Down Expand Up @@ -262,6 +269,7 @@ const NoteRouter: FastifyPluginCallback<NoteRouterOptions> = (fastify, opts, don
config: {
policy: [
'authRequired',
'userCanEditParentNote',
],
},
schema: {
Expand All @@ -288,6 +296,9 @@ const NoteRouter: FastifyPluginCallback<NoteRouterOptions> = (fastify, opts, don
},
},
},
preHandler: [
parentNoteResolver,
],
}, async (request, reply) => {
/**
* @todo Validate request query
Expand Down
Loading