From e4f84f78ba8c97db1537fcd5c8e086ddfac6a4fe Mon Sep 17 00:00:00 2001
From: "makabe.t" <makabe.t@nds-tyo.co.jp>
Date: Fri, 2 Jun 2023 04:55:28 +0000
Subject: [PATCH] =?UTF-8?q?Merged=20PR=20121:=20=E3=83=AA=E3=83=95?=
 =?UTF-8?q?=E3=83=AC=E3=83=83=E3=82=B7=E3=83=A5=E3=83=88=E3=83=BC=E3=82=AF?=
 =?UTF-8?q?=E3=83=B3=E3=81=AE=E7=99=BA=E8=A1=8C=E6=96=B9=E6=B3=95=E3=83=BB?=
 =?UTF-8?q?=E7=99=BA=E8=A1=8C=E5=86=85=E5=AE=B9=E3=81=8C=E4=B8=8D=E9=81=A9?=
 =?UTF-8?q?=E5=88=87=E3=81=AA=E5=95=8F=E9=A1=8C=E3=81=AE=E4=BF=AE=E6=AD=A3?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 概要
[Task1904: リフレッシュトークンの発行方法・発行内容が不適切な問題の修正](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/1904)

セキュリティ・権限管理の要であるリフレッシュトークンのロール設定に関する修正ですので、開発メンバー全員にご確認いただき、変更内容について認識を合わせていただきたいです。
- リフレッシュトークンの発行で使用するロールがクライアントから送られた値をそのまま利用する実装になっていましたので、APIのロールの取り扱いを修正しました
  - ユーザー追加APIで想定している（author/typist/none）のみを受け付けるようにする
    - 想定外のロールの場合は400エラー
    - クライアントからも想定通りのロールを送信するようロールの定数値を修正
  - リフレッシュトークンを発行する際にDBの値をそのまま使わず、想定値のどれかを判断して定数を設定する
    - DBに登録されているロールが想定外の文字列の場合は500エラーとなる

## レビューポイント
- トークンのロール設定が安全にできるようになっていることをメンバー全員で合意
- APIのパラメータを受け付ける際にIsInで想定ロール文字列のみを受け付けるようにしているが問題はないか
- リフレッシュトークンの生成時に直接DBのロールを設定しないようにしたが、トークンのロール設定として問題はないか
  - 意図しないトークンへの権限の付与などは起こりえないか。
    - クライアントからadminなどのユーザー追加時に想定していない権限を付与しようとしてもAPIではじかれるか

## UIの変更
- なし

## 動作確認状況
- ローカルで確認

## 補足
- ユーザー追加時に登録されるロールが変更となったのでマージ後に利用する場合は再度の登録をお願いします。
---
 .../src/features/user/constants.ts            |  6 ++---
 .../src/pages/UserListPage/popup.tsx          |  6 ++---
 dictation_server/src/common/error/code.ts     |  1 +
 dictation_server/src/common/error/message.ts  |  1 +
 dictation_server/src/constants/index.ts       | 19 +++++++++++---
 .../features/accounts/accounts.controller.ts  |  4 +--
 .../src/features/auth/auth.service.ts         | 26 ++++++++++++++++---
 .../src/features/users/types/types.ts         |  3 +++
 8 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/dictation_client/src/features/user/constants.ts b/dictation_client/src/features/user/constants.ts
index 320e99e..bc46f87 100644
--- a/dictation_client/src/features/user/constants.ts
+++ b/dictation_client/src/features/user/constants.ts
@@ -1,7 +1,7 @@
 export const ROLE = {
-  AUTHOR: "Author",
-  TYPIST: "Transcriptionist",
-  NONE: "None",
+  AUTHOR: "author",
+  TYPIST: "typist",
+  NONE: "none",
 } as const;
 
 export type RoleType = typeof ROLE[keyof typeof ROLE];
diff --git a/dictation_client/src/pages/UserListPage/popup.tsx b/dictation_client/src/pages/UserListPage/popup.tsx
index 45fd300..557ba94 100644
--- a/dictation_client/src/pages/UserListPage/popup.tsx
+++ b/dictation_client/src/pages/UserListPage/popup.tsx
@@ -199,14 +199,14 @@ export const UserAddPopup: React.FC<UserAddPopupProps> = (props) => {
                 />
                 {t(getTranslationID("userListPage.label.transcriptionist"))}
               </label>
-              <label htmlFor="None">
+              <label htmlFor={ROLE.NONE}>
                 <input
                   type="radio"
                   name="role"
                   className={styles.formRadio}
-                  checked={role === "None"}
+                  checked={role === ROLE.NONE}
                   onChange={() => {
-                    dispatch(changeRole({ role: "None" }));
+                    dispatch(changeRole({ role: ROLE.NONE }));
                   }}
                 />
                 {t(getTranslationID("userListPage.label.none"))}
diff --git a/dictation_server/src/common/error/code.ts b/dictation_server/src/common/error/code.ts
index 8d68836..c430e3f 100644
--- a/dictation_server/src/common/error/code.ts
+++ b/dictation_server/src/common/error/code.ts
@@ -27,6 +27,7 @@ export const ErrorCodes = [
   'E010202', // 認証済ユーザエラー
   'E010203', // 管理ユーザ権限エラー
   'E010204', // ユーザ不在エラー
+  'E010205', // DBのRoleが想定外の値エラー
   'E010301', // メールアドレス登録済みエラー
   'E010302', // authorId重複エラー
   'E010401', // PONumber重複エラー
diff --git a/dictation_server/src/common/error/message.ts b/dictation_server/src/common/error/message.ts
index 96e8c25..060dabe 100644
--- a/dictation_server/src/common/error/message.ts
+++ b/dictation_server/src/common/error/message.ts
@@ -16,6 +16,7 @@ export const errors: Errors = {
   E010202: 'Email already verified user Error.',
   E010203: 'Administrator Permissions Error.',
   E010204: 'User not Found Error.',
+  E010205: 'Role from DB is unexpected value Error.',
   E010301: 'This email user already created Error',
   E010302: 'This AuthorId already used Error',
   E010401: 'This PoNumber already used Error',
diff --git a/dictation_server/src/constants/index.ts b/dictation_server/src/constants/index.ts
index 134d4fb..19d6bb6 100644
--- a/dictation_server/src/constants/index.ts
+++ b/dictation_server/src/constants/index.ts
@@ -83,10 +83,23 @@ export const BLOB_STORAGE_REGION_EU = [
 ];
 
 /**
- * ロールNone
- * @const {string}
+ * 管理ロール
+ * @const {string[]}
  */
-export const ROLE_NONE = 'None';
+export const ADMIN_ROLES = {
+  ADMIN: 'admin',
+  STANDARD: 'standard',
+} as const;
+
+/**
+ * ロール
+ * @const {string[]}
+ */
+export const USER_ROLES = {
+  NONE: 'none',
+  AUTHOR: 'author',
+  TYPIST: 'typist',
+} as const;
 
 /**
  * ライセンス注文ステータス（発行待ち）
diff --git a/dictation_server/src/features/accounts/accounts.controller.ts b/dictation_server/src/features/accounts/accounts.controller.ts
index 4158e81..b2f359c 100644
--- a/dictation_server/src/features/accounts/accounts.controller.ts
+++ b/dictation_server/src/features/accounts/accounts.controller.ts
@@ -3,7 +3,7 @@ import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';
 import { ErrorResponse } from '../../common/error/types/types';
 import { AccountsService } from './accounts.service';
 import { CreateAccountRequest, CreateAccountResponse } from './types/types';
-import { ROLE_NONE } from '../../constants';
+import { USER_ROLES } from '../../constants';
 
 @ApiTags('accounts')
 @Controller('accounts')
@@ -39,7 +39,7 @@ export class AccountsController {
       adminName,
       acceptedTermsVersion,
     } = body;
-    const role = ROLE_NONE;
+    const role = USER_ROLES.NONE;
 
     await this.accountService.createAccount(
       companyName,
diff --git a/dictation_server/src/features/auth/auth.service.ts b/dictation_server/src/features/auth/auth.service.ts
index 12eae50..9657ac0 100644
--- a/dictation_server/src/features/auth/auth.service.ts
+++ b/dictation_server/src/features/auth/auth.service.ts
@@ -13,6 +13,7 @@ import {
 import { AdB2cService } from '../../gateways/adb2c/adb2c.service';
 import { CryptoService } from '../../gateways/crypto/crypto.service';
 import { User } from '../../repositories/users/entity/user.entity';
+import { ADMIN_ROLES, USER_ROLES } from '../../constants';
 
 @Injectable()
 export class AuthService {
@@ -76,14 +77,33 @@ export class AuthService {
 
     const privateKey = await this.cryptoService.getPrivateKey();
 
+    // ユーザーのロールを設定
+    // 万一不正なRoleが登録されていた場合、そのままDBの値を使用すると不正なロールのリフレッシュトークンが発行されるため、
+    // ロールの設定値はDBに保存したRoleの値を直接トークンに入れないように定数で設定する
+    // ※none/author/typist以外はロールに設定されない
+    let role = '';
+    if (user.role === USER_ROLES.NONE) {
+      role = USER_ROLES.NONE;
+    } else if (user.role === USER_ROLES.AUTHOR) {
+      role = USER_ROLES.AUTHOR;
+    } else if (user.role === USER_ROLES.TYPIST) {
+      role = USER_ROLES.TYPIST;
+    } else {
+      this.logger.error(`Role from DB is unexpected value. role=${user.role}`);
+      throw new HttpException(
+        makeErrorResponse('E010205'),
+        HttpStatus.INTERNAL_SERVER_ERROR,
+      );
+    }
+
     const token = sign<RefreshToken>(
       {
         //ユーザーの属しているアカウントの管理者にユーザーが設定されていればadminをセットする
-        role: `${user.role} ${
+        role: `${role} ${
           user.account.primary_admin_user_id === user.id ||
           user.account.secondary_admin_user_id === user.id
-            ? 'admin'
-            : 'standard'
+            ? ADMIN_ROLES.ADMIN
+            : ADMIN_ROLES.STANDARD
         }`,
         userId: idToken.sub,
       },
diff --git a/dictation_server/src/features/users/types/types.ts b/dictation_server/src/features/users/types/types.ts
index 6d6116f..a960eba 100644
--- a/dictation_server/src/features/users/types/types.ts
+++ b/dictation_server/src/features/users/types/types.ts
@@ -1,4 +1,6 @@
 import { ApiProperty } from '@nestjs/swagger';
+import { IsIn } from 'class-validator';
+import { USER_ROLES } from '../../../constants';
 
 export class ConfirmRequest {
   @ApiProperty()
@@ -46,6 +48,7 @@ export class SignupRequest {
   name: string;
 
   @ApiProperty({ description: 'none/author/typist' })
+  @IsIn([USER_ROLES.NONE, USER_ROLES.AUTHOR, USER_ROLES.TYPIST])
   role: string;
 
   @ApiProperty({ required: false })