Merged PR 121: リフレッシュトークンの発行方法・発行内容が不適切な問題の修正
## 概要 [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の変更 - なし ## 動作確認状況 - ローカルで確認 ## 補足 - ユーザー追加時に登録されるロールが変更となったのでマージ後に利用する場合は再度の登録をお願いします。
This commit is contained in:
makabe.t
parent
ab618984eb
commit
e4f84f78ba
@ -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];
|
||||
|
||||
@ -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"))}
|
||||
|
||||
@ -27,6 +27,7 @@ export const ErrorCodes = [
|
||||
'E010202', // 認証済ユーザエラー
|
||||
'E010203', // 管理ユーザ権限エラー
|
||||
'E010204', // ユーザ不在エラー
|
||||
'E010205', // DBのRoleが想定外の値エラー
|
||||
'E010301', // メールアドレス登録済みエラー
|
||||
'E010302', // authorId重複エラー
|
||||
'E010401', // PONumber重複エラー
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -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;
|
||||
|
||||
/**
|
||||
* ライセンス注文ステータス（発行待ち）
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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,
|
||||
},
|
||||
|
||||
@ -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 })
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user