From 8c3e1db63b16d153de9d4835b116c59303ebbe9f Mon Sep 17 00:00:00 2001 From: "saito.k" Date: Tue, 27 Jun 2023 07:18:19 +0000 Subject: [PATCH] =?UTF-8?q?Merged=20PR=20185:=20API=E4=BF=AE=E6=AD=A3?= =?UTF-8?q?=EF=BC=88=E3=83=81=E3=82=A7=E3=83=83=E3=82=AF=E3=82=A2=E3=82=A6?= =?UTF-8?q?=E3=83=88=E5=80=99=E8=A3=9C=E5=A4=89=E6=9B=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 概要 [Task2070: API修正](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/2070) - DBの検索条件を変更 - ユーザーグループ検索 - 同一アカウント内という条件を追加 - ユーザーグループIDは画面側から渡されるが、そのＩＤが同じアカウント内のＩＤかわからないため - タイピストユーザー検索 - 同一アカウント内でロールが`typist`という条件を追加 - 画面側から渡されるユーザーＩＤが、同じアカウント内のＩＤかわからないため - 画面側から渡されるユーザーＩＤが、Typistであるかわからないため - タスク検索 - 同一アカウント内という条件を追加 - 渡される音声ファイルIDが同じアカウント内であるかわからないため - 自身のロールが`author`だった場合、AuthorIDが一致するタスクという条件を追加（管理者ロールがある場合は条件無し） ## レビューポイント - 不要な条件追加ではないか - ロールの型変換処理に問題はないか ## UIの変更 - Before/Afterのスクショなど - スクショ置き場 ## 動作確認状況 - ローカルで確認 ## 補足 - 相談、参考資料などがあれば --- .../src/common/guards/role/roleguards.spec.ts | 33 ++++++++++++++++ .../src/common/guards/role/roleguards.ts | 39 +++++++++++++++++-- .../src/common/types/role/index.ts | 8 ++++ dictation_server/src/constants/index.ts | 7 ---- .../src/features/tasks/tasks.controller.ts | 16 +++++++- .../src/features/tasks/tasks.service.spec.ts | 17 +++++--- .../src/features/tasks/tasks.service.ts | 13 ++++++- .../tasks/tasks.repository.service.ts | 26 ++++++++++--- 8 files changed, 135 insertions(+), 24 deletions(-) create mode 100644 dictation_server/src/common/types/role/index.ts diff --git a/dictation_server/src/common/guards/role/roleguards.spec.ts b/dictation_server/src/common/guards/role/roleguards.spec.ts index 690b40b..e9b34a8 100644 --- a/dictation_server/src/common/guards/role/roleguards.spec.ts +++ b/dictation_server/src/common/guards/role/roleguards.spec.ts @@ -72,3 +72,36 @@ describe('RoleGuard', () => { expect(guards.checkRole('')).toBeFalsy(); }); }); + +describe('isRoles', () => { + it('roleの各要素が正しい値であったとき、許可される', () => { + const guards = RoleGuard.requireds({ + roles: [[USER_ROLES.AUTHOR, ADMIN_ROLES.ADMIN], USER_ROLES.TYPIST], + }); + const result = guards.isRoles('admin author'); + expect(result).toBeTruthy(); + }); + it('roleの各要素に不正な値があったとき、拒否される', () => { + const guards = RoleGuard.requireds({ + roles: [[USER_ROLES.AUTHOR, ADMIN_ROLES.ADMIN], USER_ROLES.TYPIST], + }); + const result = guards.isRoles('admin xxx'); + expect(result).toBeFalsy(); + }); + + it('roleの各要素に重複があったとき、拒否される', () => { + const guards = RoleGuard.requireds({ + roles: [[USER_ROLES.AUTHOR, ADMIN_ROLES.ADMIN], USER_ROLES.TYPIST], + }); + const result = guards.isRoles('admin admin'); + expect(result).toBeFalsy(); + }); + + it('roleの値がないとき、拒否される', () => { + const guards = RoleGuard.requireds({ + roles: [[USER_ROLES.AUTHOR, ADMIN_ROLES.ADMIN], USER_ROLES.TYPIST], + }); + const result = guards.isRoles(''); + expect(result).toBeFalsy(); + }); +}); diff --git a/dictation_server/src/common/guards/role/roleguards.ts b/dictation_server/src/common/guards/role/roleguards.ts index 13472c0..ad810f4 100644 --- a/dictation_server/src/common/guards/role/roleguards.ts +++ b/dictation_server/src/common/guards/role/roleguards.ts @@ -9,11 +9,10 @@ import { AccessToken } from '../../token'; import { Request } from 'express'; import { retrieveAuthorizationToken } from '../../../common/http/helper'; import { makeErrorResponse } from '../../../common/error/makeErrorResponse'; - -export type RoleType = 'typist' | 'author' | 'none' | 'admin'; - +import { Roles } from '../../types/role'; +import { ADMIN_ROLES, USER_ROLES } from '../../../constants'; export interface RoleSetting { - roles: (RoleType | RoleType[])[]; + roles: (Roles | Roles[])[]; } export class RoleGuard implements CanActivate { @@ -46,6 +45,13 @@ export class RoleGuard implements CanActivate { return true; } + if (!this.isRoles(payload.role)) { + throw new HttpException( + makeErrorResponse('E000101'), + HttpStatus.UNAUTHORIZED, + ); + } + const isValid = this.checkRole(payload.role); if (isValid) { return true; @@ -87,6 +93,31 @@ export class RoleGuard implements CanActivate { return false; } + /** + * ※ テストコード以外からの直接呼び出しは禁止。テスト容易性のため、publicメソッドとして切り出したもの。 + * Roleの値の妥当性チェック + * @param role アクセストークンに含まれるroleの値 + * @returns true/false + */ + isRoles = (role: string): boolean => { + const userRoles = role.split(' ') as Roles[]; + const rolesValues = [ + ...Object.values(ADMIN_ROLES), + ...Object.values(USER_ROLES), + ]; + //重複チェック + if (userRoles.length !== new Set(userRoles).size) { + return false; + } + //各要素が正しい値かチェック + return userRoles.every((role) => { + if (!rolesValues.includes(role)) { + return false; + } + return true; + }); + }; + /** * 権限の許可設定を指定したGuardを作成する * { roles: ['admin', 'author'] } "admin"または"author"なら許可 diff --git a/dictation_server/src/common/types/role/index.ts b/dictation_server/src/common/types/role/index.ts new file mode 100644 index 0000000..287488d --- /dev/null +++ b/dictation_server/src/common/types/role/index.ts @@ -0,0 +1,8 @@ +import { ADMIN_ROLES, USER_ROLES } from '../../../constants'; + +/** + * Token.roleに配置されうる文字列リテラル型 + */ +export type Roles = + | (typeof ADMIN_ROLES)[keyof typeof ADMIN_ROLES] + | (typeof USER_ROLES)[keyof typeof USER_ROLES]; diff --git a/dictation_server/src/constants/index.ts b/dictation_server/src/constants/index.ts index ae4070d..d209424 100644 --- a/dictation_server/src/constants/index.ts +++ b/dictation_server/src/constants/index.ts @@ -101,13 +101,6 @@ export const USER_ROLES = { TYPIST: 'typist', } as const; -/** - * Token.roleに配置されうる文字列リテラル型 - */ -export type Roles = - | (typeof ADMIN_ROLES)[keyof typeof ADMIN_ROLES] - | (typeof USER_ROLES)[keyof typeof USER_ROLES]; - /** * ライセンス注文ステータス（発行待ち） * @const {string} diff --git a/dictation_server/src/features/tasks/tasks.controller.ts b/dictation_server/src/features/tasks/tasks.controller.ts index 858de11..6920119 100644 --- a/dictation_server/src/features/tasks/tasks.controller.ts +++ b/dictation_server/src/features/tasks/tasks.controller.ts @@ -40,6 +40,7 @@ import { AccessToken } from '../../common/token'; import { AuthGuard } from '../../common/guards/auth/authguards'; import { RoleGuard } from '../../common/guards/role/roleguards'; import { ADMIN_ROLES, USER_ROLES } from '../../constants'; +import { Roles } from '../../common/types/role'; @ApiTags('tasks') @Controller('tasks') @@ -430,7 +431,20 @@ export class TasksController { @Body() body: PostCheckoutPermissionRequest, ): Promise { const { assignees } = body; - await this.taskService.changeCheckoutPermission(audioFileId, assignees); + const accessToken = retrieveAuthorizationToken(req); + + const { role, userId } = jwt.decode(accessToken, { + json: true, + }) as AccessToken; + // RoleGuardでroleの要素が正しい値であることは担保されているためここでは型変換のみ行う + const roles = role.split(' ') as Roles[]; + + await this.taskService.changeCheckoutPermission( + audioFileId, + assignees, + userId, + roles, + ); return {}; } diff --git a/dictation_server/src/features/tasks/tasks.service.spec.ts b/dictation_server/src/features/tasks/tasks.service.spec.ts index 4d4d42e..59890f9 100644 --- a/dictation_server/src/features/tasks/tasks.service.spec.ts +++ b/dictation_server/src/features/tasks/tasks.service.spec.ts @@ -674,7 +674,7 @@ describe('TasksService', () => { }); }); -describe('TasksService', () => { +describe('changeCheckoutPermission', () => { // TODO sqliteを用いたテストを別途実装予定 /* 指定したユーザーグループがない場合 @@ -692,9 +692,14 @@ describe('TasksService', () => { adb2cServiceMockValue, ); - expect(await service.tasksService.changeCheckoutPermission(1, [])).toEqual( - undefined, - ); + expect( + await service.tasksService.changeCheckoutPermission( + 1, + [], + 'xxx-xxx-xxxx', + ['admin'], + ), + ).toEqual(undefined); }); it('ユーザーが存在しない場合、タスクのチェックアウト権限を変更できない', async () => { @@ -710,7 +715,9 @@ describe('TasksService', () => { ); await expect( - service.tasksService.changeCheckoutPermission(1, []), + service.tasksService.changeCheckoutPermission(1, [], 'xxx-xxxx-xxxx', [ + 'admin', + ]), ).rejects.toEqual( new HttpException(makeErrorResponse('E010601'), HttpStatus.BAD_REQUEST), ); diff --git a/dictation_server/src/features/tasks/tasks.service.ts b/dictation_server/src/features/tasks/tasks.service.ts index a4765ba..f46be41 100644 --- a/dictation_server/src/features/tasks/tasks.service.ts +++ b/dictation_server/src/features/tasks/tasks.service.ts @@ -17,11 +17,12 @@ import { } from '../../gateways/adb2c/adb2c.service'; import { AdB2cUser } from '../../gateways/adb2c/types/types'; import { CheckoutPermission } from '../../repositories/checkout_permissions/entity/checkout_permission.entity'; -import { UserNotFoundError } from '../../repositories/users/errors/types'; import { TasksNotFoundError, TypistUserGroupNotFoundError, + TypistUserNotFoundError, } from '../../repositories/tasks/errors/types'; +import { Roles } from '../../common/types/role'; @Injectable() export class TasksService { @@ -170,17 +171,25 @@ export class TasksService { async changeCheckoutPermission( audioFileId: number, assignees: Assignee[], + externalId: string, + role: Roles[], ): Promise { try { + const { author_id, account_id } = + await this.usersRepository.findUserByExternalId(externalId); + await this.taskRepository.changeCheckoutPermission( audioFileId, + author_id, + account_id, + role, assignees, ); } catch (e) { this.logger.error(`error=${e}`); if (e instanceof Error) { switch (e.constructor) { - case UserNotFoundError: + case TypistUserNotFoundError: case TypistUserGroupNotFoundError: throw new HttpException( makeErrorResponse('E010204'), diff --git a/dictation_server/src/repositories/tasks/tasks.repository.service.ts b/dictation_server/src/repositories/tasks/tasks.repository.service.ts index d89e523..e338883 100644 --- a/dictation_server/src/repositories/tasks/tasks.repository.service.ts +++ b/dictation_server/src/repositories/tasks/tasks.repository.service.ts @@ -7,7 +7,7 @@ import { IsNull, } from 'typeorm'; import { Task } from './entity/task.entity'; -import { TASK_STATUS } from '../../constants'; +import { ADMIN_ROLES, TASK_STATUS, USER_ROLES } from '../../constants'; import { AudioOptionItem as ParamOptionItem } from '../../features/files/types/types'; import { AudioFile } from '../audio_files/entity/audio_file.entity'; import { AudioOptionItem } from '../audio_option_items/entity/audio_option_item.entity'; @@ -25,6 +25,7 @@ import { TypistUserGroupNotFoundError, TypistUserNotFoundError, } from './errors/types'; +import { Roles } from '../../common/types/role'; @Injectable() export class TasksRepositoryService { @@ -381,13 +382,16 @@ export class TasksRepositoryService { /** * Changes checkout permission - * @param audioFileId + * @param audio_file_id * @param assignees * @param accountId * @returns checkout permission */ async changeCheckoutPermission( - audioFileId: number, + audio_file_id: number, + author_id: string, + account_id: number, + roles: Roles[], assignees: Assignee[], ): Promise { await this.dataSource.transaction(async (entityManager) => { @@ -401,6 +405,7 @@ export class TasksRepositoryService { const groupRecords = await groupRepo.find({ where: { id: In(userGroupIds), + account_id: account_id, deleted_at: IsNull(), }, }); @@ -423,6 +428,8 @@ export class TasksRepositoryService { const userRecords = await userRepo.find({ where: { id: In(typistUserIds), + account_id: account_id, + role: USER_ROLES.TYPIST, deleted_at: IsNull(), }, }); @@ -439,12 +446,21 @@ export class TasksRepositoryService { const taskRepo = entityManager.getRepository(Task); const taskRecord = await taskRepo.findOne({ - where: { audio_file_id: audioFileId, status: TASK_STATUS.UPLOADED }, + where: { + audio_file_id: audio_file_id, + status: TASK_STATUS.UPLOADED, + account_id: account_id, + file: { + author_id: roles.includes(ADMIN_ROLES.ADMIN) + ? undefined + : author_id, + }, + }, }); //タスクが存在しない or ステータスがUploadedでなければエラー if (!taskRecord) { throw new TasksNotFoundError( - `Task not found Error. audio_file_id:${audioFileId}`, + `Task not found Error. audio_file_id:${audio_file_id}`, ); }