From 8a2ca2b786fb13c337afb89d35b34d06a15d3023 Mon Sep 17 00:00:00 2001 From: "makabe.t" Date: Tue, 7 Nov 2023 08:41:47 +0000 Subject: [PATCH] =?UTF-8?q?Merged=20PR=20519:=20=E4=BB=A3=E8=A1=8C?= =?UTF-8?q?=E6=93=8D=E4=BD=9C=E3=81=A7=E4=BD=BF=E7=94=A8=E3=81=99=E3=82=8B?= =?UTF-8?q?API=E3=81=AE=E3=82=AC=E3=83=BC=E3=83=89=E4=BF=AE=E6=AD=A3?= =?UTF-8?q?=EF=BC=8B=E3=83=AD=E3=82=B0=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 概要 [Task2907: 代行操作で使用するAPIのガード修正+ログ修正](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/2907) - 代行操作するAPIについてガードに代行操作許可の設定を追加しました。 - ガードの引数で設定したAPIだけが代行操作許可となるように設定しています。 - APIのバリデーションで意図せぬエラーが発生する問題に対処しました。 ※ログは未適用なのでガードの設定だけ確認をお願いします。 ## レビューポイント - ガードを設定する対象は適切か - ガード内での代行操作許可のロジックはホワイトリスト形式の実装として適切か - アクセストークンが代行操作用の(代行操作ユーザーIDがある)場合には、ガードで代行操作が許可されているかをチェックするようにしています。 ※ログの代行操作への適用は未実装です。 ## UIの変更 - なし ## 動作確認状況 - ローカルで確認 --- .../src/common/guards/role/roleguards.spec.ts | 23 ++++++++ .../src/common/guards/role/roleguards.ts | 36 ++++++++++++- .../common/validators/assignees.validator.ts | 4 ++ .../features/accounts/accounts.controller.ts | 53 ++++++++++++++----- .../src/features/files/files.controller.ts | 8 ++- .../features/licenses/licenses.controller.ts | 14 ++++- .../templates/templates.controller.ts | 4 +- .../src/features/users/users.controller.ts | 25 ++++++--- .../workflows/workflows.controller.ts | 16 ++++-- 9 files changed, 155 insertions(+), 28 deletions(-) diff --git a/dictation_server/src/common/guards/role/roleguards.spec.ts b/dictation_server/src/common/guards/role/roleguards.spec.ts index 2c9c460..e2a8303 100644 --- a/dictation_server/src/common/guards/role/roleguards.spec.ts +++ b/dictation_server/src/common/guards/role/roleguards.spec.ts @@ -120,3 +120,26 @@ describe('RoleGuard(Tier)', () => { expect(guards.checkTier(TIERS.TIER4)).toBeFalsy(); }); }); + +describe('RoleGuard(代行操作)', () => { + it('代行操作許可設定時、delegateUserIdに値が含まれる場合、許可される', () => { + const guards = RoleGuard.requireds({ + delegation: true, + }); + expect(guards.checkDelegate('delegateUser')).toBeTruthy(); + }); + it('代行操作許可設定時、delegateUserIdに値が含まれない場合、許可される', () => { + const guards = RoleGuard.requireds({ + delegation: true, + }); + expect(guards.checkDelegate(undefined)).toBeTruthy(); + }); + it('代行操作許可未設定時、delegateUserIdに値が含まれる場合、拒否される', () => { + const guards = RoleGuard.requireds({}); + expect(guards.checkDelegate('delegateUser')).toBeFalsy(); + }); + it('代行操作許可未設定時、delegateUserIdに値が含まれない場合、許可される', () => { + const guards = RoleGuard.requireds({}); + expect(guards.checkDelegate(undefined)).toBeTruthy(); + }); +}); diff --git a/dictation_server/src/common/guards/role/roleguards.ts b/dictation_server/src/common/guards/role/roleguards.ts index 6670bed..18890e0 100644 --- a/dictation_server/src/common/guards/role/roleguards.ts +++ b/dictation_server/src/common/guards/role/roleguards.ts @@ -15,6 +15,7 @@ import { ADMIN_ROLES, USER_ROLES, TIERS } from '../../../constants'; export interface RoleSetting { roles?: (Roles | Roles[])[]; tiers?: Tiers[]; + delegation?: boolean; } export class RoleGuard implements CanActivate { @@ -64,6 +65,17 @@ export class RoleGuard implements CanActivate { ); } + // アクセストークンに代行操作ユーザーの情報が含まれているか(含まれている場合は代行操作) + const isDelegationUser = payload.delegateUserId !== undefined; + + // 代行操作ユーザーの場合、代行操作許可設定がない場合は例外を送出 + if (isDelegationUser && !this.settings.delegation) { + throw new HttpException( + makeErrorResponse('E000108'), + HttpStatus.UNAUTHORIZED, + ); + } + // 権限チェック // settingsで設定されていない場合チェック自体行わないので初期値はtrueとする let hasRolePermission = true; @@ -74,7 +86,9 @@ export class RoleGuard implements CanActivate { if (this.settings.tiers) { hasTierPermission = this.checkTier(payload.tier); } - if (hasRolePermission && hasTierPermission) { + const hasDelegatePermission = this.checkDelegate(payload.delegateUserId); + + if (hasRolePermission && hasTierPermission && hasDelegatePermission) { return true; } @@ -182,4 +196,24 @@ export class RoleGuard implements CanActivate { // 宣言された階層中にパラメータの内容が含まれていればtrue return settings.tiers.includes(tier as (typeof TIERS)[keyof typeof TIERS]); } + + /** + * ※ テストコード以外からの直接呼び出しは禁止。テスト容易性のため、publicメソッドとして切り出したもの。 + * 階層の判別を行う + * @param delegateUserId アクセストークンに含まれるdelegateUserIdの値 + * @returns true/false + */ + checkDelegate(delegateUserId?: string | undefined): boolean { + const settings = this.settings; + + // 通常ユーザの場合は許可 + if (delegateUserId === undefined) { + return true; + } else if (settings?.delegation ?? false) { + // 代行操作ユーザーの場合は許可設定がある場合のみ許可; + return true; + } + + return false; + } } diff --git a/dictation_server/src/common/validators/assignees.validator.ts b/dictation_server/src/common/validators/assignees.validator.ts index 45550d0..2c0dc58 100644 --- a/dictation_server/src/common/validators/assignees.validator.ts +++ b/dictation_server/src/common/validators/assignees.validator.ts @@ -9,6 +9,10 @@ import { Assignee } from '../../features/tasks/types/types'; @ValidatorConstraint() export class IsTypist implements ValidatorConstraintInterface { validate(values: Assignee[]): boolean { + if (!values) { + return false; + } + return values.every((value) => { const { typistUserId, typistGroupId, typistName } = value; if (typistUserId === undefined && typistGroupId === undefined) { diff --git a/dictation_server/src/features/accounts/accounts.controller.ts b/dictation_server/src/features/accounts/accounts.controller.ts index 241a72a..ac29f0c 100644 --- a/dictation_server/src/features/accounts/accounts.controller.ts +++ b/dictation_server/src/features/accounts/accounts.controller.ts @@ -159,7 +159,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('licenses/summary') async getLicenseSummary( @Req() req: Request, @@ -246,7 +248,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get('authors') async getAuthors(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); @@ -293,6 +297,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get('typists') async getTypists(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); @@ -338,6 +345,7 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) + @UseGuards(RoleGuard.requireds({ delegation: true })) @Get('typist-groups') async getTypistGroups(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); @@ -388,7 +396,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get('typist-groups/:typistGroupId') async getTypistGroup( @Req() req: Request, @@ -452,7 +462,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('typist-groups') async createTypistGroup( @Req() req: Request, @@ -513,7 +525,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('typist-groups/:typistGroupId') async updateTypistGroup( @Req() req: Request, @@ -680,6 +694,7 @@ export class AccountsController { @UseGuards( RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], + delegation: true, }), ) async getOrderHistories( @@ -858,7 +873,9 @@ export class AccountsController { @ApiOperation({ operationId: 'getWorktypes' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async getWorktypes(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); if (!accessToken) { @@ -906,7 +923,9 @@ export class AccountsController { @ApiOperation({ operationId: 'createWorktype' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async createWorktype( @Req() req: Request, @Body() body: CreateWorktypesRequest, @@ -964,7 +983,9 @@ export class AccountsController { @ApiOperation({ operationId: 'updateWorktype' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async updateWorktype( @Req() req: Request, @Param() param: UpdateWorktypeRequestParam, @@ -1026,7 +1047,9 @@ export class AccountsController { @ApiOperation({ operationId: 'deleteWorktype' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async deleteWorktype( @Req() req: Request, @Param() param: DeleteWorktypeRequestParam, @@ -1079,7 +1102,9 @@ export class AccountsController { @ApiOperation({ operationId: 'getOptionItems' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async getOptionItems( @Req() req: Request, @Param() param: GetOptionItemsRequestParam, @@ -1137,7 +1162,9 @@ export class AccountsController { @ApiOperation({ operationId: 'updateOptionItems' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async updateOptionItems( @Req() req: Request, @Param() param: UpdateOptionItemsRequestParam, @@ -1198,7 +1225,9 @@ export class AccountsController { @ApiOperation({ operationId: 'activeWorktype' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async activeWorktype( @Req() req: Request, @Body() body: PostActiveWorktypeRequest, diff --git a/dictation_server/src/features/files/files.controller.ts b/dictation_server/src/features/files/files.controller.ts index 8578e52..e3a772e 100644 --- a/dictation_server/src/features/files/files.controller.ts +++ b/dictation_server/src/features/files/files.controller.ts @@ -337,7 +337,9 @@ export class FilesController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async uploadTemplateLocation( @Req() req: Request, ): Promise { @@ -393,7 +395,9 @@ export class FilesController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('template/upload-finished') async templateUploadFinished( @Req() req: Request, diff --git a/dictation_server/src/features/licenses/licenses.controller.ts b/dictation_server/src/features/licenses/licenses.controller.ts index ae47eb1..eb702b9 100644 --- a/dictation_server/src/features/licenses/licenses.controller.ts +++ b/dictation_server/src/features/licenses/licenses.controller.ts @@ -68,6 +68,7 @@ export class LicensesController { RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER2, TIERS.TIER3, TIERS.TIER4, TIERS.TIER5], + delegation: true, }), ) @Post('/orders') @@ -175,7 +176,11 @@ export class LicensesController { @ApiBearerAuth() @UseGuards(AuthGuard) @UseGuards( - RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + RoleGuard.requireds({ + roles: [ADMIN_ROLES.ADMIN], + tiers: [TIERS.TIER5], + delegation: true, + }), ) @Post('/cards/activate') async activateCardLicenses( @@ -228,7 +233,11 @@ export class LicensesController { @ApiBearerAuth() @UseGuards(AuthGuard) @UseGuards( - RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + RoleGuard.requireds({ + roles: [ADMIN_ROLES.ADMIN], + tiers: [TIERS.TIER5], + delegation: true, + }), ) @Get('/allocatable') async getAllocatableLicenses( @@ -289,6 +298,7 @@ export class LicensesController { RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER2, TIERS.TIER3, TIERS.TIER4, TIERS.TIER5], + delegation: true, }), ) @Post('/orders/cancel') diff --git a/dictation_server/src/features/templates/templates.controller.ts b/dictation_server/src/features/templates/templates.controller.ts index ef972cf..08a77d4 100644 --- a/dictation_server/src/features/templates/templates.controller.ts +++ b/dictation_server/src/features/templates/templates.controller.ts @@ -51,7 +51,9 @@ export class TemplatesController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get() async getTemplates(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); diff --git a/dictation_server/src/features/users/users.controller.ts b/dictation_server/src/features/users/users.controller.ts index c0e4330..b1fe7c9 100644 --- a/dictation_server/src/features/users/users.controller.ts +++ b/dictation_server/src/features/users/users.controller.ts @@ -54,7 +54,6 @@ import { RoleGuard } from '../../common/guards/role/roleguards'; import { makeContext } from '../../common/log'; import { UserRoles } from '../../common/types/role'; import { v4 as uuidv4 } from 'uuid'; -import { userInfo } from 'os'; @ApiTags('users') @Controller('users') @@ -129,7 +128,9 @@ export class UsersController { @ApiOperation({ operationId: 'getUsers' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get() async getUsers(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); @@ -176,7 +177,9 @@ export class UsersController { @ApiBearerAuth() @Post('/signup') @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) async signup( @Req() req: Request, @Body() body: SignupRequest, @@ -413,7 +416,9 @@ export class UsersController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('update') async updateUser( @Body() body: PostUpdateUserRequest, @@ -492,7 +497,11 @@ export class UsersController { @ApiBearerAuth() @UseGuards(AuthGuard) @UseGuards( - RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + RoleGuard.requireds({ + roles: [ADMIN_ROLES.ADMIN], + tiers: [TIERS.TIER5], + delegation: true, + }), ) @Post('/license/allocate') async allocateLicense( @@ -551,7 +560,11 @@ export class UsersController { @ApiBearerAuth() @UseGuards(AuthGuard) @UseGuards( - RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + RoleGuard.requireds({ + roles: [ADMIN_ROLES.ADMIN], + tiers: [TIERS.TIER5], + delegation: true, + }), ) @Post('/license/deallocate') async deallocateLicense( diff --git a/dictation_server/src/features/workflows/workflows.controller.ts b/dictation_server/src/features/workflows/workflows.controller.ts index 4c6ac32..ff312e1 100644 --- a/dictation_server/src/features/workflows/workflows.controller.ts +++ b/dictation_server/src/features/workflows/workflows.controller.ts @@ -63,7 +63,9 @@ export class WorkflowsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Get() async getWorkflows(@Req() req: Request): Promise { const accessToken = retrieveAuthorizationToken(req); @@ -115,7 +117,9 @@ export class WorkflowsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post() async createWorkflows( @Req() req: Request, @@ -178,7 +182,9 @@ export class WorkflowsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('/:workflowId') async updateWorkflow( @Req() req: Request, @@ -244,7 +250,9 @@ export class WorkflowsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], delegation: true }), + ) @Post('/:workflowId/delete') async deleteWorkflow( @Req() req: Request,