From c836157edb869e80b15f51bb8f48725e3b898b9a Mon Sep 17 00:00:00 2001 From: syuilo Date: Fri, 22 Sep 2023 14:12:33 +0900 Subject: enhance: 二要素認証設定時のセキュリティを強化 (#11863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * enhance: 二要素認証設定時のセキュリティを強化 パスワード入力が必要な操作を行う際、二要素認証が有効であれば確認コードの入力も必要にする * Update CoreModule.ts * Update 2fa.ts * wip * wip * Update 2fa.ts * tweak --- .../backend/src/server/api/endpoints/i/2fa/done.ts | 2 +- .../src/server/api/endpoints/i/2fa/key-done.ts | 20 ++++++++++++++++--- .../src/server/api/endpoints/i/2fa/register-key.ts | 20 ++++++++++++++++--- .../src/server/api/endpoints/i/2fa/register.ts | 21 +++++++++++++++++--- .../src/server/api/endpoints/i/2fa/remove-key.ts | 20 ++++++++++++++++--- .../src/server/api/endpoints/i/2fa/unregister.ts | 20 ++++++++++++++++--- .../src/server/api/endpoints/i/change-password.ts | 22 ++++++++++++++++++--- .../src/server/api/endpoints/i/delete-account.ts | 23 ++++++++++++++++++---- .../src/server/api/endpoints/i/update-email.ts | 20 ++++++++++++++++--- 9 files changed, 142 insertions(+), 26 deletions(-) (limited to 'packages/backend/src/server/api/endpoints/i') diff --git a/packages/backend/src/server/api/endpoints/i/2fa/done.ts b/packages/backend/src/server/api/endpoints/i/2fa/done.ts index c6a193fbb5..9f8e2894b8 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/done.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/done.ts @@ -47,7 +47,7 @@ export default class extends Endpoint { // eslint- secret: OTPAuth.Secret.fromBase32(profile.twoFactorTempSecret), digits: 6, token, - window: 1, + window: 5, }); if (delta === null) { diff --git a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts index 4b0e761bb2..6d530aba3b 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts @@ -12,6 +12,7 @@ import { GlobalEventService } from '@/core/GlobalEventService.js'; import type { UserProfilesRepository, UserSecurityKeysRepository } from '@/models/_.js'; import { WebAuthnService } from '@/core/WebAuthnService.js'; import { ApiError } from '@/server/api/error.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -37,6 +38,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, name: { type: 'string', minLength: 1, maxLength: 30 }, credential: { type: 'object' }, }, @@ -54,16 +56,28 @@ export default class extends Endpoint { private userSecurityKeysRepository: UserSecurityKeysRepository, private webAuthnService: WebAuthnService, + private userAuthService: UserAuthService, private userEntityService: UserEntityService, private globalEventService: GlobalEventService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password ?? ''); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } - if (!same) { + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts index b4d5237941..c39005f2dd 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts @@ -10,6 +10,7 @@ import type { UserProfilesRepository } from '@/models/_.js'; import { DI } from '@/di-symbols.js'; import { WebAuthnService } from '@/core/WebAuthnService.js'; import { ApiError } from '@/server/api/error.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -41,6 +42,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, }, required: ['password'], } as const; @@ -53,8 +55,10 @@ export default class extends Endpoint { private userProfilesRepository: UserProfilesRepository, private webAuthnService: WebAuthnService, + private userAuthService: UserAuthService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOne({ where: { userId: me.id, @@ -66,10 +70,20 @@ export default class extends Endpoint { throw new ApiError(meta.errors.userNotFound); } - // Compare password - const same = await bcrypt.compare(ps.password, profile.password ?? ''); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } - if (!same) { + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register.ts b/packages/backend/src/server/api/endpoints/i/2fa/register.ts index 9d027b25bb..b358c812ee 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register.ts @@ -12,6 +12,7 @@ import { Endpoint } from '@/server/api/endpoint-base.js'; import { DI } from '@/di-symbols.js'; import type { Config } from '@/config.js'; import { ApiError } from '@/server/api/error.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -31,6 +32,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, }, required: ['password'], } as const; @@ -43,14 +45,27 @@ export default class extends Endpoint { // eslint- @Inject(DI.userProfilesRepository) private userProfilesRepository: UserProfilesRepository, + + private userAuthService: UserAuthService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password ?? ''); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } + + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } - if (!same) { + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts index ad2cb8c20b..da8ac98556 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts @@ -11,6 +11,7 @@ import { UserEntityService } from '@/core/entities/UserEntityService.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; import { DI } from '@/di-symbols.js'; import { ApiError } from '@/server/api/error.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -30,6 +31,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, credentialId: { type: 'string' }, }, required: ['password', 'credentialId'], @@ -45,15 +47,27 @@ export default class extends Endpoint { // eslint- private userProfilesRepository: UserProfilesRepository, private userEntityService: UserEntityService, + private userAuthService: UserAuthService, private globalEventService: GlobalEventService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password ?? ''); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } - if (!same) { + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts index b834dfff4c..338f12c5cd 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts @@ -11,6 +11,7 @@ import type { UserProfilesRepository } from '@/models/_.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; import { DI } from '@/di-symbols.js'; import { ApiError } from '@/server/api/error.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -30,6 +31,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, }, required: ['password'], } as const; @@ -41,15 +43,27 @@ export default class extends Endpoint { // eslint- private userProfilesRepository: UserProfilesRepository, private userEntityService: UserEntityService, + private userAuthService: UserAuthService, private globalEventService: GlobalEventService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password ?? ''); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } - if (!same) { + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + + const passwordMatched = await bcrypt.compare(ps.password, profile.password ?? ''); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } diff --git a/packages/backend/src/server/api/endpoints/i/change-password.ts b/packages/backend/src/server/api/endpoints/i/change-password.ts index 868cff8ad7..a3c37ffdb7 100644 --- a/packages/backend/src/server/api/endpoints/i/change-password.ts +++ b/packages/backend/src/server/api/endpoints/i/change-password.ts @@ -8,6 +8,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UserProfilesRepository } from '@/models/_.js'; import { DI } from '@/di-symbols.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -20,6 +21,7 @@ export const paramDef = { properties: { currentPassword: { type: 'string' }, newPassword: { type: 'string', minLength: 1 }, + token: { type: 'string', nullable: true }, }, required: ['currentPassword', 'newPassword'], } as const; @@ -29,14 +31,28 @@ export default class extends Endpoint { // eslint- constructor( @Inject(DI.userProfilesRepository) private userProfilesRepository: UserProfilesRepository, + + private userAuthService: UserAuthService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.currentPassword, profile.password!); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } + + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + + const passwordMatched = await bcrypt.compare(ps.currentPassword, profile.password!); - if (!same) { + if (!passwordMatched) { throw new Error('incorrect password'); } diff --git a/packages/backend/src/server/api/endpoints/i/delete-account.ts b/packages/backend/src/server/api/endpoints/i/delete-account.ts index f318d9cda9..fbac845fda 100644 --- a/packages/backend/src/server/api/endpoints/i/delete-account.ts +++ b/packages/backend/src/server/api/endpoints/i/delete-account.ts @@ -9,6 +9,7 @@ import type { UsersRepository, UserProfilesRepository } from '@/models/_.js'; import { Endpoint } from '@/server/api/endpoint-base.js'; import { DeleteAccountService } from '@/core/DeleteAccountService.js'; import { DI } from '@/di-symbols.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; export const meta = { requireCredential: true, @@ -20,6 +21,7 @@ export const paramDef = { type: 'object', properties: { password: { type: 'string' }, + token: { type: 'string', nullable: true }, }, required: ['password'], } as const; @@ -33,19 +35,32 @@ export default class extends Endpoint { // eslint- @Inject(DI.userProfilesRepository) private userProfilesRepository: UserProfilesRepository, + private userAuthService: UserAuthService, private deleteAccountService: DeleteAccountService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); + + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } + + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } + const userDetailed = await this.usersRepository.findOneByOrFail({ id: me.id }); if (userDetailed.isDeleted) { return; } - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + const passwordMatched = await bcrypt.compare(ps.password, profile.password!); + if (!passwordMatched) { throw new Error('incorrect password'); } diff --git a/packages/backend/src/server/api/endpoints/i/update-email.ts b/packages/backend/src/server/api/endpoints/i/update-email.ts index 77135bf855..a36b3a732b 100644 --- a/packages/backend/src/server/api/endpoints/i/update-email.ts +++ b/packages/backend/src/server/api/endpoints/i/update-email.ts @@ -14,6 +14,7 @@ import type { Config } from '@/config.js'; import { DI } from '@/di-symbols.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -46,6 +47,7 @@ export const paramDef = { properties: { password: { type: 'string' }, email: { type: 'string', nullable: true }, + token: { type: 'string', nullable: true }, }, required: ['password'], } as const; @@ -61,15 +63,27 @@ export default class extends Endpoint { // eslint- private userEntityService: UserEntityService, private emailService: EmailService, + private userAuthService: UserAuthService, private globalEventService: GlobalEventService, ) { super(meta, paramDef, async (ps, me) => { + const token = ps.token; const profile = await this.userProfilesRepository.findOneByOrFail({ userId: me.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); + if (profile.twoFactorEnabled) { + if (token == null) { + throw new Error('authentication failed'); + } + + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { + throw new Error('authentication failed'); + } + } - if (!same) { + const passwordMatched = await bcrypt.compare(ps.password, profile.password!); + if (!passwordMatched) { throw new ApiError(meta.errors.incorrectPassword); } -- cgit v1.2.3-freya