diff options
| author | syuilo <Syuilotan@yahoo.co.jp> | 2023-09-22 14:12:33 +0900 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2023-09-22 14:12:33 +0900 |
| commit | c836157edb869e80b15f51bb8f48725e3b898b9a (patch) | |
| tree | c275a865b697afa4c5d045d16b1ca8999999f9cf /packages/backend/src | |
| parent | tweak ui (diff) | |
| download | sharkey-c836157edb869e80b15f51bb8f48725e3b898b9a.tar.gz sharkey-c836157edb869e80b15f51bb8f48725e3b898b9a.tar.bz2 sharkey-c836157edb869e80b15f51bb8f48725e3b898b9a.zip | |
enhance: 二要素認証設定時のセキュリティを強化 (#11863)
* enhance: 二要素認証設定時のセキュリティを強化
パスワード入力が必要な操作を行う際、二要素認証が有効であれば確認コードの入力も必要にする
* Update CoreModule.ts
* Update 2fa.ts
* wip
* wip
* Update 2fa.ts
* tweak
Diffstat (limited to 'packages/backend/src')
12 files changed, 202 insertions, 45 deletions
diff --git a/packages/backend/src/core/CoreModule.ts b/packages/backend/src/core/CoreModule.ts index 18271ee346..78333e70a5 100644 --- a/packages/backend/src/core/CoreModule.ts +++ b/packages/backend/src/core/CoreModule.ts @@ -51,6 +51,7 @@ import { UserKeypairService } from './UserKeypairService.js'; import { UserListService } from './UserListService.js'; import { UserMutingService } from './UserMutingService.js'; import { UserSuspendService } from './UserSuspendService.js'; +import { UserAuthService } from './UserAuthService.js'; import { VideoProcessingService } from './VideoProcessingService.js'; import { WebhookService } from './WebhookService.js'; import { ProxyAccountService } from './ProxyAccountService.js'; @@ -177,6 +178,7 @@ const $UserKeypairService: Provider = { provide: 'UserKeypairService', useExisti const $UserListService: Provider = { provide: 'UserListService', useExisting: UserListService }; const $UserMutingService: Provider = { provide: 'UserMutingService', useExisting: UserMutingService }; const $UserSuspendService: Provider = { provide: 'UserSuspendService', useExisting: UserSuspendService }; +const $UserAuthService: Provider = { provide: 'UserAuthService', useExisting: UserAuthService }; const $VideoProcessingService: Provider = { provide: 'VideoProcessingService', useExisting: VideoProcessingService }; const $WebhookService: Provider = { provide: 'WebhookService', useExisting: WebhookService }; const $UtilityService: Provider = { provide: 'UtilityService', useExisting: UtilityService }; @@ -306,6 +308,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting UserListService, UserMutingService, UserSuspendService, + UserAuthService, VideoProcessingService, WebhookService, UtilityService, @@ -428,6 +431,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting $UserListService, $UserMutingService, $UserSuspendService, + $UserAuthService, $VideoProcessingService, $WebhookService, $UtilityService, @@ -551,6 +555,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting UserListService, UserMutingService, UserSuspendService, + UserAuthService, VideoProcessingService, WebhookService, UtilityService, @@ -672,6 +677,7 @@ const $ApQuestionService: Provider = { provide: 'ApQuestionService', useExisting $UserListService, $UserMutingService, $UserSuspendService, + $UserAuthService, $VideoProcessingService, $WebhookService, $UtilityService, diff --git a/packages/backend/src/core/UserAuthService.ts b/packages/backend/src/core/UserAuthService.ts new file mode 100644 index 0000000000..ccf4dfc6bd --- /dev/null +++ b/packages/backend/src/core/UserAuthService.ts @@ -0,0 +1,45 @@ +/* + * SPDX-FileCopyrightText: syuilo and other misskey contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { Inject, Injectable } from '@nestjs/common'; +import { QueryFailedError } from 'typeorm'; +import * as OTPAuth from 'otpauth'; +import { DI } from '@/di-symbols.js'; +import type { MiUserProfile, UserProfilesRepository, UsersRepository } from '@/models/_.js'; +import { bindThis } from '@/decorators.js'; +import { isDuplicateKeyValueError } from '@/misc/is-duplicate-key-value-error.js'; +import type { MiLocalUser } from '@/models/User.js'; + +@Injectable() +export class UserAuthService { + constructor( + @Inject(DI.usersRepository) + private usersRepository: UsersRepository, + + @Inject(DI.userProfilesRepository) + private userProfilesRepository: UserProfilesRepository, + ) { + } + + @bindThis + public async twoFactorAuthenticate(profile: MiUserProfile, token: string): Promise<void> { + if (profile.twoFactorBackupSecret?.includes(token)) { + await this.userProfilesRepository.update({ userId: profile.userId }, { + twoFactorBackupSecret: profile.twoFactorBackupSecret.filter((secret) => secret !== token), + }); + } else { + const delta = OTPAuth.TOTP.validate({ + secret: OTPAuth.Secret.fromBase32(profile.twoFactorSecret!), + digits: 6, + token, + window: 5, + }); + + if (delta === null) { + throw new Error('authentication failed'); + } + } + } +} diff --git a/packages/backend/src/server/api/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts index 48d74e2b02..150f3f24d4 100644 --- a/packages/backend/src/server/api/SigninApiService.ts +++ b/packages/backend/src/server/api/SigninApiService.ts @@ -19,6 +19,7 @@ import type { MiLocalUser } from '@/models/User.js'; import { IdService } from '@/core/IdService.js'; import { bindThis } from '@/decorators.js'; import { WebAuthnService } from '@/core/WebAuthnService.js'; +import { UserAuthService } from '@/core/UserAuthService.js'; import { RateLimiterService } from './RateLimiterService.js'; import { SigninService } from './SigninService.js'; import type { AuthenticationResponseJSON } from '@simplewebauthn/typescript-types'; @@ -42,6 +43,7 @@ export class SigninApiService { private idService: IdService, private rateLimiterService: RateLimiterService, private signinService: SigninService, + private userAuthService: UserAuthService, private webAuthnService: WebAuthnService, ) { } @@ -124,7 +126,7 @@ export class SigninApiService { const same = await bcrypt.compare(password, profile.password!); const fail = async (status?: number, failure?: { id: string }) => { - // Append signin history + // Append signin history await this.signinsRepository.insert({ id: this.idService.genId(), createdAt: new Date(), @@ -154,27 +156,15 @@ export class SigninApiService { }); } - if (profile.twoFactorBackupSecret?.includes(token)) { - await this.userProfilesRepository.update({ userId: profile.userId }, { - twoFactorBackupSecret: profile.twoFactorBackupSecret.filter((secret) => secret !== token), - }); - return this.signinService.signin(request, reply, user); - } - - const delta = OTPAuth.TOTP.validate({ - secret: OTPAuth.Secret.fromBase32(profile.twoFactorSecret!), - digits: 6, - token, - window: 1, - }); - - if (delta === null) { + try { + await this.userAuthService.twoFactorAuthenticate(profile, token); + } catch (e) { return await fail(403, { id: 'cdf1235b-ac71-46d4-a3a6-84ccce48df6f', }); - } else { - return this.signinService.signin(request, reply, user); } + + return this.signinService.signin(request, reply, user); } else if (body.credential) { if (!same && !profile.usePasswordLessLogin) { return await fail(403, { @@ -203,6 +193,6 @@ export class SigninApiService { reply.code(200); return authRequest; } - // never get here + // never get here } } 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { 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<typeof meta, typeof paramDef> { 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<typeof meta, typeof paramDef> { 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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<typeof meta, typeof paramDef> { // 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); } |