From a455304a7a479174be98cb9a4b8c92086028d98d Mon Sep 17 00:00:00 2001 From: Mo Date: Mon, 19 Feb 2024 10:43:05 -0600 Subject: [PATCH] refactor: remove application mfa helper functions (#2852) --- packages/snjs/lib/Application/Application.ts | 22 ----------------- .../Application/Dependencies/Dependencies.ts | 2 ++ packages/snjs/lib/Services/Mfa/MfaService.ts | 13 +++++++++- packages/snjs/mocha/mfa_service.test.js | 24 +++++++++---------- packages/snjs/mocha/recovery.test.js | 10 ++++---- 5 files changed, 31 insertions(+), 40 deletions(-) diff --git a/packages/snjs/lib/Application/Application.ts b/packages/snjs/lib/Application/Application.ts index 857ddb85d..f44bc95bb 100644 --- a/packages/snjs/lib/Application/Application.ts +++ b/packages/snjs/lib/Application/Application.ts @@ -917,28 +917,6 @@ export class SNApplication implements ApplicationInterface, AppGroupManagedAppli return service.canAttemptDecryptionOfItem(item) } - public async isMfaActivated(): Promise { - return this.mfa.isMfaActivated() - } - - public async generateMfaSecret(): Promise { - return this.mfa.generateMfaSecret() - } - - public async getOtpToken(secret: string): Promise { - return this.mfa.getOtpToken(secret) - } - - public async enableMfa(secret: string, otpToken: string): Promise { - return this.mfa.enableMfa(secret, otpToken) - } - - public async disableMfa(): Promise { - if (await this.protections.authorizeMfaDisable()) { - return this.mfa.disableMfa() - } - } - async isUsingHomeServer(): Promise { const homeServerService = this.dependencies.get(TYPES.HomeServerService) diff --git a/packages/snjs/lib/Application/Dependencies/Dependencies.ts b/packages/snjs/lib/Application/Dependencies/Dependencies.ts index 7f5815742..d9bfbcb24 100644 --- a/packages/snjs/lib/Application/Dependencies/Dependencies.ts +++ b/packages/snjs/lib/Application/Dependencies/Dependencies.ts @@ -148,6 +148,7 @@ import { SyncBackoffService, SyncBackoffServiceInterface, StorageServiceInterface, + ProtectionsClientInterface, } from '@standardnotes/services' import { ItemManager } from '../../Services/Items/ItemManager' import { PayloadManager } from '../../Services/Payloads/PayloadManager' @@ -1229,6 +1230,7 @@ export class Dependencies { this.get(TYPES.SettingsService), this.get(TYPES.Crypto), this.get(TYPES.FeaturesService), + this.get(TYPES.ProtectionService), this.get(TYPES.InternalEventBus), ) }) diff --git a/packages/snjs/lib/Services/Mfa/MfaService.ts b/packages/snjs/lib/Services/Mfa/MfaService.ts index 462fc83d9..495af062f 100644 --- a/packages/snjs/lib/Services/Mfa/MfaService.ts +++ b/packages/snjs/lib/Services/Mfa/MfaService.ts @@ -1,7 +1,13 @@ import { SettingsService } from '../Settings' import { PureCryptoInterface } from '@standardnotes/sncrypto-common' import { FeaturesService } from '../Features/FeaturesService' -import { AbstractService, InternalEventBusInterface, MfaServiceInterface, SignInStrings } from '@standardnotes/services' +import { + AbstractService, + InternalEventBusInterface, + MfaServiceInterface, + ProtectionsClientInterface, + SignInStrings, +} from '@standardnotes/services' import { SettingName } from '@standardnotes/domain-core' export class MfaService extends AbstractService implements MfaServiceInterface { @@ -9,6 +15,7 @@ export class MfaService extends AbstractService implements MfaServiceInterface { private settingsService: SettingsService, private crypto: PureCryptoInterface, private featuresService: FeaturesService, + private protections: ProtectionsClientInterface, protected override internalEventBus: InternalEventBusInterface, ) { super(internalEventBus) @@ -48,6 +55,10 @@ export class MfaService extends AbstractService implements MfaServiceInterface { } async disableMfa(): Promise { + if (!(await this.protections.authorizeMfaDisable())) { + return + } + return await this.settingsService.deleteSetting(SettingName.create(SettingName.NAMES.MfaSecret).getValue()) } diff --git a/packages/snjs/mocha/mfa_service.test.js b/packages/snjs/mocha/mfa_service.test.js index 6cb62b967..6b1e68d75 100644 --- a/packages/snjs/mocha/mfa_service.test.js +++ b/packages/snjs/mocha/mfa_service.test.js @@ -33,7 +33,7 @@ describe('mfa service', () => { it('generates 160 bit base32-encoded mfa secret', async () => { const RFC4648 = /[ABCDEFGHIJKLMNOPQRSTUVWXYZ234567]/g - const secret = await application.generateMfaSecret() + const secret = await application.mfa.generateMfaSecret() expect(secret).to.have.lengthOf(32) expect(secret.replace(RFC4648, '')).to.have.lengthOf(0) }) @@ -43,30 +43,30 @@ describe('mfa service', () => { Factory.handlePasswordChallenges(application, accountPassword) - expect(await application.isMfaActivated()).to.equal(false) + expect(await application.mfa.isMfaActivated()).to.equal(false) - const secret = await application.generateMfaSecret() - const token = await application.getOtpToken(secret) + const secret = await application.mfa.generateMfaSecret() + const token = await application.mfa.getOtpToken(secret) - await application.enableMfa(secret, token) + await application.mfa.enableMfa(secret, token) - expect(await application.isMfaActivated()).to.equal(true) + expect(await application.mfa.isMfaActivated()).to.equal(true) - await application.disableMfa() + await application.mfa.disableMfa() - expect(await application.isMfaActivated()).to.equal(false) + expect(await application.mfa.isMfaActivated()).to.equal(false) }).timeout(Factory.TenSecondTimeout) it('prompts for account password when disabling mfa', async () => { await registerApp(application) Factory.handlePasswordChallenges(application, accountPassword) - const secret = await application.generateMfaSecret() - const token = await application.getOtpToken(secret) + const secret = await application.mfa.generateMfaSecret() + const token = await application.mfa.getOtpToken(secret) sinon.spy(application.challenges, 'sendChallenge') - await application.enableMfa(secret, token) - await application.disableMfa() + await application.mfa.enableMfa(secret, token) + await application.mfa.disableMfa() const spyCall = application.challenges.sendChallenge.getCall(0) const challenge = spyCall.firstArg diff --git a/packages/snjs/mocha/recovery.test.js b/packages/snjs/mocha/recovery.test.js index f4e84a86e..574f39fee 100644 --- a/packages/snjs/mocha/recovery.test.js +++ b/packages/snjs/mocha/recovery.test.js @@ -71,12 +71,12 @@ describe('account recovery', function () { }) it('should disable MFA after recovery sign in', async () => { - const secret = await application.generateMfaSecret() - const token = await application.getOtpToken(secret) + const secret = await application.mfa.generateMfaSecret() + const token = await application.mfa.getOtpToken(secret) - await application.enableMfa(secret, token) + await application.mfa.enableMfa(secret, token) - expect(await application.isMfaActivated()).to.equal(true) + expect(await application.mfa.isMfaActivated()).to.equal(true) const generatedRecoveryCodes = await application.getRecoveryCodes.execute() @@ -88,7 +88,7 @@ describe('account recovery', function () { password: context.password, }) - expect(await application.isMfaActivated()).to.equal(false) + expect(await application.mfa.isMfaActivated()).to.equal(false) }) it('should not allow to sign in with recovery code and invalid credentials', async () => {