From a1b7505035d3f2a4754bab3fcb689c42e7ff2b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B9=AF=E6=9C=AC=20=E9=96=8B?= Date: Tue, 6 Feb 2024 05:03:45 +0000 Subject: [PATCH] =?UTF-8?q?Merged=20PR=20732:=20[=E6=94=B9=E5=96=84]?= =?UTF-8?q?=E8=AA=8D=E8=A8=BC=E7=94=A8URL=E3=81=AB=E3=81=A4=E3=81=84?= =?UTF-8?q?=E3=81=A6=E3=80=81=E3=83=89=E3=83=A1=E3=82=A4=E3=83=B3=E5=90=8D?= =?UTF-8?q?=E3=81=AE=E6=9C=AB=E5=B0=BE=E3=81=AB/=E3=81=8C=E5=BF=85?= =?UTF-8?q?=E8=A6=81=E3=81=A8=E3=81=AA=E3=82=8B=E3=81=93=E3=81=A8=E3=81=B8?= =?UTF-8?q?=E3=81=AE=E5=AF=BE=E5=BF=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 概要 [Task3625: [改善]認証用URLについて、ドメイン名の末尾に/が必要となることへの対応](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/3625) - URLクラスとpathクラスを用いてURLを構築するよう修正 - 送信するメールに関わるテストを追加 ## レビューポイント - 修正内容は妥当であるか - 漏れていそうなURL系の処理はないか - 工数面を考慮したコスト対効果の観点から、メール送信を行うテスト全てに送信メール内容のチェックを行うテストコードは入れなかったが許容可能か ## 動作確認状況 - npm run testは通過 - `.env.test` の `APP_DOMAIN` の末尾 `/` を付けて通過 & 消して通過 するかを確認 - **一応追試をお願いしたいです** --- dictation_server/src/common/test/overrides.ts | 3 +- .../accounts/accounts.service.spec.ts | 115 +++++++++++++++++- .../licenses/licenses.service.spec.ts | 72 ++++++++++- .../src/features/users/users.service.spec.ts | 49 +++++++- .../src/gateways/sendgrid/sendgrid.service.ts | 40 +++--- 5 files changed, 250 insertions(+), 29 deletions(-) diff --git a/dictation_server/src/common/test/overrides.ts b/dictation_server/src/common/test/overrides.ts index 146d00a..ffaf05d 100644 --- a/dictation_server/src/common/test/overrides.ts +++ b/dictation_server/src/common/test/overrides.ts @@ -82,7 +82,8 @@ export const overrideSendgridService = ( overrides: { sendMail?: ( context: Context, - to: string, + to: string[], + cc: string[], from: string, subject: string, text: string, diff --git a/dictation_server/src/features/accounts/accounts.service.spec.ts b/dictation_server/src/features/accounts/accounts.service.spec.ts index 4bfd687..d661635 100644 --- a/dictation_server/src/features/accounts/accounts.service.spec.ts +++ b/dictation_server/src/features/accounts/accounts.service.spec.ts @@ -134,7 +134,26 @@ describe('createAccount', () => { }, }); - overrideSendgridService(service, {}); + let _subject: string = ""; + let _url: string | undefined = ""; + overrideSendgridService(service, { + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); overrideBlobstorageService(service, { createContainer: async () => { return; @@ -175,6 +194,10 @@ describe('createAccount', () => { expect(user?.accepted_dpa_version).toBe(acceptedDpaVersion); expect(user?.account_id).toBe(accountId); expect(user?.role).toBe(role); + + // 想定通りのメールが送られているか確認 + expect(_subject).toBe('User Registration Notification [U-102]'); + expect(_url?.startsWith('http://localhost:8081/mail-confirm?verify=')).toBeTruthy(); }); it('アカウントを作成がAzure AD B2Cへの通信失敗によって失敗すると500エラーが発生する', async () => { @@ -5704,9 +5727,39 @@ describe('アカウント情報更新', () => { const module = await makeTestingModule(source); if (!module) fail(); const service = module.get(AccountsService); + let _subject: string = ""; + let _url: string | undefined = ""; overrideSendgridService(service, { - sendMail: async () => { - return; + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); + overrideAdB2cService(service, { + getUser: async (context, externalId) => { + return { + displayName: 'TEMP' + externalId, + id: externalId, + identities: [ + { + signInType: ADB2C_SIGN_IN_TYPE.EMAILADDRESS, + issuer: 'xxxxxx', + issuerAssignedId: 'mail@example.com', + }, + ], + }; }, }); @@ -5733,6 +5786,9 @@ describe('アカウント情報更新', () => { expect(account?.delegation_permission).toBe(true); expect(account?.primary_admin_user_id).toBe(tier5Accounts.admin.id); expect(account?.secondary_admin_user_id).toBe(null); + // 想定通りのメールが送られているか確認 + expect(_subject).toBe('Account Edit Notification [U-112]'); + expect(_url).toBe('http://localhost:8081/'); }); it('アカウント情報を更新する（第五階層以外が実行）', async () => { if (!source) fail(); @@ -6364,7 +6420,27 @@ describe('deleteAccountAndData', () => { const module = await makeTestingModule(source); if (!module) fail(); const service = module.get(AccountsService); - overrideSendgridService(service, {}); + let _subject: string = ''; + let _url: string | undefined = ''; + overrideSendgridService(service, { + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); + // 第一～第四階層のアカウント作成 const { tier1Accounts: tier1Accounts, @@ -6485,10 +6561,36 @@ describe('deleteAccountAndData', () => { licensesB[0].id, ); - // ADB2Cユーザーの削除成功 overrideAdB2cService(service, { + getUser: async (context, externalId) => { + return { + displayName: 'TEMP' + externalId, + id: externalId, + identities: [ + { + signInType: ADB2C_SIGN_IN_TYPE.EMAILADDRESS, + issuer: 'xxxxxx', + issuerAssignedId: 'mail@example.com', + }, + ], + }; + }, + getUsers: async (context, externalIds) => { + return externalIds.map((x) => ({ + displayName: 'admin', + id: x, + identities: [ + { + signInType: ADB2C_SIGN_IN_TYPE.EMAILADDRESS, + issuer: 'xxxxxx', + issuerAssignedId: `mail+${x}@example.com`, + }, + ], + })); + }, deleteUsers: jest.fn(), }); + // blobstorageコンテナの削除成功 overrideBlobstorageService(service, { deleteContainer: jest.fn(), @@ -6559,6 +6661,9 @@ describe('deleteAccountAndData', () => { const LicenseAllocationHistoryArchive = await getLicenseAllocationHistoryArchive(source); expect(LicenseAllocationHistoryArchive.length).toBe(1); + + expect(_subject).toBe('Account Deleted Notification [U-111]'); + expect(_url).toBe('http://localhost:8081/'); }); it('アカウントの削除に失敗した場合はエラーを返す', async () => { if (!source) fail(); diff --git a/dictation_server/src/features/licenses/licenses.service.spec.ts b/dictation_server/src/features/licenses/licenses.service.spec.ts index 22be77f..cdc56b2 100644 --- a/dictation_server/src/features/licenses/licenses.service.spec.ts +++ b/dictation_server/src/features/licenses/licenses.service.spec.ts @@ -17,15 +17,15 @@ import { selectOrderLicense, } from './test/utility'; import { UsersService } from '../users/users.service'; -import { makeContext } from '../../common/log'; -import { LICENSE_ALLOCATED_STATUS, LICENSE_TYPE } from '../../constants'; +import { Context, makeContext } from '../../common/log'; +import { ADB2C_SIGN_IN_TYPE, LICENSE_ALLOCATED_STATUS, LICENSE_TYPE } from '../../constants'; import { makeHierarchicalAccounts, makeTestSimpleAccount, makeTestUser, } from '../../common/test/utility'; import { LicensesRepositoryService } from '../../repositories/licenses/licenses.repository.service'; -import { overrideSendgridService } from '../../common/test/overrides'; +import { overrideAdB2cService, overrideSendgridService } from '../../common/test/overrides'; import { truncateAllTable } from '../../common/test/init'; describe('ライセンス注文', () => { @@ -672,7 +672,18 @@ describe('ライセンス割り当て', () => { const module = await makeTestingModule(source); if (!module) fail(); - const { id: accountId } = await makeTestSimpleAccount(source); + const { id: dealerId } = await makeTestSimpleAccount(source, { company_name: "DEALER_COMPANY", tier: 4 }); + const { id: dealerAdminId } = await makeTestUser(source, { + account_id: dealerId, + external_id: 'userId_admin', + role: 'admin', + author_id: undefined, + }); + + const { id: accountId } = await makeTestSimpleAccount(source, { + parent_account_id: dealerId, + tier: 5 + }); const { id: userId } = await makeTestUser(source, { account_id: accountId, external_id: 'userId', @@ -701,7 +712,55 @@ describe('ライセンス割り当て', () => { ); const service = module.get(UsersService); - overrideSendgridService(service, {}); + let _subject: string = ''; + let _url: string | undefined = ''; + overrideAdB2cService(service, { + getUser: async (context, externalId) => { + return { + displayName: 'TEMP' + externalId, + id: externalId, + identities: [ + { + signInType: ADB2C_SIGN_IN_TYPE.EMAILADDRESS, + issuer: 'xxxxxx', + issuerAssignedId: 'mail@example.com', + }, + ], + }; + }, + getUsers: async (context, externalIds) => { + return externalIds.map((x) => ({ + displayName: 'admin', + id: x, + identities: [ + { + signInType: ADB2C_SIGN_IN_TYPE.EMAILADDRESS, + issuer: 'xxxxxx', + issuerAssignedId: `mail+${x}@example.com`, + }, + ], + })); + } + }); + + overrideSendgridService(service, { + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); const expiry_date = new NewAllocatedLicenseExpirationDate(); @@ -735,6 +794,9 @@ describe('ライセンス割り当て', () => { expect(licenseAllocationHistory.licenseAllocationHistory?.account_id).toBe( accountId, ); + + expect(_subject).toBe('License Assigned Notification [U-108]'); + expect(_url).toBe('http://localhost:8081/'); }); it('再割り当て可能なライセンスに対して、ライセンス割り当てが完了する', async () => { diff --git a/dictation_server/src/features/users/users.service.spec.ts b/dictation_server/src/features/users/users.service.spec.ts index cb27055..8e3cbf5 100644 --- a/dictation_server/src/features/users/users.service.spec.ts +++ b/dictation_server/src/features/users/users.service.spec.ts @@ -108,7 +108,26 @@ describe('UsersService.confirmUser', () => { }); const service = module.get(UsersService); - overrideSendgridService(service, {}); + let _subject: string = ''; + let _url: string | undefined = ''; + overrideSendgridService(service, { + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); // account id:1, user id: 2のトークン const token = @@ -149,6 +168,8 @@ describe('UsersService.confirmUser', () => { delete_order_id: null, user: null, }); + expect(_subject).toBe('Account Registered Notification [U-101]'); + expect(_url).toBe('http://localhost:8081/'); }, 600000); it('トークンの形式が不正な場合、形式不正エラーとなる。', async () => { @@ -506,7 +527,26 @@ describe('UsersService.createUser', () => { }; }, }); - overrideSendgridService(service, {}); + let _subject: string = ''; + let _url: string | undefined = ''; + overrideSendgridService(service, { + sendMail: async ( + context: Context, + to: string[], + cc: string[], + from: string, + subject: string, + text: string, + html: string, + ) => { + const urlPattern = /https?:\/\/[^\s]+/g; + const urls = text.match(urlPattern); + const url = urls?.pop(); + + _subject = subject; + _url = url; + }, + }); expect( await service.createUser( @@ -536,6 +576,11 @@ describe('UsersService.createUser', () => { // 他にユーザーが登録されていないことを確認 const users = await getUsers(source); expect(users.length).toEqual(2); + + expect(_subject).toBe('User Registration Notification [U-114]'); + expect( + _url?.startsWith('http://localhost:8081/mail-confirm/user?verify='), + ).toBeTruthy(); }); it('管理者権限のあるアクセストークンを使用して、新規ユーザが追加される(role:Author; 暗号化あり)', async () => { diff --git a/dictation_server/src/gateways/sendgrid/sendgrid.service.ts b/dictation_server/src/gateways/sendgrid/sendgrid.service.ts index 6e2fb15..985c7b1 100644 --- a/dictation_server/src/gateways/sendgrid/sendgrid.service.ts +++ b/dictation_server/src/gateways/sendgrid/sendgrid.service.ts @@ -21,6 +21,7 @@ import { VERIFY_LINK, TEMPORARY_PASSWORD, } from '../../templates/constants'; +import { URL } from 'node:url'; @Injectable() export class SendGridService { @@ -204,12 +205,13 @@ export class SendGridService { ); try { const subject = 'Account Registered Notification [U-101]'; + const url = new URL(this.appDomain).href; const html = this.templateU101Html .replaceAll(CUSTOMER_NAME, customerAccountName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); const text = this.templateU101Text .replaceAll(CUSTOMER_NAME, customerAccountName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); await this.sendMail( context, @@ -255,8 +257,9 @@ export class SendGridService { this.emailConfirmLifetime, privateKey, ); - const path = 'mail-confirm/'; - const verifyUrl = `${this.appDomain}${path}?verify=${token}`; + const paths = path.join('mail-confirm'); + const url = new URL(paths, this.appDomain).href; + const verifyUrl = `${url}?verify=${token}`; const subject = 'User Registration Notification [U-102]'; const html = this.templateU102Html.replaceAll(VERIFY_LINK, verifyUrl); @@ -466,6 +469,7 @@ export class SendGridService { ); try { const subject = 'License Assigned Notification [U-108]'; + const url = new URL(this.appDomain).href; // メールの本文を作成する const html = this.templateU108Html @@ -473,13 +477,13 @@ export class SendGridService { .replaceAll(DEALER_NAME, dealerAccountName) .replaceAll(USER_NAME, userName) .replaceAll(USER_EMAIL, userMail) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); const text = this.templateU108Text .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(DEALER_NAME, dealerAccountName) .replaceAll(USER_NAME, userName) .replaceAll(USER_EMAIL, userMail) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); const ccAddress = customerAdminMails.includes(userMail) ? [] : [userMail]; @@ -574,17 +578,18 @@ export class SendGridService { ); try { const subject = 'Account Deleted Notification [U-111]'; + const url = new URL(this.appDomain).href; // メールの本文を作成する const html = this.templateU111Html .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); const text = this.templateU111Text .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); // メールを送信する await this.sendMail( @@ -627,6 +632,7 @@ export class SendGridService { let html: string; let text: string; + const url = new URL(this.appDomain).href; // 親アカウントがない場合は別のテンプレートを使用する if (dealerAccountName === null) { @@ -634,22 +640,22 @@ export class SendGridService { html = this.templateU112NoParentHtml .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); text = this.templateU112NoParentText .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); } else { html = this.templateU112Html .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(DEALER_NAME, dealerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); text = this.templateU112Text .replaceAll(CUSTOMER_NAME, customerAccountName) .replaceAll(DEALER_NAME, dealerAccountName) .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(TOP_URL, this.appDomain); + .replaceAll(TOP_URL, url); } // メールを送信する @@ -745,18 +751,20 @@ export class SendGridService { this.emailConfirmLifetime, privateKey, ); - const path = 'mail-confirm/user/'; - const verifyLink = `${this.appDomain}${path}?verify=${token}`; + + const paths = path.join('mail-confirm', '/user'); + const url = new URL(paths, this.appDomain); + const verifyUrl = `${url}?verify=${token}`; const subject = 'User Registration Notification [U-114]'; // メールの本文を作成する const html = this.templateU114Html .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(VERIFY_LINK, verifyLink); + .replaceAll(VERIFY_LINK, verifyUrl); const text = this.templateU114Text .replaceAll(PRIMARY_ADMIN_NAME, primaryAdminName) - .replaceAll(VERIFY_LINK, verifyLink); + .replaceAll(VERIFY_LINK, verifyUrl); // メールを送信する await this.sendMail(