Skip to content

Commit

Permalink
Merge pull request #182 from codex-team/feat/note-team-role-access-po…
Browse files Browse the repository at this point in the history
…licy

feat: note team role access policy
  • Loading branch information
GoldenJaden authored Feb 23, 2024
2 parents 0dd5192 + 81ca782 commit 772d6dc
Show file tree
Hide file tree
Showing 19 changed files with 231 additions and 68 deletions.
4 changes: 2 additions & 2 deletions src/domain/entities/team.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ export enum MemberRole {
/**
* Team member can only read notes
*/
read = 0,
Read = 0,

/**
* Team member can read and write notes
*/
write = 1,
Write = 1,
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/domain/service/noteSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default class NoteSettingsService {
* @param userId - user to add
*/
public async addUserToTeamByInvitationHash(invitationHash: InvitationHash, userId: User['id']): Promise<TeamMember | null> {
const defaultUserRole = MemberRole.read;
const defaultUserRole = MemberRole.Read;
const noteSettings = await this.noteSettingsRepository.getNoteSettingsByInvitationHash(invitationHash);

/**
Expand Down Expand Up @@ -120,7 +120,7 @@ export default class NoteSettingsService {
* @param userId - user id to check his role
* @param noteId - note id where user should have role
*/
public async getUserRoleByUserIdAndNoteId(userId: User['id'], noteId: NoteInternalId): Promise<MemberRole | null> {
public async getUserRoleByUserIdAndNoteId(userId: User['id'], noteId: NoteInternalId): Promise<MemberRole | undefined> {
return await this.teamRepository.getUserRoleByUserIdAndNoteId(userId, noteId);
}

Expand Down Expand Up @@ -182,7 +182,7 @@ export default class NoteSettingsService {
* @param role - new team member role
* @returns returns 1 if the role has been changed and 0 otherwise
*/
public async patchMemberRoleByUserId(id: TeamMember['id'], noteId: NoteInternalId, role: MemberRole): Promise<MemberRole | null> {
public async patchMemberRoleByUserId(id: TeamMember['id'], noteId: NoteInternalId, role: MemberRole): Promise<MemberRole | undefined> {
return await this.teamRepository.patchMemberRoleByUserId(id, noteId, role);
}
}
2 changes: 1 addition & 1 deletion src/infrastructure/utils/hasProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
*/
export default function hasProperty<Obj, Name extends string>(obj: Obj, name: Name): obj is Obj & Record<Name, unknown> {
return typeof obj === 'object' && obj !== null && name in obj;
}
}
6 changes: 6 additions & 0 deletions src/presentation/http/fastify.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type Policies from './policies/index.js';
import type AuthPayload from '@domain/entities/authPayload.js';
import type { Note } from '@domain/entities/note.js';
import type NoteSettings from '@domain/entities/noteSettings.js';
import type { MemberRole } from '@domain/entities/team.js';

declare module 'fastify' {
export interface FastifyInstance<
Expand Down Expand Up @@ -63,6 +64,11 @@ declare module 'fastify' {
*/
noteSettings: NoteSettings | null;

/**
* This property added by userTeamRoleResolver middleware
*/
memberRole?: MemberRole;

/**
* This property added by noteRelationshipResolver middleware
*/
Expand Down
11 changes: 8 additions & 3 deletions src/presentation/http/http-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export default class HttpApi implements Api {

this.addSchema();
this.addDecorators();
this.addPoliciesCheckHook();

await this.addPoliciesCheckHook(domainServices);
await this.addApiRoutes(domainServices);

this.domainErrorHandler();
Expand Down Expand Up @@ -310,8 +310,10 @@ export default class HttpApi implements Api {

/**
* Add "onRoute" hook that will add "preHandler" checking policies passed through the route config
*
* @param domainServices - instances of domain services
*/
private addPoliciesCheckHook(): void {
private addPoliciesCheckHook(domainServices: DomainServices): void {
this.server?.addHook('onRoute', (routeOptions) => {
const policies = routeOptions.config?.policy ?? [];

Expand All @@ -330,7 +332,10 @@ export default class HttpApi implements Api {

routeOptions.preHandler.push(async (request, reply) => {
for (const policy of policies) {
await Policies[policy](request, reply);
await Policies[policy]({
request,
reply,
domainServices });
}
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import type { preHandlerHookHandler } from 'fastify';
import { getLogger } from '@infrastructure/logging/index.js';
import type NoteSettingsService from '@domain/service/noteSettings.js';
import type { MemberRole } from '@domain/entities/team';
import { isEmpty } from '@infrastructure/utils/empty.js';

/**
* Add middleware to resolve Member's role in a team by user id and note id and add it to request
*
* @param noteSettingsService - note settings domain service
*/
export default function useMemberRoleResolver(noteSettingsService: NoteSettingsService): {
/**
* Resolve Member's role in a team by user id and note id and add it to request
*
* Use this middleware as "preHandler" hook with a particular route
*/
memberRoleResolver: preHandlerHookHandler;
} {
/**
* Get logger instance
*/
const logger = getLogger('appServer');

return {
memberRoleResolver: async function memberRoleResolver(request, reply) {
/** If MemberRole equals null, it means that user is not in the team or is not authenticated */
let memberRole: MemberRole | undefined;

try {
if (isEmpty(request.note)) {
throw new Error('Note was not resolved');
}

/** If user is not authenticated, we can't resolve his role */
if (isEmpty(request.userId)) {
memberRole = undefined;
} else {
memberRole = await noteSettingsService.getUserRoleByUserIdAndNoteId(request.userId, request.note.id);
}

if (memberRole !== undefined) {
request.memberRole = memberRole;
}
} catch (error) {
logger.error('Can not resolve Member role by note [id = ${request.note.id}] and user [id = ${request.userId}]');
logger.error(error);

await reply.notAcceptable('Team member not found');
}
},
};
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { preHandlerHookHandler } from 'fastify';
import { StatusCodes } from 'http-status-codes';
import { getLogger } from '@infrastructure/logging/index.js';
import type NoteSettingsService from '@domain/service/noteSettings.js';
import type NoteSettings from '@domain/entities/noteSettings';
Expand Down Expand Up @@ -28,7 +27,7 @@ export default function useNoteSettingsResolver(noteSettingsService: NoteSetting

try {
if (!request.note) {
throw new Error('Note did not resolved');
throw new Error('Note was not resolved');
}

noteSettings = await noteSettingsService.getNoteSettingsByNoteId(request.note.id);
Expand All @@ -38,11 +37,7 @@ export default function useNoteSettingsResolver(noteSettingsService: NoteSetting
logger.error('Can not resolve Note settings by note');
logger.error(error);

await reply
.code(StatusCodes.NOT_ACCEPTABLE)
.send({
message: 'Note settings not found',
});
await reply.notAcceptable('Note settings not found');
}
},
};
Expand Down
9 changes: 5 additions & 4 deletions src/presentation/http/policies/authRequired.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { FastifyReply, FastifyRequest } from 'fastify';
import type { PolicyContext } from '@presentation/http/types/PolicyContext';

/**
* Policy to enforce user to be logged in
*
* @param request - Fastify request object
* @param reply - Fastify reply object
* @param context - Context object, containing Fatify request, Fastify reply and domain services
*/
export default async function authRequired(request: FastifyRequest, reply: FastifyReply): Promise<void> {
export default async function authRequired(context: PolicyContext): Promise<void> {
const { request, reply } = context;

const { userId } = request;

if (userId === null) {
Expand Down
6 changes: 4 additions & 2 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 userInTeam from './userInTeam.js';
import userIsCreator from './userIsCreator.js';
import userCanEdit from './userCanEdit.js';

export default {
authRequired,
notePublicOrUserInTeam,
userInTeam,
userIsCreator,
userCanEdit,
};
23 changes: 17 additions & 6 deletions src/presentation/http/policies/notePublicOrUserInTeam.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import type { FastifyReply, FastifyRequest } from 'fastify';
import { isEmpty } from '@infrastructure/utils/empty.js';
import { notEmpty } from '@infrastructure/utils/empty.js';
import type { PolicyContext } from '@presentation/http/types/PolicyContext.js';

/**
* Policy to check does user have permission to access note
*
* @param request - Fastify request object
* @param reply - Fastify reply object
* @param context - Context, object containing Fatify request, Fastify reply and domain services
*/
export default async function notePublicOrUserInTeam(request: FastifyRequest, reply: FastifyReply): Promise<void> {
export default async function notePublicOrUserInTeam(context: PolicyContext): Promise<void> {
const { request, reply, domainServices } = context;

const { userId } = request;

/**
Expand All @@ -19,12 +21,21 @@ export default async function notePublicOrUserInTeam(request: FastifyRequest, re

const { creatorId } = request.note;
const { isPublic } = request.noteSettings;
let memberRole;

/**
* If user is not authorized, we can't check his role
* If note is public, we don't need to check for the role
*/
if (notEmpty(userId) && isPublic === false) {
memberRole = domainServices.noteSettingsService.getUserRoleByUserIdAndNoteId(userId, request.note.id);
}

/**
* If note is public, everyone can access it
* If note is private, only creator can access it
* If note is private, only team member and creator can access it
*/
if (isPublic === false && creatorId !== userId) {
if (isPublic === false && creatorId !== userId && isEmpty(memberRole)) {
return await reply.forbidden();
}
}
40 changes: 40 additions & 0 deletions src/presentation/http/policies/userCanEdit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { isEmpty } from '@infrastructure/utils/empty.js';
import { MemberRole } from '@domain/entities/team';
import type { PolicyContext } from '@presentation/http/types/PolicyContext.js';

/**
* Policy to check whether a user has permission to edit the note
*
* @param context - Context object, containing Fatify request, Fastify reply and domain services
*/
export default async function userCanEdit(context: PolicyContext): Promise<void> {
const { request, reply, domainServices } = context;

const { userId } = request;

/**
* If user is not authorized, we can't check his permissions
*/
if (isEmpty(userId)) {
return await reply.unauthorized();
};

/**
* If note is not resolved, we can't check permissions
*/
if (isEmpty(request.note)) {
return await reply.notAcceptable('Note not found');
};

const { creatorId } = request.note;
const memberRole = await domainServices.noteSettingsService.getUserRoleByUserIdAndNoteId(request.userId!, request.note.id);

/**
* If user is not a creator of the note and
* user has a Read Role or is not in team at all,
* he doesn't have permission to edit the note
*/
if (creatorId !== userId && memberRole !== MemberRole.Write) {
return await reply.forbidden();
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import type { FastifyReply, FastifyRequest } from 'fastify';
import { isEmpty } from '@infrastructure/utils/empty.js';
import type { PolicyContext } from '@presentation/http/types/PolicyContext.js';


/**
* Policy to check whether user in a team of note
* Policy to check whether a user is a creator of the note
*
* @param request - Fastify request object
* @param reply - Fastify reply object
* @param context - Context object, containing Fatify request, Fastify reply and domain services
*/
export default async function userInTeam(request: FastifyRequest, reply: FastifyReply): Promise<void> {
export default async function userIsCreator(context: PolicyContext): Promise<void> {
const { request, reply } = context;

const { userId } = request;

if (isEmpty(userId)) {
Expand All @@ -25,7 +26,7 @@ export default async function userInTeam(request: FastifyRequest, reply: Fastify
const { creatorId } = request.note;

/**
* Check if user can edit note
* Check if user is a creator of the note
*/
if (creatorId !== userId) {
return await reply.forbidden();
Expand Down
13 changes: 10 additions & 3 deletions src/presentation/http/router/note.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, test, expect } from 'vitest';

import notes from '@tests/test-data/notes.json';
import noteSettings from '@tests/test-data/notes-settings.json';
import noteTeams from '@tests/test-data/note-teams.json';

describe('Note API', () => {
describe('GET note/resolve-hostname/:hostname ', () => {
Expand Down Expand Up @@ -157,14 +158,15 @@ describe('Note API', () => {
expect(response?.json()).toStrictEqual({ message: 'Permission denied' });
});

test('Returns 403 when public access is disabled, user is not creator of the note', async () => {
test('Returns 403 when public access is disabled, user is not creator of the note and is not in the team', async () => {
const userId = 2;
const accessToken = global.auth(userId);

const notPublicNote = notes.find(newNote => {
const settings = noteSettings.find(ns => ns.note_id === newNote.id);
const team = noteTeams.find(nt => nt.note_id === newNote.id && nt.user_id === userId);

return settings!.is_public === false && newNote.creator_id != userId;
return settings!.is_public === false && newNote.creator_id !== userId && team === undefined;
});

const response = await global.api?.fakeRequest({
Expand Down Expand Up @@ -263,6 +265,9 @@ describe('Note API', () => {

expect(response?.json().accessRights).toStrictEqual({ canEdit: true });
});

test.todo('Returns 200 when note is private, user is in team but is not creator');
test.todo('Returns canEdit=true, when user is authorized, is in the team and is not creator');
});
});

Expand Down Expand Up @@ -338,7 +343,9 @@ describe('Note API', () => {
expect(response?.json().message).toStrictEqual(expectedMessage);
});

test.todo('Returns 400 when parentId has incorrect characters and lenght');
test.todo('Returns 400 when parentId has incorrect characters and length');
test.todo('Return 200 when user is not the creator, but a team member with a Write role');
test.todo('Return 403 when user has no Write role and he is not a creator');
});

describe('POST /note', () => {
Expand Down
Loading

0 comments on commit 772d6dc

Please sign in to comment.