diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts index 78bee121afb5..b5d8b6002cb9 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts @@ -4,7 +4,7 @@ import { Router } from "@angular/router"; import { Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { LockService } from "@bitwarden/auth/common"; +import { LockService, LogoutService } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -70,6 +70,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private authService: AuthService, private lockService: LockService, + private logoutService: LogoutService, ) {} get accountLimit() { @@ -141,12 +142,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { }); if (confirmed) { - const result = await this.accountSwitcherService.logoutAccount(userId); - // unlocked logout responses need to be navigated out of the account switcher. - // other responses will be handled by background and app.component - if (result?.status === AuthenticationStatus.Unlocked) { - this.location.back(); - } + await this.logoutService.logout(userId); + // navigate to root so redirect guard can properly route next active user or null user to correct page + await this.router.navigate(["/"]); } this.loading = false; } diff --git a/apps/browser/src/auth/popup/account-switching/account.component.ts b/apps/browser/src/auth/popup/account-switching/account.component.ts index dad74977d340..2a44451f49eb 100644 --- a/apps/browser/src/auth/popup/account-switching/account.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account.component.ts @@ -1,7 +1,8 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { CommonModule, Location } from "@angular/common"; +import { CommonModule } from "@angular/common"; import { Component, EventEmitter, Input, Output } from "@angular/core"; +import { Router } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -24,7 +25,7 @@ export class AccountComponent { constructor( private accountSwitcherService: AccountSwitcherService, - private location: Location, + private router: Router, private i18nService: I18nService, private logService: LogService, private biometricsService: BiometricsService, @@ -45,8 +46,8 @@ export class AccountComponent { // Navigate out of account switching for unlocked accounts // locked or logged out account statuses are handled by background and app.component - if (result?.status === AuthenticationStatus.Unlocked) { - this.location.back(); + if (result?.authenticationStatus === AuthenticationStatus.Unlocked) { + await this.router.navigate(["vault"]); await this.biometricsService.setShouldAutopromptNow(false); } else { await this.biometricsService.setShouldAutopromptNow(true); diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index 1fdd0b1ecf2a..4bacd4538030 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -207,35 +207,4 @@ describe("AccountSwitcherService", () => { expect(removeListenerSpy).toBeCalledTimes(1); }); }); - - describe("logout", () => { - const userId1 = "1" as UserId; - const userId2 = "2" as UserId; - it("initiates logout", async () => { - let listener: ( - message: { command: string; userId: UserId; status: AuthenticationStatus }, - sender: unknown, - sendResponse: unknown, - ) => void; - jest.spyOn(chrome.runtime.onMessage, "addListener").mockImplementation((addedListener) => { - listener = addedListener; - }); - - const removeListenerSpy = jest.spyOn(chrome.runtime.onMessage, "removeListener"); - - const logoutPromise = accountSwitcherService.logoutAccount(userId1); - - listener( - { command: "switchAccountFinish", userId: userId2, status: AuthenticationStatus.Unlocked }, - undefined, - undefined, - ); - - const result = await logoutPromise; - - expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: userId1 }); - expect(result).toEqual({ newUserId: userId2, status: AuthenticationStatus.Unlocked }); - expect(removeListenerSpy).toBeCalledTimes(1); - }); - }); }); diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts index bfed7dc14080..7bb12fc260d2 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -12,6 +12,7 @@ import { timeout, } from "rxjs"; +import { NewActiveUser } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; @@ -43,7 +44,7 @@ export class AccountSwitcherService { SPECIAL_ADD_ACCOUNT_ID = "addAccount"; availableAccounts$: Observable; - switchAccountFinished$: Observable<{ userId: UserId; status: AuthenticationStatus }>; + switchAccountFinished$: Observable; constructor( private accountService: AccountService, @@ -118,7 +119,7 @@ export class AccountSwitcherService { [message: { command: string; userId: UserId; status: AuthenticationStatus }] >(chrome.runtime.onMessage).pipe( filter(([message]) => message.command === "switchAccountFinish"), - map(([message]) => ({ userId: message.userId, status: message.status })), + map(([message]) => ({ userId: message.userId, authenticationStatus: message.status })), ); } @@ -143,29 +144,9 @@ export class AccountSwitcherService { return await switchAccountFinishedPromise; } - /** - * - * @param userId the user id to logout - * @returns the userId and status of the that has been switch to due to the logout. null on errors. - */ - async logoutAccount( - userId: UserId, - ): Promise<{ newUserId: UserId; status: AuthenticationStatus } | null> { - // logout creates an account switch to the next up user, which may be null - const switchPromise = this.listenForSwitchAccountFinish(null); - - await this.messagingService.send("logout", { userId }); - - // wait for account switch to happen, the result will be the new user id and status - const result = await switchPromise; - return { newUserId: result.userId, status: result.status }; - } - // Listens for the switchAccountFinish message and returns the userId from the message // Optionally filters switchAccountFinish to an expected userId - private listenForSwitchAccountFinish( - expectedUserId: UserId | null, - ): Promise<{ userId: UserId; status: AuthenticationStatus } | null> { + listenForSwitchAccountFinish(expectedUserId: UserId | null): Promise { return firstValueFrom( this.switchAccountFinished$.pipe( filter(({ userId }) => (expectedUserId ? userId === expectedUserId : true)), diff --git a/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.spec.ts b/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.spec.ts deleted file mode 100644 index 8f3199cdfcef..000000000000 --- a/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.spec.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { Router } from "@angular/router"; -import { MockProxy, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; - -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; - -import { postLogoutMessageListener$ } from "../utils/post-logout-message-listener"; - -import { ExtensionLoginDecryptionOptionsService } from "./extension-login-decryption-options.service"; - -// Mock the module providing postLogoutMessageListener$ -jest.mock("../utils/post-logout-message-listener", () => { - return { - postLogoutMessageListener$: new BehaviorSubject(""), // Replace with mock subject - }; -}); - -describe("ExtensionLoginDecryptionOptionsService", () => { - let service: ExtensionLoginDecryptionOptionsService; - - let messagingService: MockProxy; - let router: MockProxy; - let postLogoutMessageSubject: BehaviorSubject; - - beforeEach(() => { - messagingService = mock(); - router = mock(); - - // Cast postLogoutMessageListener$ to BehaviorSubject for dynamic control - postLogoutMessageSubject = postLogoutMessageListener$ as BehaviorSubject; - - service = new ExtensionLoginDecryptionOptionsService(messagingService, router); - }); - - it("should instantiate the service", () => { - expect(service).not.toBeFalsy(); - }); - - describe("logOut()", () => { - it("should send a logout message", async () => { - postLogoutMessageSubject.next("switchAccountFinish"); - - await service.logOut(); - - expect(messagingService.send).toHaveBeenCalledWith("logout"); - }); - - it("should navigate to root on 'switchAccountFinish'", async () => { - postLogoutMessageSubject.next("switchAccountFinish"); - - await service.logOut(); - - expect(router.navigate).toHaveBeenCalledWith(["/"]); - }); - - it("should not navigate for 'doneLoggingOut'", async () => { - postLogoutMessageSubject.next("doneLoggingOut"); - - await service.logOut(); - - expect(router.navigate).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.ts b/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.ts deleted file mode 100644 index ea529e277e6f..000000000000 --- a/apps/browser/src/auth/popup/login-decryption-options/extension-login-decryption-options.service.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { Router } from "@angular/router"; -import { firstValueFrom } from "rxjs"; - -import { - DefaultLoginDecryptionOptionsService, - LoginDecryptionOptionsService, -} from "@bitwarden/auth/angular"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; - -import { postLogoutMessageListener$ } from "../utils/post-logout-message-listener"; - -export class ExtensionLoginDecryptionOptionsService - extends DefaultLoginDecryptionOptionsService - implements LoginDecryptionOptionsService -{ - constructor( - protected messagingService: MessagingService, - private router: Router, - ) { - super(messagingService); - } - - override async logOut(): Promise { - // start listening for "switchAccountFinish" or "doneLoggingOut" - const messagePromise = firstValueFrom(postLogoutMessageListener$); - - super.logOut(); - - // wait for messages - const command = await messagePromise; - - // doneLoggingOut already has a message handler that will navigate us - if (command === "switchAccountFinish") { - await this.router.navigate(["/"]); - } - } -} diff --git a/apps/browser/src/auth/popup/logout/extension-logout.service.spec.ts b/apps/browser/src/auth/popup/logout/extension-logout.service.spec.ts new file mode 100644 index 000000000000..7ab7742c1c3d --- /dev/null +++ b/apps/browser/src/auth/popup/logout/extension-logout.service.spec.ts @@ -0,0 +1,101 @@ +import { MockProxy, mock } from "jest-mock-extended"; + +import { LogoutReason, LogoutService } from "@bitwarden/auth/common"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { AccountSwitcherService } from "../account-switching/services/account-switcher.service"; + +import { ExtensionLogoutService } from "./extension-logout.service"; + +describe("ExtensionLogoutService", () => { + let logoutService: LogoutService; + let messagingService: MockProxy; + let accountSwitcherService: MockProxy; + + let primaryUserId: UserId; + let secondaryUserId: UserId; + let logoutReason: LogoutReason; + + beforeEach(() => { + primaryUserId = "1" as UserId; + secondaryUserId = "2" as UserId; + logoutReason = "vaultTimeout"; + + messagingService = mock(); + accountSwitcherService = mock(); + logoutService = new ExtensionLogoutService(messagingService, accountSwitcherService); + }); + + it("instantiates", () => { + expect(logoutService).not.toBeFalsy(); + }); + + describe("logout", () => { + describe("No new active user", () => { + beforeEach(() => { + accountSwitcherService.listenForSwitchAccountFinish.mockResolvedValue(null); + }); + + it("sends logout message without a logout reason when not provided", async () => { + const result = await logoutService.logout(primaryUserId); + + expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: primaryUserId }); + + expect(result).toBeUndefined(); + }); + + it("sends logout message with a logout reason when provided", async () => { + const result = await logoutService.logout(primaryUserId, logoutReason); + + expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("logout", { + userId: primaryUserId, + logoutReason, + }); + expect(result).toBeUndefined(); + }); + }); + + describe("New active user", () => { + const newActiveUserAuthenticationStatus = AuthenticationStatus.Unlocked; + + beforeEach(() => { + accountSwitcherService.listenForSwitchAccountFinish.mockResolvedValue({ + userId: secondaryUserId, + authenticationStatus: newActiveUserAuthenticationStatus, + }); + }); + + it("sends logout message without a logout reason when not provided and returns the new active user", async () => { + const result = await logoutService.logout(primaryUserId); + + expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1); + + expect(messagingService.send).toHaveBeenCalledWith("logout", { userId: primaryUserId }); + + expect(result).toEqual({ + userId: secondaryUserId, + authenticationStatus: newActiveUserAuthenticationStatus, + }); + }); + + it("sends logout message with a logout reason when provided and returns the new active user", async () => { + const result = await logoutService.logout(primaryUserId, logoutReason); + + expect(accountSwitcherService.listenForSwitchAccountFinish).toHaveBeenCalledTimes(1); + + expect(messagingService.send).toHaveBeenCalledWith("logout", { + userId: primaryUserId, + logoutReason, + }); + expect(result).toEqual({ + userId: secondaryUserId, + authenticationStatus: newActiveUserAuthenticationStatus, + }); + }); + }); + }); +}); diff --git a/apps/browser/src/auth/popup/logout/extension-logout.service.ts b/apps/browser/src/auth/popup/logout/extension-logout.service.ts new file mode 100644 index 000000000000..c43c18f157af --- /dev/null +++ b/apps/browser/src/auth/popup/logout/extension-logout.service.ts @@ -0,0 +1,39 @@ +import { + DefaultLogoutService, + LogoutReason, + LogoutService, + NewActiveUser, +} from "@bitwarden/auth/common"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { AccountSwitcherService } from "../account-switching/services/account-switcher.service"; + +export class ExtensionLogoutService extends DefaultLogoutService implements LogoutService { + constructor( + protected messagingService: MessagingService, + private accountSwitcherService: AccountSwitcherService, + ) { + super(messagingService); + } + + override async logout( + userId: UserId, + logoutReason?: LogoutReason, + ): Promise { + // logout can result in an account switch to the next up user + const accountSwitchFinishPromise = + this.accountSwitcherService.listenForSwitchAccountFinish(null); + + // send the logout message + this.messagingService.send("logout", { userId, logoutReason }); + + // wait for the account switch to finish + const result = await accountSwitchFinishPromise; + if (result) { + return { userId: result.userId, authenticationStatus: result.authenticationStatus }; + } + // if there is no account switch, return undefined + return undefined; + } +} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 5225ebc0fb14..d3f64034ad92 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1540,6 +1540,7 @@ export default class MainBackground { } } + // TODO: PM-21212 - consolidate the logic of this method into the new ExtensionLogoutService async logout(logoutReason: LogoutReason, userId?: UserId) { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe( diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 3dde9f15fdbb..187ec96473ec 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -496,7 +496,8 @@ const routes: Routes = [ canActivate: [tdeDecryptionRequiredGuard()], data: { pageIcon: DevicesIcon, - }, + showAcctSwitcher: true, + } satisfies ExtensionAnonLayoutWrapperData, children: [{ path: "", component: LoginDecryptionOptionsComponent }], }, { diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 5f7fbc1fad7f..6333d175a8d1 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -3,10 +3,10 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit, inject } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; -import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs"; +import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap, map } from "rxjs"; import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; -import { LogoutReason } from "@bitwarden/auth/common"; +import { LogoutReason, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -23,7 +23,7 @@ import { ToastOptions, ToastService, } from "@bitwarden/components"; -import { BiometricsService, BiometricStateService } from "@bitwarden/key-management"; +import { BiometricsService, BiometricStateService, KeyService } from "@bitwarden/key-management"; import { PopupCompactModeService } from "../platform/popup/layout/popup-compact-mode.service"; import { PopupSizeService } from "../platform/popup/layout/popup-size.service"; @@ -73,6 +73,8 @@ export class AppComponent implements OnInit, OnDestroy { private biometricStateService: BiometricStateService, private biometricsService: BiometricsService, private deviceTrustToastService: DeviceTrustToastService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private keyService: KeyService, private popupSizeService: PopupSizeService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -128,9 +130,24 @@ export class AppComponent implements OnInit, OnDestroy { (msg.userId == null || msg.userId == this.activeUserId) ) { await this.biometricsService.setShouldAutopromptNow(false); - // 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(["lock"]); + + // When user is locked, normally we can just send them the lock screen. + // However, for account switching scenarios, we need to consider the TDE lock state. + const tdeEnabled = await firstValueFrom( + this.userDecryptionOptionsService + .userDecryptionOptionsById$(msg.userId) + .pipe(map((decryptionOptions) => decryptionOptions?.trustedDeviceOption != null)), + ); + + const everHadUserKey = await firstValueFrom( + this.keyService.everHadUserKey$(msg.userId), + ); + if (tdeEnabled && !everHadUserKey) { + await this.router.navigate(["login-initiated"]); + return; + } + + await this.router.navigate(["lock"]); } else if (msg.command === "showDialog") { // 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 diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 6ede88dfc13c..00e493fc035c 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -1,7 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { APP_INITIALIZER, NgModule, NgZone } from "@angular/core"; -import { Router } from "@angular/router"; import { merge, of, Subject } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; @@ -25,7 +24,6 @@ import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services. import { AnonLayoutWrapperDataService, LoginComponentService, - LoginDecryptionOptionsService, TwoFactorAuthComponentService, TwoFactorAuthEmailComponentService, TwoFactorAuthDuoComponentService, @@ -37,6 +35,7 @@ import { LoginEmailService, PinServiceAbstraction, SsoUrlService, + LogoutService, } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -137,11 +136,12 @@ import { SshImportPromptService, } from "@bitwarden/vault"; +import { AccountSwitcherService } from "../../auth/popup/account-switching/services/account-switcher.service"; import { ForegroundLockService } from "../../auth/popup/accounts/foreground-lock.service"; import { ExtensionAnonLayoutWrapperDataService } from "../../auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service"; import { ExtensionLoginComponentService } from "../../auth/popup/login/extension-login-component.service"; import { ExtensionSsoComponentService } from "../../auth/popup/login/extension-sso-component.service"; -import { ExtensionLoginDecryptionOptionsService } from "../../auth/popup/login-decryption-options/extension-login-decryption-options.service"; +import { ExtensionLogoutService } from "../../auth/popup/logout/extension-logout.service"; import { ExtensionTwoFactorAuthComponentService } from "../../auth/services/extension-two-factor-auth-component.service"; import { ExtensionTwoFactorAuthDuoComponentService } from "../../auth/services/extension-two-factor-auth-duo-component.service"; import { ExtensionTwoFactorAuthEmailComponentService } from "../../auth/services/extension-two-factor-auth-email-component.service"; @@ -642,6 +642,11 @@ const safeProviders: SafeProvider[] = [ useClass: ExtensionAnonLayoutWrapperDataService, deps: [], }), + safeProvider({ + provide: LogoutService, + useClass: ExtensionLogoutService, + deps: [MessagingServiceAbstraction, AccountSwitcherService], + }), safeProvider({ provide: CompactModeService, useExisting: PopupCompactModeService, @@ -652,11 +657,6 @@ const safeProviders: SafeProvider[] = [ useClass: ExtensionSsoComponentService, deps: [SyncService, AuthService, EnvironmentService, I18nServiceAbstraction, LogService], }), - safeProvider({ - provide: LoginDecryptionOptionsService, - useClass: ExtensionLoginDecryptionOptionsService, - deps: [MessagingServiceAbstraction, Router], - }), safeProvider({ provide: SshImportPromptService, useClass: DefaultSshImportPromptService, diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 77ac783ac9f7..689b9b43e456 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -27,7 +27,11 @@ import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent, LoginApprovalComponent } from "@bitwarden/auth/angular"; -import { DESKTOP_SSO_CALLBACK, LogoutReason } from "@bitwarden/auth/common"; +import { + DESKTOP_SSO_CALLBACK, + LogoutReason, + UserDecryptionOptionsServiceAbstraction, +} from "@bitwarden/auth/common"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -163,6 +167,7 @@ export class AppComponent implements OnInit, OnDestroy { private accountService: AccountService, private organizationService: OrganizationService, private deviceTrustToastService: DeviceTrustToastService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); } @@ -417,7 +422,23 @@ export class AppComponent implements OnInit, OnDestroy { AuthenticationStatus.Locked; if (locked) { this.modalService.closeAll(); - await this.router.navigate(["lock"]); + + // We only have to handle TDE lock on "switchAccount" message scenarios but not normal + // lock scenarios since the user will have always decrypted the vault at least once in those cases. + const tdeEnabled = await firstValueFrom( + this.userDecryptionOptionsService + .userDecryptionOptionsById$(message.userId) + .pipe(map((decryptionOptions) => decryptionOptions?.trustedDeviceOption != null)), + ); + + const everHadUserKey = await firstValueFrom( + this.keyService.everHadUserKey$(message.userId), + ); + if (tdeEnabled && !everHadUserKey) { + await this.router.navigate(["login-initiated"]); + } else { + await this.router.navigate(["lock"]); + } } else { this.messagingService.send("unlocked"); this.loading = true; @@ -593,6 +614,8 @@ export class AppComponent implements OnInit, OnDestroy { } } + // TODO: PM-21212 - consolidate the logic of this method into the new LogoutService + // (requires creating a desktop specific implementation of the LogoutService) // Even though the userId parameter is no longer optional doesn't mean a message couldn't be // passing null-ish values to us. private async logOut(logoutReason: LogoutReason, userId: UserId) { diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 08bcaa2165cf..a0848f80ac91 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -42,6 +42,7 @@ import { AuthRequestServiceAbstraction, DefaultAuthRequestApiService, DefaultLoginSuccessHandlerService, + DefaultLogoutService, InternalUserDecryptionOptionsServiceAbstraction, LoginApprovalComponentServiceAbstraction, LoginEmailService, @@ -50,6 +51,7 @@ import { LoginStrategyServiceAbstraction, LoginSuccessHandlerService, LogoutReason, + LogoutService, PinService, PinServiceAbstraction, UserDecryptionOptionsService, @@ -402,6 +404,7 @@ const safeProviders: SafeProvider[] = [ provide: STATE_FACTORY, useValue: new StateFactory(GlobalState, Account), }), + // TODO: PM-21212 - Deprecate LogoutCallback in favor of LogoutService safeProvider({ provide: LOGOUT_CALLBACK, useFactory: @@ -1543,6 +1546,11 @@ const safeProviders: SafeProvider[] = [ useClass: MasterPasswordApiService, deps: [ApiServiceAbstraction, LogService], }), + safeProvider({ + provide: LogoutService, + useClass: DefaultLogoutService, + deps: [MessagingServiceAbstraction], + }), safeProvider({ provide: CipherEncryptionService, useClass: DefaultCipherEncryptionService, diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts index c49e54f8c19e..57f74ea9911c 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts @@ -10,6 +10,7 @@ import { catchError, defer, firstValueFrom, from, map, of, switchMap, throwError import { JslibModule } from "@bitwarden/angular/jslib.module"; import { LoginEmailServiceAbstraction, + LogoutService, UserDecryptionOptions, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; @@ -110,6 +111,7 @@ export class LoginDecryptionOptionsComponent implements OnInit { private toastService: ToastService, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private validationService: ValidationService, + private logoutService: LogoutService, ) { this.clientType = this.platformUtilsService.getClientType(); } @@ -157,19 +159,17 @@ export class LoginDecryptionOptionsComponent implements OnInit { } private async handleMissingEmail() { + // TODO: PM-15174 - the solution for this bug will allow us to show the toast on app re-init after + // the user has been logged out and the process reload has occurred. this.toastService.showToast({ variant: "error", title: null, message: this.i18nService.t("activeUserEmailNotFoundLoggingYouOut"), }); - setTimeout(async () => { - // We can't simply redirect to `/login` because the user is authed and the unauthGuard - // will prevent navigation. We must logout the user first via messagingService, which - // redirects to `/`, which will be handled by the redirectGuard to navigate the user to `/login`. - // The timeout just gives the user a chance to see the error toast before process reload runs on logout. - await this.loginDecryptionOptionsService.logOut(); - }, 5000); + await this.logoutService.logout(this.activeAccountId); + // navigate to root so redirect guard can properly route next active user or null user to correct page + await this.router.navigate(["/"]); } private observeAndPersistRememberDeviceValueChanges() { @@ -313,7 +313,9 @@ export class LoginDecryptionOptionsComponent implements OnInit { const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; if (confirmed) { - this.messagingService.send("logout", { userId: userId }); + await this.logoutService.logout(userId); + // navigate to root so redirect guard can properly route next active user or null user to correct page + await this.router.navigate(["/"]); } } } diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.service.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.service.ts index d81d56d63933..3eb96470ed3c 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.service.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.service.ts @@ -3,8 +3,4 @@ export abstract class LoginDecryptionOptionsService { * Handles client-specific logic that runs after a user was successfully created */ abstract handleCreateUserSuccess(): Promise; - /** - * Logs the user out - */ - abstract logOut(): Promise; } diff --git a/libs/auth/src/common/abstractions/index.ts b/libs/auth/src/common/abstractions/index.ts index c0dc500ddb95..937b79e5ba01 100644 --- a/libs/auth/src/common/abstractions/index.ts +++ b/libs/auth/src/common/abstractions/index.ts @@ -6,3 +6,4 @@ export * from "./user-decryption-options.service.abstraction"; export * from "./auth-request.service.abstraction"; export * from "./login-approval-component.service.abstraction"; export * from "./login-success-handler.service"; +export * from "./logout.service"; diff --git a/libs/auth/src/common/abstractions/logout.service.ts b/libs/auth/src/common/abstractions/logout.service.ts new file mode 100644 index 000000000000..16ea3746df27 --- /dev/null +++ b/libs/auth/src/common/abstractions/logout.service.ts @@ -0,0 +1,19 @@ +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { LogoutReason } from "../types"; + +export interface NewActiveUser { + userId: UserId; + authenticationStatus: AuthenticationStatus; +} + +export abstract class LogoutService { + /** + * Logs out the user. + * @param userId The user id. + * @param logoutReason The optional reason for logging out. + * @returns The new active user or undefined if there isn't a new active account. + */ + abstract logout(userId: UserId, logoutReason?: LogoutReason): Promise; +} diff --git a/libs/auth/src/common/services/index.ts b/libs/auth/src/common/services/index.ts index 73d31799b7ed..f66a827a2c2c 100644 --- a/libs/auth/src/common/services/index.ts +++ b/libs/auth/src/common/services/index.ts @@ -7,3 +7,4 @@ export * from "./auth-request/auth-request-api.service"; export * from "./accounts/lock.service"; export * from "./login-success-handler/default-login-success-handler.service"; export * from "./sso-redirect/sso-url.service"; +export * from "./logout/default-logout.service"; diff --git a/libs/auth/src/common/services/logout/default-logout.service.spec.ts b/libs/auth/src/common/services/logout/default-logout.service.spec.ts new file mode 100644 index 000000000000..3febd8416950 --- /dev/null +++ b/libs/auth/src/common/services/logout/default-logout.service.spec.ts @@ -0,0 +1,40 @@ +import { MockProxy, mock } from "jest-mock-extended"; + +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { LogoutService } from "../../abstractions"; +import { LogoutReason } from "../../types"; + +import { DefaultLogoutService } from "./default-logout.service"; + +describe("DefaultLogoutService", () => { + let logoutService: LogoutService; + let messagingService: MockProxy; + + beforeEach(() => { + messagingService = mock(); + logoutService = new DefaultLogoutService(messagingService); + }); + + it("instantiates", () => { + expect(logoutService).not.toBeFalsy(); + }); + + describe("logout", () => { + it("sends logout message without a logout reason when not provided", async () => { + const userId = "1" as UserId; + + await logoutService.logout(userId); + + expect(messagingService.send).toHaveBeenCalledWith("logout", { userId }); + }); + + it("sends logout message with a logout reason when provided", async () => { + const userId = "1" as UserId; + const logoutReason: LogoutReason = "vaultTimeout"; + await logoutService.logout(userId, logoutReason); + expect(messagingService.send).toHaveBeenCalledWith("logout", { userId, logoutReason }); + }); + }); +}); diff --git a/libs/auth/src/common/services/logout/default-logout.service.ts b/libs/auth/src/common/services/logout/default-logout.service.ts new file mode 100644 index 000000000000..4e96c7cfba19 --- /dev/null +++ b/libs/auth/src/common/services/logout/default-logout.service.ts @@ -0,0 +1,14 @@ +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { UserId } from "@bitwarden/common/types/guid"; + +import { LogoutService, NewActiveUser } from "../../abstractions/logout.service"; +import { LogoutReason } from "../../types"; + +export class DefaultLogoutService implements LogoutService { + constructor(protected messagingService: MessagingService) {} + async logout(userId: UserId, logoutReason?: LogoutReason): Promise { + this.messagingService.send("logout", { userId, logoutReason }); + + return undefined; + } +} diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index ecfeb10dcda4..ba0d10561d25 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -89,9 +89,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { ) { this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( map((options) => { - // TODO: Eslint upgrade. Please resolve this since the ?? does nothing - // eslint-disable-next-line no-constant-binary-expression - return options?.trustedDeviceOption != null ?? false; + return options?.trustedDeviceOption != null; }), ); } @@ -99,9 +97,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { supportsDeviceTrustByUserId$(userId: UserId): Observable { return this.userDecryptionOptionsService.userDecryptionOptionsById$(userId).pipe( map((options) => { - // TODO: Eslint upgrade. Please resolve this since the ?? does nothing - // eslint-disable-next-line no-constant-binary-expression - return options?.trustedDeviceOption != null ?? false; + return options?.trustedDeviceOption != null; }), ); } diff --git a/libs/key-management-ui/src/lock/components/lock.component.ts b/libs/key-management-ui/src/lock/components/lock.component.ts index 3cb0dbaca528..a9cd0734b6a0 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -17,7 +17,7 @@ import { import { JslibModule } from "@bitwarden/angular/jslib.module"; import { AnonLayoutWrapperDataService } from "@bitwarden/auth/angular"; -import { PinServiceAbstraction } from "@bitwarden/auth/common"; +import { LogoutService, PinServiceAbstraction } from "@bitwarden/auth/common"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -157,6 +157,7 @@ export class LockComponent implements OnInit, OnDestroy { private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, private biometricService: BiometricsService, + private logoutService: LogoutService, private lockComponentService: LockComponentService, private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, @@ -354,7 +355,9 @@ export class LockComponent implements OnInit, OnDestroy { }); if (confirmed && this.activeAccount != null) { - this.messagingService.send("logout", { userId: this.activeAccount.id }); + await this.logoutService.logout(this.activeAccount.id); + // navigate to root so redirect guard can properly route next active user or null user to correct page + await this.router.navigate(["/"]); } }