From 9c0f457e9f6f009289029897ed38e0f67b8056b9 Mon Sep 17 00:00:00 2001 From: masaaki Date: Wed, 12 Jul 2023 02:06:16 +0000 Subject: [PATCH] =?UTF-8?q?Merged=20PR=20220:=20[12-2]RoleGurad=E3=81=ABTi?= =?UTF-8?q?er=E3=81=AB=E5=AF=BE=E3=81=99=E3=82=8B=E3=83=81=E3=82=A7?= =?UTF-8?q?=E3=83=83=E3=82=AF=E3=82=92=E5=AE=9F=E8=A3=85=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 概要 [Task1951: [12-2]RoleGuradにTierに対するチェックを実装する](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/1951) - 階層の指定について宣言的にできるように対応しました。 - 階層の定数について配列化しました。 （分かりやすさとroleguards側の実装しやすさのため） - 使用していない宣言がいくつか見られたので、気づいた範囲で削除しました。 - 影響範囲（他の機能にも影響があるか） 　- これまで処理内で階層のチェックを行っていた箇所について、宣言的にチェックするよう修正しました。 　　修正対象の洗い出しについては「補足」参照 　- 階層のみチェックする場合を考慮し、既存のrolesに対するチェックを任意指定にしています。 　　これに伴い、rolesが指定されなかった場合を考慮して修正を行っています。 ## レビューポイント - 各コントローラを見ていただき、階層チェックのやり方について使いづらさがないか ## UIの変更 - 無し ## 動作確認状況 - ローカルで確認済 ## 補足 - 修正対象の洗い出しは以下の通り実施しています。 https://ndstokyo.sharepoint.com/:u:/r/sites/Piranha/Shared%20Documents/General/OMDS/%E3%83%A9%E3%83%95%E3%82%B9%E3%82%B1%E3%83%83%E3%83%81/PBI1189_%5B%E9%9A%8E%E5%B1%A4%E5%91%A8%E3%82%8A%E6%95%B4%E7%90%86%5D%E7%AC%AC%E4%B8%80%EF%BD%9E%E7%AC%AC%E5%9B%9B%E9%9A%8E%E5%B1%A4%E3%81%A8%E3%81%97%E3%81%A6%E3%80%81%E3%83%91%E3%83%BC%E3%83%88%E3%83%8A%E3%83%BC%E5%90%91%E3%81%91Web%E3%82%B5%E3%83%BC%E3%83%93%E3%82%B9%E3%81%AB%E3%83%AD%E3%82%B0%E3%82%A4%E3%83%B3%E3%81%97%E3%81%9F%E3%81%84.drawio?csf=1&web=1&e=h3Sbf6 --- .../src/components/header/constants.ts | 2 +- .../src/common/guards/role/roleguards.spec.ts | 17 +++++- .../src/common/guards/role/roleguards.ts | 56 +++++++++++++++++-- .../src/common/types/tier/index.ts | 6 ++ dictation_server/src/constants/index.ts | 39 +++++-------- .../features/accounts/accounts.controller.ts | 11 ++-- .../src/features/accounts/accounts.service.ts | 4 +- .../features/licenses/licenses.controller.ts | 34 +++++------ 8 files changed, 106 insertions(+), 63 deletions(-) create mode 100644 dictation_server/src/common/types/tier/index.ts diff --git a/dictation_client/src/components/header/constants.ts b/dictation_client/src/components/header/constants.ts index e0a16c1..ede1a6a 100644 --- a/dictation_client/src/components/header/constants.ts +++ b/dictation_client/src/components/header/constants.ts @@ -21,4 +21,4 @@ export const HEADER_NAME = "ODMS Cloud"; /** * adminのみに表示するヘッダータブ */ -export const ADMIN_ONLY_TABS = [HEADER_MENUS_LICENSE]; +export const ADMIN_ONLY_TABS = [HEADER_MENUS_LICENSE, HEADER_MENUS_USER]; diff --git a/dictation_server/src/common/guards/role/roleguards.spec.ts b/dictation_server/src/common/guards/role/roleguards.spec.ts index e9b34a8..2c9c460 100644 --- a/dictation_server/src/common/guards/role/roleguards.spec.ts +++ b/dictation_server/src/common/guards/role/roleguards.spec.ts @@ -1,4 +1,4 @@ -import { ADMIN_ROLES, USER_ROLES } from '../../../constants'; +import { ADMIN_ROLES, USER_ROLES, TIERS } from '../../../constants'; import { RoleGuard } from './roleguards'; describe('RoleGuard', () => { @@ -105,3 +105,18 @@ describe('isRoles', () => { expect(result).toBeFalsy(); }); }); + +describe('RoleGuard(Tier)', () => { + it('設定したTierに指定した値が含まれる場合、許可される', () => { + const guards = RoleGuard.requireds({ + tiers: [TIERS.TIER1, TIERS.TIER2, TIERS.TIER3], + }); + expect(guards.checkTier(TIERS.TIER1)).toBeTruthy(); + }); + it('設定したTierに指定した値が含まれない場合、拒否される', () => { + const guards = RoleGuard.requireds({ + tiers: [TIERS.TIER1, TIERS.TIER2, TIERS.TIER3], + }); + expect(guards.checkTier(TIERS.TIER4)).toBeFalsy(); + }); +}); diff --git a/dictation_server/src/common/guards/role/roleguards.ts b/dictation_server/src/common/guards/role/roleguards.ts index ad810f4..c3ba25d 100644 --- a/dictation_server/src/common/guards/role/roleguards.ts +++ b/dictation_server/src/common/guards/role/roleguards.ts @@ -10,9 +10,11 @@ import { Request } from 'express'; import { retrieveAuthorizationToken } from '../../../common/http/helper'; import { makeErrorResponse } from '../../../common/error/makeErrorResponse'; import { Roles } from '../../types/role'; -import { ADMIN_ROLES, USER_ROLES } from '../../../constants'; +import { Tiers } from '../../types/tier'; +import { ADMIN_ROLES, USER_ROLES, TIERS } from '../../../constants'; export interface RoleSetting { - roles: (Roles | Roles[])[]; + roles?: (Roles | Roles[])[]; + tiers?: Tiers[]; } export class RoleGuard implements CanActivate { @@ -45,15 +47,34 @@ export class RoleGuard implements CanActivate { return true; } - if (!this.isRoles(payload.role)) { + // 値チェック + // settingsで設定されていない場合チェック自体行わないので初期値はtrueとする + let isRoleValid = true; + let isTierValid = true; + if (this.settings.roles) { + isRoleValid = this.isRoles(payload.role); + } + if (this.settings.tiers) { + isTierValid = this.isTier(payload.tier); + } + if (!isRoleValid || !isTierValid) { throw new HttpException( makeErrorResponse('E000101'), HttpStatus.UNAUTHORIZED, ); } - const isValid = this.checkRole(payload.role); - if (isValid) { + // 権限チェック + // settingsで設定されていない場合チェック自体行わないので初期値はtrueとする + let hasRolePermission = true; + let hasTierPermission = true; + if (this.settings.roles) { + hasRolePermission = this.checkRole(payload.role); + } + if (this.settings.tiers) { + hasTierPermission = this.checkTier(payload.tier); + } + if (hasRolePermission && hasTierPermission) { return true; } @@ -131,4 +152,29 @@ export class RoleGuard implements CanActivate { guard.settings = settings; return guard; } + + /** + * Tierの値の妥当性チェック + * @param tier アクセストークンに含まれるtierの値 + * @returns true/false + */ + private isTier = (tier: number): boolean => { + //値のチェック + return Object.values(TIERS).includes( + tier as (typeof TIERS)[keyof typeof TIERS], + ); + }; + + /** + * ※ テストコード以外からの直接呼び出しは禁止。テスト容易性のため、publicメソッドとして切り出したもの。 + * 階層の判別を行う + * @param tier アクセストークンに含まれるtierの値 + * @returns true/false + */ + checkTier(tier: number): boolean { + const { tiers } = this.settings; + + // 宣言された階層中にパラメータの内容が含まれていればtrue + return tiers.includes(tier as (typeof TIERS)[keyof typeof TIERS]); + } } diff --git a/dictation_server/src/common/types/tier/index.ts b/dictation_server/src/common/types/tier/index.ts new file mode 100644 index 0000000..75162d4 --- /dev/null +++ b/dictation_server/src/common/types/tier/index.ts @@ -0,0 +1,6 @@ +import { TIERS } from '../../../constants'; + +/** + * Token.tierに配置されうる数値リテラル型 + */ +export type Tiers = (typeof TIERS)[keyof typeof TIERS]; diff --git a/dictation_server/src/constants/index.ts b/dictation_server/src/constants/index.ts index 3bae42a..4fbd81b 100644 --- a/dictation_server/src/constants/index.ts +++ b/dictation_server/src/constants/index.ts @@ -1,32 +1,19 @@ /** - * OMDS東京 + * 階層 * @const {number} */ -export const TIER_1 = 1; - -/** - * OMDS現地法人 - * @const {number} - */ -export const TIER_2 = 2; - -/** - * 代理店 - * @const {number} - */ -export const TIER_3 = 3; - -/** - * 販売店 - * @const {number} - */ -export const TIER_4 = 4; - -/** - * エンドユーザー - * @const {number} - */ -export const TIER_5 = 5; +export const TIERS = { + //OMDS東京 + TIER1: 1, + //OMDS現地法人 + TIER2: 2, + //代理店 + TIER3: 3, + //販売店 + TIER4: 4, + //エンドユーザー + TIER5: 5, +} as const; /** * 音声ファイルをEast USに保存する国リスト diff --git a/dictation_server/src/features/accounts/accounts.controller.ts b/dictation_server/src/features/accounts/accounts.controller.ts index 5cf5a22..b7d80c6 100644 --- a/dictation_server/src/features/accounts/accounts.controller.ts +++ b/dictation_server/src/features/accounts/accounts.controller.ts @@ -1,7 +1,6 @@ import { Body, Controller, - HttpException, HttpStatus, Post, Get, @@ -28,15 +27,11 @@ import { CreatePartnerAccountRequest, CreatePartnerAccountResponse, } from './types/types'; -import { USER_ROLES, ADMIN_ROLES } from '../../constants'; +import { USER_ROLES, ADMIN_ROLES, TIERS } from '../../constants'; import { AuthGuard } from '../../common/guards/auth/authguards'; import { RoleGuard } from '../../common/guards/role/roleguards'; -//import { CryptoService } from '../../gateways/crypto/crypto.service'; import { retrieveAuthorizationToken } from '../../common/http/helper'; -import { makeErrorResponse } from '../../common/error/makeErrorResponse'; -import { isVerifyError, verify } from '../../common/jwt'; import { AccessToken } from '../../common/token'; -//import { confirmPermission } from '../../common/auth/auth'; import jwt from 'jsonwebtoken'; @ApiTags('accounts') @@ -112,7 +107,9 @@ export class AccountsController { }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + ) @Post('licenses/summary') async getLicenseSummary( @Req() req: Request, diff --git a/dictation_server/src/features/accounts/accounts.service.ts b/dictation_server/src/features/accounts/accounts.service.ts index 2a1ffa1..bbf92cb 100644 --- a/dictation_server/src/features/accounts/accounts.service.ts +++ b/dictation_server/src/features/accounts/accounts.service.ts @@ -10,7 +10,7 @@ import { } from '../../gateways/adb2c/adb2c.service'; import { Account } from '../../repositories/accounts/entity/account.entity'; import { User } from '../../repositories/users/entity/user.entity'; -import { LICENSE_EXPIRATION_THRESHOLD_DAYS, TIER_5 } from '../../constants'; +import { LICENSE_EXPIRATION_THRESHOLD_DAYS, TIERS } from '../../constants'; import { makeErrorResponse } from '../../common/error/makeErrorResponse'; import { TypistGroup } from './types/types'; import { GetLicenseSummaryResponse, Typist } from './types/types'; @@ -147,7 +147,7 @@ export class AccountsService { companyName, country, dealerAccountId, - TIER_5, + TIERS.TIER5, externalUser.sub, role, acceptedTermsVersion, diff --git a/dictation_server/src/features/licenses/licenses.controller.ts b/dictation_server/src/features/licenses/licenses.controller.ts index 8925c49..9ce59f0 100644 --- a/dictation_server/src/features/licenses/licenses.controller.ts +++ b/dictation_server/src/features/licenses/licenses.controller.ts @@ -5,7 +5,6 @@ import { Post, Req, UseGuards, - HttpException, } from '@nestjs/common'; import { ApiResponse, @@ -28,9 +27,8 @@ import { retrieveAuthorizationToken } from '../../common/http/helper'; import { AccessToken } from '../../common/token'; import { AuthGuard } from '../../common/guards/auth/authguards'; import { RoleGuard } from '../../common/guards/role/roleguards'; -import { ADMIN_ROLES, TIER_1, TIER_5 } from '../../constants'; +import { ADMIN_ROLES, TIERS } from '../../constants'; import jwt from 'jsonwebtoken'; -import { makeErrorResponse } from '../../common/error/makeErrorResponse'; @ApiTags('licenses') @Controller('licenses') @@ -59,7 +57,12 @@ export class LicensesController { @ApiOperation({ operationId: 'createOrders' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ + roles: [ADMIN_ROLES.ADMIN], + tiers: [TIERS.TIER2, TIERS.TIER3, TIERS.TIER4, TIERS.TIER5], + }), + ) @Post('/orders') async createOrders( @Req() req: Request, @@ -99,7 +102,9 @@ export class LicensesController { @ApiOperation({ operationId: 'issueCardLicenses' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER1] }), + ) @Post('/cards') async issueCardLicenses( @Req() req: Request, @@ -111,13 +116,6 @@ export class LicensesController { const accessToken = retrieveAuthorizationToken(req); const payload = jwt.decode(accessToken, { json: true }) as AccessToken; - // 第一階層以外は401を返す（後々UseGuardsで弾く） - if (payload.tier != TIER_1) { - throw new HttpException( - makeErrorResponse('E000108'), - HttpStatus.UNAUTHORIZED, - ); - } const cardLicenseKeys = await this.licensesService.issueCardLicenseKeys( payload.userId, body.createCount, @@ -150,7 +148,9 @@ export class LicensesController { @ApiOperation({ operationId: 'activateCardLicenses' }) @ApiBearerAuth() @UseGuards(AuthGuard) - @UseGuards(RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN] })) + @UseGuards( + RoleGuard.requireds({ roles: [ADMIN_ROLES.ADMIN], tiers: [TIERS.TIER5] }), + ) @Post('/cards/activate') async activateCardLicenses( @Req() req: Request, @@ -162,14 +162,6 @@ export class LicensesController { const accessToken = retrieveAuthorizationToken(req); const payload = jwt.decode(accessToken, { json: true }) as AccessToken; - // TODO 第五階層以外は401を返す（後々UseGuardsで弾く） - if (payload.tier != TIER_5) { - throw new HttpException( - makeErrorResponse('E000108'), - HttpStatus.UNAUTHORIZED, - ); - } - await this.licensesService.activateCardLicenseKey( payload.userId, body.cardLicenseKey,