From b6217a40075544d3b278ca5133dd81a1fce68d06 Mon Sep 17 00:00:00 2001 From: Sergey Yarkov Date: Wed, 8 Jun 2022 23:08:09 +0300 Subject: [PATCH] refactor: moved bouncer actions to policies --- .../Http/Api/v1/UsersController.ts | 2 +- app/Policies/LessonPolicy.ts | 35 +++++++++ app/Policies/RolePolicy.ts | 36 ++++++++++ app/Services/CourseService.ts | 14 ++-- app/Services/LessonService.ts | 71 ++----------------- app/Services/UserService.ts | 41 ++++------- start/bouncer.ts | 27 ++----- start/routes/apis/v1/users.ts | 2 +- 8 files changed, 106 insertions(+), 122 deletions(-) create mode 100644 app/Policies/LessonPolicy.ts create mode 100644 app/Policies/RolePolicy.ts diff --git a/app/Controllers/Http/Api/v1/UsersController.ts b/app/Controllers/Http/Api/v1/UsersController.ts index 3f03c28..3219071 100644 --- a/app/Controllers/Http/Api/v1/UsersController.ts +++ b/app/Controllers/Http/Api/v1/UsersController.ts @@ -93,7 +93,7 @@ export default class UsersController extends BaseController { */ public async delete(ctx: HttpContextContract) { - const result = await this.userService.deleteUser(ctx.params.id); + const result = await this.userService.deleteUser(ctx.params.id, ctx); if (!result.success && result.error) { throw new Exception(result.message, result.status, result.error.code); diff --git a/app/Policies/LessonPolicy.ts b/app/Policies/LessonPolicy.ts new file mode 100644 index 0000000..16cb1fd --- /dev/null +++ b/app/Policies/LessonPolicy.ts @@ -0,0 +1,35 @@ +import { BasePolicy } from '@ioc:Adonis/Addons/Bouncer'; + +/** + * Helpers + */ +import RoleHelper from 'App/Helpers/RoleHelper'; + +/** + * Models + */ +import Lesson from 'App/Models/Lesson'; +import User from 'App/Models/User'; + +/** + * Datatypes + */ +import RoleEnum from 'App/Datatypes/Enums/RoleEnum'; + +export default class LessonPolicy extends BasePolicy { + public async view(user: User, lesson: Lesson) { + /** + * Load required data to check permissions + */ + await user.load(loader => loader.load('roles').load('courses')); + await lesson.load('course'); + + const isAdminOrTeacher = RoleHelper.userContainRoles(user.roles, [RoleEnum.ADMIN, RoleEnum.TEACHER]); + + if (isAdminOrTeacher) { + return true; + } + + return !!user.courses.find(course => course.id === lesson.course.id); + } +} diff --git a/app/Policies/RolePolicy.ts b/app/Policies/RolePolicy.ts new file mode 100644 index 0000000..10840a8 --- /dev/null +++ b/app/Policies/RolePolicy.ts @@ -0,0 +1,36 @@ +import { BasePolicy } from '@ioc:Adonis/Addons/Bouncer'; + +/** + * Models + */ +import User from 'App/Models/User'; + +/** + * Helpers + */ +import RoleHelper from 'App/Helpers/RoleHelper'; + +/** + * Datatypes + */ +import RoleEnum from 'App/Datatypes/Enums/RoleEnum'; + +export default class RolePolicy extends BasePolicy { + public async manage(user: User, roles: Array) { + await user.load('roles'); + + const isAdmin = RoleHelper.userContainRoles(user.roles, [RoleEnum.ADMIN]); + const isTeacher = RoleHelper.userContainRoles(user.roles, [RoleEnum.TEACHER]); + + if (isAdmin) { + return true; + } + + /** + * User with role `TEACHER` can manage only user with `STUDENT` role + */ + if (!(roles.includes(RoleEnum.TEACHER) || roles.includes(RoleEnum.ADMIN)) && isTeacher) { + return true; + } + } +} diff --git a/app/Services/CourseService.ts b/app/Services/CourseService.ts index 9e7903e..35b0efa 100644 --- a/app/Services/CourseService.ts +++ b/app/Services/CourseService.ts @@ -234,9 +234,9 @@ export default class CourseService { /** * Find course teacher */ - const teacher = await this.userRepository.getById(data.teacher_id); + const user = await this.userRepository.getById(data.teacher_id); - if (!teacher) { + if (!user) { return { success: false, status: HttpStatusEnum.NOT_FOUND, @@ -248,13 +248,13 @@ export default class CourseService { }; } - const isTeacher = RoleHelper.userHasRoles(teacher.roles, [RoleEnum.TEACHER]); + const isHasPermissions = RoleHelper.userContainRoles(user.roles, [RoleEnum.ADMIN, RoleEnum.TEACHER]); - if (!isTeacher) { + if (!isHasPermissions) { return { success: false, status: HttpStatusEnum.BAD_REQUEST, - message: `User with id "${teacher.id}" not a teacher.`, + message: `User with id "${user.id}" does not have sufficient permissions.`, data: {}, error: { code: 'E_BAD_REQUEST', @@ -359,6 +359,10 @@ export default class CourseService { }; } + // if (data.teacher_id) { + + // } + return { success: true, status: HttpStatusEnum.OK, diff --git a/app/Services/LessonService.ts b/app/Services/LessonService.ts index cf0fe3b..4f3062e 100644 --- a/app/Services/LessonService.ts +++ b/app/Services/LessonService.ts @@ -78,20 +78,7 @@ export default class LessonService { }; } - /** - * Allow user to view lesson content - */ - if (await ctx.bouncer.denies('viewLessonContent', lesson)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'The user is not a student of this course.', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('LessonPolicy').authorize('view', lesson); /** * Load lesson with materials @@ -109,7 +96,8 @@ export default class LessonService { /** * Fetch lesson material by file name * - * @param fileName Name of file + * @param ctx Http context + * @param name Filename * @returns Response */ public async fetchMaterialFile(ctx: HttpContextContract, name: string): Promise> { @@ -131,18 +119,7 @@ export default class LessonService { * Allow user to view lesson material */ await material.load('lesson'); - - if (await ctx.bouncer.denies('viewLessonContent', material.lesson)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'You are not able to view this file.', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('LessonPolicy').authorize('view', material.lesson); return { success: true, @@ -168,18 +145,7 @@ export default class LessonService { } await video.load('lesson'); - - if (await ctx.bouncer.denies('viewLessonContent', video.lesson)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'You are not able to view this file.', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('LessonPolicy').authorize('view', video.lesson); return { success: true, @@ -211,17 +177,7 @@ export default class LessonService { }; } - if (await ctx.bouncer.denies('viewLessonContent', lesson)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'The user is not a student of this course.', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('LessonPolicy').authorize('view', lesson); const progress = await this.lessonProgressRepository.get(user.id, lesson.id); @@ -354,20 +310,7 @@ export default class LessonService { }; } - /** - * Allow user to view lesson content - */ - if (await ctx.bouncer.denies('viewLessonContent', lesson)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'The user is not a student of this course.', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('LessonPolicy').authorize('view', lesson); /** * Load lesson with materials diff --git a/app/Services/UserService.ts b/app/Services/UserService.ts index df95a08..432a828 100644 --- a/app/Services/UserService.ts +++ b/app/Services/UserService.ts @@ -27,6 +27,7 @@ import UserRepository from 'App/Repositories/UserRepository'; */ import CreateUserValidator from 'App/Validators/User/CreateUserValidator'; import UpdateUserValidator from 'App/Validators/User/UpdateUserValidator'; +import RoleEnum from 'App/Datatypes/Enums/RoleEnum'; @inject() export default class UserService { @@ -120,20 +121,7 @@ export default class UserService { }; } - /** - * Check user permissions - */ - if (await ctx.bouncer.denies('manageUserRole', role)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'You dont have permissions to permorm that action', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } + await ctx.bouncer.with('RolePolicy').authorize('manage', [role.slug as RoleEnum]); /** * Create new user @@ -166,7 +154,7 @@ export default class UserService { data: UpdateUserValidator['schema']['props'], ctx: HttpContextContract ): Promise { - const user = await this.userRepository.update(id, data); + const user = await this.userRepository.getById(id); if (!user) { return { @@ -180,6 +168,9 @@ export default class UserService { }; } + await ctx.bouncer.with('RolePolicy').authorize('manage', user.roles.map(r => r.slug) as [RoleEnum]); + await this.userRepository.update(id, data); + /** * Update user role */ @@ -187,18 +178,7 @@ export default class UserService { const role = await this.roleRepository.getBySlug(data.role); if (role) { - if (await ctx.bouncer.denies('manageUserRole', role)) { - return { - success: false, - status: HttpStatusEnum.FORBIDDEN, - message: 'You dont have permissions to permorm that action', - data: {}, - error: { - code: 'E_FORBIDDEN', - }, - }; - } - + await ctx.bouncer.with('RolePolicy').authorize('manage', [data.role]); await this.userRepository.updateRoles(user, [role]); } } @@ -217,8 +197,8 @@ export default class UserService { * @param id User id * @returns Response */ - public async deleteUser(id: string | number): Promise { - const user = await this.userRepository.delete(id); + public async deleteUser(id: string | number, ctx: HttpContextContract): Promise { + const user = await this.userRepository.getById(id); if (!user) { return { @@ -232,6 +212,9 @@ export default class UserService { }; } + await ctx.bouncer.with('RolePolicy').authorize('manage', user.roles.map(r => r.slug) as [RoleEnum]); + await this.userRepository.delete(id); + return { success: true, status: HttpStatusEnum.OK, diff --git a/start/bouncer.ts b/start/bouncer.ts index 6493865..0755d17 100644 --- a/start/bouncer.ts +++ b/start/bouncer.ts @@ -6,11 +6,6 @@ */ import Bouncer from '@ioc:Adonis/Addons/Bouncer'; -import RoleEnum from 'App/Datatypes/Enums/RoleEnum'; -import RoleHelper from 'App/Helpers/RoleHelper'; -import Lesson from 'App/Models/Lesson'; -import Role from 'App/Models/Role'; -import User from 'App/Models/User'; /* |-------------------------------------------------------------------------- @@ -34,22 +29,7 @@ import User from 'App/Models/User'; | NOTE: Always export the "actions" const from this file |**************************************************************** */ -export const { actions } = Bouncer.define('manageUserRole', async (user: User, role: Role) => { - await user.load('roles'); - return !( - (role.slug === RoleEnum.ADMIN || role.slug === RoleEnum.TEACHER) && - !RoleHelper.userContainRoles(user.roles, [RoleEnum.ADMIN]) - ); -}).define('viewLessonContent', async (user: User, lesson: Lesson) => { - await user.load(loader => loader.load('roles').load('courses')); - await lesson.load('course'); - - if (!RoleHelper.userContainRoles(user.roles, [RoleEnum.ADMIN, RoleEnum.TEACHER])) { - return !!user.courses.find(course => course.id === lesson.course.id); - } - - return true; -}); +export const { actions } = Bouncer; /* |-------------------------------------------------------------------------- @@ -74,4 +54,7 @@ export const { actions } = Bouncer.define('manageUserRole', async (user: User, r | NOTE: Always export the "policies" const from this file |**************************************************************** */ -export const { policies } = Bouncer.registerPolicies({}); +export const { policies } = Bouncer.registerPolicies({ + LessonPolicy: () => import('App/Policies/LessonPolicy'), + RolePolicy: () => import('App/Policies/RolePolicy'), +}); diff --git a/start/routes/apis/v1/users.ts b/start/routes/apis/v1/users.ts index a82f47d..0ea165d 100644 --- a/start/routes/apis/v1/users.ts +++ b/start/routes/apis/v1/users.ts @@ -7,7 +7,7 @@ Route.group(() => { Route.patch('/:id', 'Api/v1/UsersController.update').middleware('role:admin,teacher').as('users.update'); Route.delete('/:id', 'Api/v1/UsersController.delete').middleware('role:admin,teacher').as('users.delete'); Route.post('/:id/attach-roles', 'Api/v1/UsersController.attachRoles') - .middleware('role:admin,teacher') + .middleware('role:admin') .as('users.attach-role'); Route.delete('/:id/detach-roles', 'Api/v1/UsersController.detachRoles') .middleware('role:admin')