diff --git a/apps/browser/src/auth/background/service-factories/two-factor-service.factory.ts b/apps/browser/src/auth/background/service-factories/two-factor-service.factory.ts index 1d79bbbaf1..5af5eb0017 100644 --- a/apps/browser/src/auth/background/service-factories/two-factor-service.factory.ts +++ b/apps/browser/src/auth/background/service-factories/two-factor-service.factory.ts @@ -1,11 +1,13 @@ import { TwoFactorService as AbstractTwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { TwoFactorService } from "@bitwarden/common/auth/services/two-factor.service"; +import { GlobalStateProvider } from "@bitwarden/common/platform/state"; import { FactoryOptions, CachedServices, factory, } from "../../../platform/background/service-factories/factory-options"; +import { globalStateProviderFactory } from "../../../platform/background/service-factories/global-state-provider.factory"; import { I18nServiceInitOptions, i18nServiceFactory, @@ -19,7 +21,8 @@ type TwoFactorServiceFactoryOptions = FactoryOptions; export type TwoFactorServiceInitOptions = TwoFactorServiceFactoryOptions & I18nServiceInitOptions & - PlatformUtilsServiceInitOptions; + PlatformUtilsServiceInitOptions & + GlobalStateProvider; export async function twoFactorServiceFactory( cache: { twoFactorService?: AbstractTwoFactorService } & CachedServices, @@ -33,6 +36,7 @@ export async function twoFactorServiceFactory( new TwoFactorService( await i18nServiceFactory(cache, opts), await platformUtilsServiceFactory(cache, opts), + await globalStateProviderFactory(cache, opts), ), ); service.init(); diff --git a/apps/browser/src/auth/popup/two-factor-options.component.ts b/apps/browser/src/auth/popup/two-factor-options.component.ts index bad2e4a9e7..6191d277ad 100644 --- a/apps/browser/src/auth/popup/two-factor-options.component.ts +++ b/apps/browser/src/auth/popup/two-factor-options.component.ts @@ -2,7 +2,10 @@ import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { TwoFactorOptionsComponent as BaseTwoFactorOptionsComponent } from "@bitwarden/angular/auth/components/two-factor-options.component"; -import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; +import { + TwoFactorProviderDetails, + TwoFactorService, +} from "@bitwarden/common/auth/abstractions/two-factor.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -27,9 +30,9 @@ export class TwoFactorOptionsComponent extends BaseTwoFactorOptionsComponent { this.navigateTo2FA(); } - choose(p: any) { - super.choose(p); - this.twoFactorService.setSelectedProvider(p.type); + override async choose(p: TwoFactorProviderDetails) { + await super.choose(p); + await this.twoFactorService.setSelectedProvider(p.type); this.navigateTo2FA(); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index b4375df7d5..758c226bc3 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -3,8 +3,6 @@ import { Subject, firstValueFrom, merge } from "rxjs"; import { PinCryptoServiceAbstraction, PinCryptoService, - LoginStrategyServiceAbstraction, - LoginStrategyService, InternalUserDecryptionOptionsServiceAbstraction, UserDecryptionOptionsService, AuthRequestServiceAbstraction, @@ -38,7 +36,6 @@ import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarde import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service"; -import { TwoFactorService as TwoFactorServiceAbstraction } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { UserVerificationApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification-api.service.abstraction"; import { UserVerificationService as UserVerificationServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -54,7 +51,6 @@ import { KeyConnectorService } from "@bitwarden/common/auth/services/key-connect import { MasterPasswordService } from "@bitwarden/common/auth/services/master-password/master-password.service"; import { SsoLoginService } from "@bitwarden/common/auth/services/sso-login.service"; import { TokenService } from "@bitwarden/common/auth/services/token.service"; -import { TwoFactorService } from "@bitwarden/common/auth/services/two-factor.service"; import { UserVerificationApiService } from "@bitwarden/common/auth/services/user-verification/user-verification-api.service"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { @@ -277,7 +273,6 @@ export default class MainBackground { containerService: ContainerService; auditService: AuditServiceAbstraction; authService: AuthServiceAbstraction; - loginStrategyService: LoginStrategyServiceAbstraction; loginEmailService: LoginEmailServiceAbstraction; importApiService: ImportApiServiceAbstraction; importService: ImportServiceAbstraction; @@ -301,7 +296,6 @@ export default class MainBackground { providerService: ProviderServiceAbstraction; keyConnectorService: KeyConnectorServiceAbstraction; userVerificationService: UserVerificationServiceAbstraction; - twoFactorService: TwoFactorServiceAbstraction; vaultFilterService: VaultFilterService; usernameGenerationService: UsernameGenerationServiceAbstraction; encryptService: EncryptService; @@ -614,8 +608,6 @@ export default class MainBackground { this.stateService, ); - this.twoFactorService = new TwoFactorService(this.i18nService, this.platformUtilsService); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); this.devicesApiService = new DevicesApiServiceImplementation(this.apiService); @@ -659,32 +651,6 @@ export default class MainBackground { this.loginEmailService = new LoginEmailService(this.stateProvider); - this.loginStrategyService = new LoginStrategyService( - this.accountService, - this.masterPasswordService, - this.cryptoService, - this.apiService, - this.tokenService, - this.appIdService, - this.platformUtilsService, - this.messagingService, - this.logService, - this.keyConnectorService, - this.environmentService, - this.stateService, - this.twoFactorService, - this.i18nService, - this.encryptService, - this.passwordStrengthService, - this.policyService, - this.deviceTrustService, - this.authRequestService, - this.userDecryptionOptionsService, - this.globalStateProvider, - this.billingAccountProfileStateService, - this.kdfConfigService, - ); - this.ssoLoginService = new SsoLoginService(this.stateProvider); this.userVerificationApiService = new UserVerificationApiService(this.apiService); @@ -1114,8 +1080,7 @@ export default class MainBackground { this.userKeyInitService.listenForActiveUserChangesToSetUserKey(); await (this.i18nService as I18nService).init(); - await (this.eventUploadService as EventUploadService).init(true); - this.twoFactorService.init(); + (this.eventUploadService as EventUploadService).init(true); if (this.popupOnlyContext) { return; diff --git a/apps/browser/src/popup/services/init.service.ts b/apps/browser/src/popup/services/init.service.ts index ee842565d7..63ce45c9b7 100644 --- a/apps/browser/src/popup/services/init.service.ts +++ b/apps/browser/src/popup/services/init.service.ts @@ -2,6 +2,7 @@ import { DOCUMENT } from "@angular/common"; import { Inject, Injectable } from "@angular/core"; import { AbstractThemingService } from "@bitwarden/angular/platform/services/theming/theming.service.abstraction"; +import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService as LogServiceAbstraction } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -15,6 +16,7 @@ export class InitService { private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private stateService: StateServiceAbstraction, + private twoFactorService: TwoFactorService, private logService: LogServiceAbstraction, private themingService: AbstractThemingService, @Inject(DOCUMENT) private document: Document, @@ -24,6 +26,7 @@ export class InitService { return async () => { await this.stateService.init({ runMigrations: false }); // Browser background is responsible for migrations await this.i18nService.init(); + this.twoFactorService.init(); if (!BrowserPopupUtils.inPopup(window)) { window.document.body.classList.add("body-full"); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 38068d1849..052e341004 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -15,10 +15,7 @@ import { INTRAPROCESS_MESSAGING_SUBJECT, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; -import { - AuthRequestServiceAbstraction, - LoginStrategyServiceAbstraction, -} from "@bitwarden/auth/common"; +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { SearchService as SearchServiceAbstraction } from "@bitwarden/common/abstractions/search.service"; @@ -33,7 +30,6 @@ import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/d import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; -import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { @@ -168,21 +164,11 @@ const safeProviders: SafeProvider[] = [ useClass: UnauthGuardService, deps: [AuthServiceAbstraction, Router], }), - safeProvider({ - provide: TwoFactorService, - useFactory: getBgService("twoFactorService"), - deps: [], - }), safeProvider({ provide: AuthServiceAbstraction, useFactory: getBgService("authService"), deps: [], }), - safeProvider({ - provide: LoginStrategyServiceAbstraction, - useFactory: getBgService("loginStrategyService"), - deps: [], - }), safeProvider({ provide: SsoLoginServiceAbstraction, useFactory: getBgService("ssoLoginService"), diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts index 3606285c72..bd61727a6c 100644 --- a/apps/cli/src/auth/commands/login.command.ts +++ b/apps/cli/src/auth/commands/login.command.ts @@ -231,7 +231,7 @@ export class LoginCommand { } } if (response.requiresTwoFactor) { - const twoFactorProviders = this.twoFactorService.getSupportedProviders(null); + const twoFactorProviders = await this.twoFactorService.getSupportedProviders(null); if (twoFactorProviders.length === 0) { return Response.badRequest("No providers available for this client."); } @@ -272,7 +272,7 @@ export class LoginCommand { if ( twoFactorToken == null && - response.twoFactorProviders.size > 1 && + Object.keys(response.twoFactorProviders).length > 1 && selectedProvider.type === TwoFactorProviderType.Email ) { const emailReq = new TwoFactorEmailRequest(); diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index ffe6c128b5..45c394e912 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -455,7 +455,11 @@ export class Main { this.stateProvider, ); - this.twoFactorService = new TwoFactorService(this.i18nService, this.platformUtilsService); + this.twoFactorService = new TwoFactorService( + this.i18nService, + this.platformUtilsService, + this.globalStateProvider, + ); this.passwordStrengthService = new PasswordStrengthService(); diff --git a/libs/angular/src/auth/components/sso.component.spec.ts b/libs/angular/src/auth/components/sso.component.spec.ts index 269ec51e30..ae644028f9 100644 --- a/libs/angular/src/auth/components/sso.component.spec.ts +++ b/libs/angular/src/auth/components/sso.component.spec.ts @@ -253,7 +253,7 @@ describe("SsoComponent", () => { describe("2FA scenarios", () => { beforeEach(() => { const authResult = new AuthResult(); - authResult.twoFactorProviders = new Map([[TwoFactorProviderType.Authenticator, {}]]); + authResult.twoFactorProviders = { [TwoFactorProviderType.Authenticator]: {} }; // use standard user with MP because this test is not concerned with password reset. selectedUserDecryptionOptions.next(mockUserDecryptionOpts.withMasterPassword); diff --git a/libs/angular/src/auth/components/two-factor-options.component.ts b/libs/angular/src/auth/components/two-factor-options.component.ts index 2808e41cc2..1bbf81fa34 100644 --- a/libs/angular/src/auth/components/two-factor-options.component.ts +++ b/libs/angular/src/auth/components/two-factor-options.component.ts @@ -2,7 +2,10 @@ import { Directive, EventEmitter, OnInit, Output } from "@angular/core"; import { Router } from "@angular/router"; import { firstValueFrom } from "rxjs"; -import { TwoFactorService } from "@bitwarden/common/auth/abstractions/two-factor.service"; +import { + TwoFactorProviderDetails, + TwoFactorService, +} from "@bitwarden/common/auth/abstractions/two-factor.service"; import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -24,11 +27,11 @@ export class TwoFactorOptionsComponent implements OnInit { protected environmentService: EnvironmentService, ) {} - ngOnInit() { - this.providers = this.twoFactorService.getSupportedProviders(this.win); + async ngOnInit() { + this.providers = await this.twoFactorService.getSupportedProviders(this.win); } - choose(p: any) { + async choose(p: TwoFactorProviderDetails) { this.onProviderSelected.emit(p.type); } diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index f73f0483be..8e96c48ba0 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -102,7 +102,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI } async ngOnInit() { - if (!(await this.authing()) || this.twoFactorService.getProviders() == null) { + if (!(await this.authing()) || (await this.twoFactorService.getProviders()) == null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.router.navigate([this.loginRoute]); @@ -145,7 +145,9 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI ); } - this.selectedProviderType = this.twoFactorService.getDefaultProvider(this.webAuthnSupported); + this.selectedProviderType = await this.twoFactorService.getDefaultProvider( + this.webAuthnSupported, + ); await this.init(); } @@ -162,12 +164,14 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI this.cleanupWebAuthn(); this.title = (TwoFactorProviders as any)[this.selectedProviderType].name; - const providerData = this.twoFactorService.getProviders().get(this.selectedProviderType); + const providerData = await this.twoFactorService.getProviders().then((providers) => { + return providers.get(this.selectedProviderType); + }); switch (this.selectedProviderType) { case TwoFactorProviderType.WebAuthn: if (!this.webAuthnNewTab) { - setTimeout(() => { - this.authWebAuthn(); + setTimeout(async () => { + await this.authWebAuthn(); }, 500); } break; @@ -212,7 +216,7 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI break; case TwoFactorProviderType.Email: this.twoFactorEmail = providerData.Email; - if (this.twoFactorService.getProviders().size > 1) { + if ((await this.twoFactorService.getProviders()).size > 1) { await this.sendEmail(false); } break; @@ -474,8 +478,10 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI this.emailPromise = null; } - authWebAuthn() { - const providerData = this.twoFactorService.getProviders().get(this.selectedProviderType); + async authWebAuthn() { + const providerData = await this.twoFactorService.getProviders().then((providers) => { + return providers.get(this.selectedProviderType); + }); if (!this.webAuthnSupported || this.webAuthn == null) { return; diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 88494a1cbb..c7b27a25c2 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -874,7 +874,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: TwoFactorServiceAbstraction, useClass: TwoFactorService, - deps: [I18nServiceAbstraction, PlatformUtilsServiceAbstraction], + deps: [I18nServiceAbstraction, PlatformUtilsServiceAbstraction, GlobalStateProvider], }), safeProvider({ provide: FormValidationErrorsServiceAbstraction, diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index 5e70c348f4..a123e30053 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -86,7 +86,9 @@ describe("AuthRequestLoginStrategy", () => { tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); - tokenService.decodeAccessToken.mockResolvedValue({}); + tokenService.decodeAccessToken.mockResolvedValue({ + sub: mockUserId, + }); authRequestLoginStrategy = new AuthRequestLoginStrategy( cache, diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 7c022db23b..3284f6e947 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -25,11 +25,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { - Account, - AccountProfile, - AccountKeys, -} from "@bitwarden/common/platform/models/domain/account"; +import { Account, AccountProfile } from "@bitwarden/common/platform/models/domain/account"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; @@ -214,7 +210,6 @@ describe("LoginStrategy", () => { email: email, }, }, - keys: new AccountKeys(), }), ); expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( @@ -223,6 +218,21 @@ describe("LoginStrategy", () => { expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); }); + it("throws if active account isn't found after being initialized", async () => { + const idTokenResponse = identityTokenResponseFactory(); + apiService.postIdentityToken.mockResolvedValue(idTokenResponse); + + const mockVaultTimeoutAction = VaultTimeoutAction.Lock; + const mockVaultTimeout = 1000; + + stateService.getVaultTimeoutAction.mockResolvedValue(mockVaultTimeoutAction); + stateService.getVaultTimeout.mockResolvedValue(mockVaultTimeout); + + accountService.activeAccountSubject.next(null); + + await expect(async () => await passwordLoginStrategy.logIn(credentials)).rejects.toThrow(); + }); + it("builds AuthResult", async () => { const tokenResponse = identityTokenResponseFactory(); tokenResponse.forcePasswordReset = true; @@ -306,8 +316,10 @@ describe("LoginStrategy", () => { expect(tokenService.clearTwoFactorToken).toHaveBeenCalled(); const expected = new AuthResult(); - expected.twoFactorProviders = new Map(); - expected.twoFactorProviders.set(0, null); + expected.twoFactorProviders = { 0: null } as Record< + TwoFactorProviderType, + Record + >; expect(result).toEqual(expected); }); @@ -336,8 +348,9 @@ describe("LoginStrategy", () => { expect(messagingService.send).not.toHaveBeenCalled(); const expected = new AuthResult(); - expected.twoFactorProviders = new Map(); - expected.twoFactorProviders.set(1, { Email: "k***@bitwarden.com" }); + expected.twoFactorProviders = { + [TwoFactorProviderType.Email]: { Email: "k***@bitwarden.com" }, + }; expected.email = userEmail; expected.ssoEmail2FaSessionToken = ssoEmail2FaSessionToken; diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 06fc98db13..fd268d955e 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, filter, firstValueFrom, timeout } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -101,7 +101,7 @@ export abstract class LoginStrategy { } protected async startLogIn(): Promise<[AuthResult, IdentityResponse]> { - this.twoFactorService.clearSelectedProvider(); + await this.twoFactorService.clearSelectedProvider(); const tokenRequest = this.cache.value.tokenRequest; const response = await this.apiService.postIdentityToken(tokenRequest); @@ -159,12 +159,12 @@ export abstract class LoginStrategy { * It also sets the access token and refresh token in the token service. * * @param {IdentityTokenResponse} tokenResponse - The response from the server containing the identity token. - * @returns {Promise} - A promise that resolves when the account information has been successfully saved. + * @returns {Promise} - A promise that resolves the the UserId when the account information has been successfully saved. */ protected async saveAccountInformation(tokenResponse: IdentityTokenResponse): Promise { const accountInformation = await this.tokenService.decodeAccessToken(tokenResponse.accessToken); - const userId = accountInformation.sub; + const userId = accountInformation.sub as UserId; const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId }); const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); @@ -191,6 +191,8 @@ export abstract class LoginStrategy { }), ); + await this.verifyAccountAdded(userId); + await this.userDecryptionOptionsService.setUserDecryptionOptions( UserDecryptionOptions.fromResponse(tokenResponse), ); @@ -207,7 +209,7 @@ export abstract class LoginStrategy { ); await this.billingAccountProfileStateService.setHasPremium(accountInformation.premium, false); - return userId as UserId; + return userId; } protected async processTokenResponse(response: IdentityTokenResponse): Promise { @@ -284,7 +286,7 @@ export abstract class LoginStrategy { const result = new AuthResult(); result.twoFactorProviders = response.twoFactorProviders2; - this.twoFactorService.setProviders(response); + await this.twoFactorService.setProviders(response); this.cache.next({ ...this.cache.value, captchaBypassToken: response.captchaToken ?? null }); result.ssoEmail2FaSessionToken = response.ssoEmail2faSessionToken; result.email = response.email; @@ -306,4 +308,24 @@ export abstract class LoginStrategy { result.captchaSiteKey = response.siteKey; return result; } + + /** + * Verifies that the active account is set after initialization. + * Note: In browser there is a slight delay between when active account emits in background, + * and when it emits in foreground. We're giving the foreground 1 second to catch up. + * If nothing is emitted, we throw an error. + */ + private async verifyAccountAdded(expectedUserId: UserId) { + await firstValueFrom( + this.accountService.activeAccount$.pipe( + filter((account) => account?.id === expectedUserId), + timeout({ + first: 1000, + with: () => { + throw new Error("Expected user never made active user after initialization."); + }, + }), + ), + ); + } } diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index be09448fdd..5c1fe9b1fe 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -99,7 +99,9 @@ describe("PasswordLoginStrategy", () => { kdfConfigService = mock(); appIdService.getAppId.mockResolvedValue(deviceId); - tokenService.decodeAccessToken.mockResolvedValue({}); + tokenService.decodeAccessToken.mockResolvedValue({ + sub: userId, + }); loginStrategyService.makePreloginKey.mockResolvedValue(masterKey); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 3439a1c199..416e910b47 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -92,7 +92,9 @@ describe("SsoLoginStrategy", () => { tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); - tokenService.decodeAccessToken.mockResolvedValue({}); + tokenService.decodeAccessToken.mockResolvedValue({ + sub: userId, + }); ssoLoginStrategy = new SsoLoginStrategy( null, diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts index 5fce8b0b82..130e6c2d89 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts @@ -82,7 +82,9 @@ describe("UserApiLoginStrategy", () => { appIdService.getAppId.mockResolvedValue(deviceId); tokenService.getTwoFactorToken.mockResolvedValue(null); - tokenService.decodeAccessToken.mockResolvedValue({}); + tokenService.decodeAccessToken.mockResolvedValue({ + sub: userId, + }); apiLogInStrategy = new UserApiLoginStrategy( cache, diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index d75e194980..bbcd3bafdd 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -18,7 +18,8 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { FakeAccountService } from "@bitwarden/common/spec"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; import { PrfKey, UserKey } from "@bitwarden/common/types/key"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -49,6 +50,7 @@ describe("WebAuthnLoginStrategy", () => { const token = "mockToken"; const deviceId = Utils.newGuid(); + const userId = Utils.newGuid() as UserId; let webAuthnCredentials!: WebAuthnLoginCredentials; @@ -69,7 +71,7 @@ describe("WebAuthnLoginStrategy", () => { beforeEach(() => { jest.clearAllMocks(); - accountService = new FakeAccountService(null); + accountService = mockAccountServiceWith(userId); masterPasswordService = new FakeMasterPasswordService(); cryptoService = mock(); @@ -87,7 +89,9 @@ describe("WebAuthnLoginStrategy", () => { tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); - tokenService.decodeAccessToken.mockResolvedValue({}); + tokenService.decodeAccessToken.mockResolvedValue({ + sub: userId, + }); webAuthnLoginStrategy = new WebAuthnLoginStrategy( cache, diff --git a/libs/common/src/auth/abstractions/two-factor.service.ts b/libs/common/src/auth/abstractions/two-factor.service.ts index 3ea7eb8db9..a0a9ecd2af 100644 --- a/libs/common/src/auth/abstractions/two-factor.service.ts +++ b/libs/common/src/auth/abstractions/two-factor.service.ts @@ -12,12 +12,12 @@ export interface TwoFactorProviderDetails { export abstract class TwoFactorService { init: () => void; - getSupportedProviders: (win: Window) => TwoFactorProviderDetails[]; - getDefaultProvider: (webAuthnSupported: boolean) => TwoFactorProviderType; - setSelectedProvider: (type: TwoFactorProviderType) => void; - clearSelectedProvider: () => void; + getSupportedProviders: (win: Window) => Promise; + getDefaultProvider: (webAuthnSupported: boolean) => Promise; + setSelectedProvider: (type: TwoFactorProviderType) => Promise; + clearSelectedProvider: () => Promise; - setProviders: (response: IdentityTwoFactorResponse) => void; - clearProviders: () => void; - getProviders: () => Map; + setProviders: (response: IdentityTwoFactorResponse) => Promise; + clearProviders: () => Promise; + getProviders: () => Promise>; } diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index 993ce08d58..f45466777e 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -14,7 +14,7 @@ export class AuthResult { resetMasterPassword = false; forcePasswordReset: ForceSetPasswordReason = ForceSetPasswordReason.None; - twoFactorProviders: Map = null; + twoFactorProviders: Partial>> = null; ssoEmail2FaSessionToken?: string; email: string; requiresEncryptionKeyMigration: boolean; diff --git a/libs/common/src/auth/models/response/identity-two-factor.response.ts b/libs/common/src/auth/models/response/identity-two-factor.response.ts index bc5d2fbf85..dce64e8ef3 100644 --- a/libs/common/src/auth/models/response/identity-two-factor.response.ts +++ b/libs/common/src/auth/models/response/identity-two-factor.response.ts @@ -4,8 +4,10 @@ import { TwoFactorProviderType } from "../../enums/two-factor-provider-type"; import { MasterPasswordPolicyResponse } from "./master-password-policy.response"; export class IdentityTwoFactorResponse extends BaseResponse { + // contains available two-factor providers twoFactorProviders: TwoFactorProviderType[]; - twoFactorProviders2 = new Map(); + // a map of two-factor providers to necessary data for completion + twoFactorProviders2: Record>; captchaToken: string; ssoEmail2faSessionToken: string; email?: string; @@ -15,15 +17,7 @@ export class IdentityTwoFactorResponse extends BaseResponse { super(response); this.captchaToken = this.getResponseProperty("CaptchaBypassToken"); this.twoFactorProviders = this.getResponseProperty("TwoFactorProviders"); - const twoFactorProviders2 = this.getResponseProperty("TwoFactorProviders2"); - if (twoFactorProviders2 != null) { - for (const prop in twoFactorProviders2) { - // eslint-disable-next-line - if (twoFactorProviders2.hasOwnProperty(prop)) { - this.twoFactorProviders2.set(parseInt(prop, null), twoFactorProviders2[prop]); - } - } - } + this.twoFactorProviders2 = this.getResponseProperty("TwoFactorProviders2"); this.masterPasswordPolicy = new MasterPasswordPolicyResponse( this.getResponseProperty("MasterPasswordPolicy"), ); diff --git a/libs/common/src/auth/services/two-factor.service.ts b/libs/common/src/auth/services/two-factor.service.ts index cd1e5ea122..50d2556157 100644 --- a/libs/common/src/auth/services/two-factor.service.ts +++ b/libs/common/src/auth/services/two-factor.service.ts @@ -1,5 +1,9 @@ +import { firstValueFrom, map } from "rxjs"; + import { I18nService } from "../../platform/abstractions/i18n.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; +import { Utils } from "../../platform/misc/utils"; +import { GlobalStateProvider, KeyDefinition, TWO_FACTOR_MEMORY } from "../../platform/state"; import { TwoFactorProviderDetails, TwoFactorService as TwoFactorServiceAbstraction, @@ -59,13 +63,36 @@ export const TwoFactorProviders: Partial, TwoFactorProviderType>( + TWO_FACTOR_MEMORY, + "providers", + { + deserializer: (obj) => obj, + }, +); + +// Memory storage as only required during authentication process +export const SELECTED_PROVIDER = new KeyDefinition( + TWO_FACTOR_MEMORY, + "selected", + { + deserializer: (obj) => obj, + }, +); + export class TwoFactorService implements TwoFactorServiceAbstraction { - private twoFactorProvidersData: Map; - private selectedTwoFactorProviderType: TwoFactorProviderType = null; + private providersState = this.globalStateProvider.get(PROVIDERS); + private selectedState = this.globalStateProvider.get(SELECTED_PROVIDER); + readonly providers$ = this.providersState.state$.pipe( + map((providers) => Utils.recordToMap(providers)), + ); + readonly selected$ = this.selectedState.state$; constructor( private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, + private globalStateProvider: GlobalStateProvider, ) {} init() { @@ -93,63 +120,60 @@ export class TwoFactorService implements TwoFactorServiceAbstraction { this.i18nService.t("yubiKeyDesc"); } - getSupportedProviders(win: Window): TwoFactorProviderDetails[] { + async getSupportedProviders(win: Window): Promise { + const data = await firstValueFrom(this.providers$); const providers: any[] = []; - if (this.twoFactorProvidersData == null) { + if (data == null) { return providers; } if ( - this.twoFactorProvidersData.has(TwoFactorProviderType.OrganizationDuo) && + data.has(TwoFactorProviderType.OrganizationDuo) && this.platformUtilsService.supportsDuo() ) { providers.push(TwoFactorProviders[TwoFactorProviderType.OrganizationDuo]); } - if (this.twoFactorProvidersData.has(TwoFactorProviderType.Authenticator)) { + if (data.has(TwoFactorProviderType.Authenticator)) { providers.push(TwoFactorProviders[TwoFactorProviderType.Authenticator]); } - if (this.twoFactorProvidersData.has(TwoFactorProviderType.Yubikey)) { + if (data.has(TwoFactorProviderType.Yubikey)) { providers.push(TwoFactorProviders[TwoFactorProviderType.Yubikey]); } - if ( - this.twoFactorProvidersData.has(TwoFactorProviderType.Duo) && - this.platformUtilsService.supportsDuo() - ) { + if (data.has(TwoFactorProviderType.Duo) && this.platformUtilsService.supportsDuo()) { providers.push(TwoFactorProviders[TwoFactorProviderType.Duo]); } if ( - this.twoFactorProvidersData.has(TwoFactorProviderType.WebAuthn) && + data.has(TwoFactorProviderType.WebAuthn) && this.platformUtilsService.supportsWebAuthn(win) ) { providers.push(TwoFactorProviders[TwoFactorProviderType.WebAuthn]); } - if (this.twoFactorProvidersData.has(TwoFactorProviderType.Email)) { + if (data.has(TwoFactorProviderType.Email)) { providers.push(TwoFactorProviders[TwoFactorProviderType.Email]); } return providers; } - getDefaultProvider(webAuthnSupported: boolean): TwoFactorProviderType { - if (this.twoFactorProvidersData == null) { + async getDefaultProvider(webAuthnSupported: boolean): Promise { + const data = await firstValueFrom(this.providers$); + const selected = await firstValueFrom(this.selected$); + if (data == null) { return null; } - if ( - this.selectedTwoFactorProviderType != null && - this.twoFactorProvidersData.has(this.selectedTwoFactorProviderType) - ) { - return this.selectedTwoFactorProviderType; + if (selected != null && data.has(selected)) { + return selected; } let providerType: TwoFactorProviderType = null; let providerPriority = -1; - this.twoFactorProvidersData.forEach((_value, type) => { + data.forEach((_value, type) => { const provider = (TwoFactorProviders as any)[type]; if (provider != null && provider.priority > providerPriority) { if (type === TwoFactorProviderType.WebAuthn && !webAuthnSupported) { @@ -164,23 +188,23 @@ export class TwoFactorService implements TwoFactorServiceAbstraction { return providerType; } - setSelectedProvider(type: TwoFactorProviderType) { - this.selectedTwoFactorProviderType = type; + async setSelectedProvider(type: TwoFactorProviderType): Promise { + await this.selectedState.update(() => type); } - clearSelectedProvider() { - this.selectedTwoFactorProviderType = null; + async clearSelectedProvider(): Promise { + await this.selectedState.update(() => null); } - setProviders(response: IdentityTwoFactorResponse) { - this.twoFactorProvidersData = response.twoFactorProviders2; + async setProviders(response: IdentityTwoFactorResponse): Promise { + await this.providersState.update(() => response.twoFactorProviders2); } - clearProviders() { - this.twoFactorProvidersData = null; + async clearProviders(): Promise { + await this.providersState.update(() => null); } - getProviders() { - return this.twoFactorProvidersData; + getProviders(): Promise> { + return firstValueFrom(this.providers$); } } diff --git a/libs/common/src/platform/state/deserialization-helpers.spec.ts b/libs/common/src/platform/state/deserialization-helpers.spec.ts new file mode 100644 index 0000000000..b1ae447997 --- /dev/null +++ b/libs/common/src/platform/state/deserialization-helpers.spec.ts @@ -0,0 +1,25 @@ +import { record } from "./deserialization-helpers"; + +describe("deserialization helpers", () => { + describe("record", () => { + it("deserializes a record when keys are strings", () => { + const deserializer = record((value: number) => value); + const input = { + a: 1, + b: 2, + }; + const output = deserializer(input); + expect(output).toEqual(input); + }); + + it("deserializes a record when keys are numbers", () => { + const deserializer = record((value: number) => value); + const input = { + 1: 1, + 2: 2, + }; + const output = deserializer(input); + expect(output).toEqual(input); + }); + }); +}); diff --git a/libs/common/src/platform/state/deserialization-helpers.ts b/libs/common/src/platform/state/deserialization-helpers.ts index d68a3d0444..8fd5d2da19 100644 --- a/libs/common/src/platform/state/deserialization-helpers.ts +++ b/libs/common/src/platform/state/deserialization-helpers.ts @@ -21,7 +21,7 @@ export function array( * * @param valueDeserializer */ -export function record( +export function record( valueDeserializer: (value: Jsonify) => T, ): (record: Jsonify>) => Record { return (jsonValue: Jsonify | null>) => { @@ -29,10 +29,10 @@ export function record( return null; } - const output: Record = {}; - for (const key in jsonValue) { - output[key] = valueDeserializer((jsonValue as Record>)[key]); - } + const output: Record = {} as any; + Object.entries(jsonValue).forEach(([key, value]) => { + output[key as TKey] = valueDeserializer(value); + }); return output; }; } diff --git a/libs/common/src/platform/state/key-definition.ts b/libs/common/src/platform/state/key-definition.ts index b2a8ff8712..bdabd8df50 100644 --- a/libs/common/src/platform/state/key-definition.ts +++ b/libs/common/src/platform/state/key-definition.ts @@ -113,7 +113,7 @@ export class KeyDefinition { * }); * ``` */ - static record( + static record( stateDefinition: StateDefinition, key: string, // We have them provide options for the value of the record, depending on future options we add, this could get a little weird. diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index 8847f7fe51..ee5005202f 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -40,6 +40,7 @@ export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); +export const TWO_FACTOR_MEMORY = new StateDefinition("twoFactor", "memory"); export const AVATAR_DISK = new StateDefinition("avatar", "disk", { web: "disk-local" }); export const ROUTER_DISK = new StateDefinition("router", "disk"); export const LOGIN_EMAIL_DISK = new StateDefinition("loginEmail", "disk", { diff --git a/libs/common/src/platform/state/user-key-definition.ts b/libs/common/src/platform/state/user-key-definition.ts index 3eb9369080..4c622e29f1 100644 --- a/libs/common/src/platform/state/user-key-definition.ts +++ b/libs/common/src/platform/state/user-key-definition.ts @@ -120,7 +120,7 @@ export class UserKeyDefinition { * }); * ``` */ - static record( + static record( stateDefinition: StateDefinition, key: string, // We have them provide options for the value of the record, depending on future options we add, this could get a little weird.