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 9e9a1ecf57..d7d3c02ab1 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 @@ -122,10 +122,8 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { async lock(userId: string) { this.loading = true; - await this.vaultTimeoutService.lock(userId); - // 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"]); + await this.lockService.lock(userId as UserId); + await this.router.navigate(["lock"]); } async lockAll() { diff --git a/apps/browser/src/auth/popup/accounts/foreground-lock.service.ts b/apps/browser/src/auth/popup/accounts/foreground-lock.service.ts index 20a52a90d8..91adecd4a0 100644 --- a/apps/browser/src/auth/popup/accounts/foreground-lock.service.ts +++ b/apps/browser/src/auth/popup/accounts/foreground-lock.service.ts @@ -6,10 +6,13 @@ import { MessageListener, MessageSender, } from "@bitwarden/common/platform/messaging"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { newGuid } from "@bitwarden/guid"; +import { UserId } from "@bitwarden/user-core"; const LOCK_ALL_FINISHED = new CommandDefinition<{ requestId: string }>("lockAllFinished"); const LOCK_ALL = new CommandDefinition<{ requestId: string }>("lockAll"); +const LOCK_USER_FINISHED = new CommandDefinition<{ requestId: string }>("lockUserFinished"); +const LOCK_USER = new CommandDefinition<{ requestId: string; userId: UserId }>("lockUser"); export class ForegroundLockService implements LockService { constructor( @@ -18,7 +21,7 @@ export class ForegroundLockService implements LockService { ) {} async lockAll(): Promise { - const requestId = Utils.newGuid(); + const requestId = newGuid(); const finishMessage = firstValueFrom( this.messageListener .messages$(LOCK_ALL_FINISHED) @@ -29,4 +32,19 @@ export class ForegroundLockService implements LockService { await finishMessage; } + + async lock(userId: UserId): Promise { + const requestId = newGuid(); + const finishMessage = firstValueFrom( + this.messageListener + .messages$(LOCK_USER_FINISHED) + .pipe(filter((m) => m.requestId === requestId)), + ); + + this.messageSender.send(LOCK_USER, { requestId, userId }); + + await finishMessage; + } + + async runPlatformOnLockActions(): Promise {} } diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index aa3639e9e9..28639cd1ed 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts @@ -6,6 +6,7 @@ import { mock } from "jest-mock-extended"; import { firstValueFrom, of } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { LockService } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -16,7 +17,6 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeoutSettingsService, - VaultTimeoutService, VaultTimeoutStringType, VaultTimeoutAction, } from "@bitwarden/common/key-management/vault-timeout"; @@ -63,6 +63,7 @@ describe("AccountSecurityComponent", () => { const validationService = mock(); const dialogService = mock(); const platformUtilsService = mock(); + const lockService = mock(); beforeEach(async () => { await TestBed.configureTestingModule({ @@ -83,7 +84,6 @@ describe("AccountSecurityComponent", () => { { provide: PopupRouterCacheService, useValue: mock() }, { provide: ToastService, useValue: mock() }, { provide: UserVerificationService, useValue: mock() }, - { provide: VaultTimeoutService, useValue: mock() }, { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService }, { provide: StateProvider, useValue: mock() }, { provide: CipherService, useValue: mock() }, @@ -92,6 +92,7 @@ describe("AccountSecurityComponent", () => { { provide: OrganizationService, useValue: mock() }, { provide: CollectionService, useValue: mock() }, { provide: ValidationService, useValue: validationService }, + { provide: LockService, useValue: lockService }, ], }) .overrideComponent(AccountSecurityComponent, { diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 65a0d33f93..4a5388ef26 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -25,6 +25,7 @@ import { JslibModule } from "@bitwarden/angular/jslib.module"; import { NudgesService, NudgeType } from "@bitwarden/angular/vault"; import { SpotlightComponent } from "@bitwarden/angular/vault/components/spotlight/spotlight.component"; import { FingerprintDialogComponent, VaultTimeoutInputComponent } from "@bitwarden/auth/angular"; +import { LockService } from "@bitwarden/auth/common"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { getFirstPolicy } from "@bitwarden/common/admin-console/services/policy/default-policy.service"; @@ -36,7 +37,6 @@ import { VaultTimeout, VaultTimeoutAction, VaultTimeoutOption, - VaultTimeoutService, VaultTimeoutSettingsService, VaultTimeoutStringType, } from "@bitwarden/common/key-management/vault-timeout"; @@ -143,7 +143,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { private formBuilder: FormBuilder, private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, - private vaultTimeoutService: VaultTimeoutService, + private lockService: LockService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, public messagingService: MessagingService, private environmentService: EnvironmentService, @@ -695,7 +695,8 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { } async lock() { - await this.vaultTimeoutService.lock(); + const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + await this.lockService.lock(activeUserId); } async logOut() { diff --git a/apps/browser/src/auth/services/extension-lock.service.ts b/apps/browser/src/auth/services/extension-lock.service.ts new file mode 100644 index 0000000000..7e01e8155e --- /dev/null +++ b/apps/browser/src/auth/services/extension-lock.service.ts @@ -0,0 +1,58 @@ +import { DefaultLockService, LogoutService } from "@bitwarden/auth/common"; +import MainBackground from "@bitwarden/browser/background/main.background"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; +import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; +import { BiometricsService, KeyService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; +import { StateEventRunnerService } from "@bitwarden/state"; + +export class ExtensionLockService extends DefaultLockService { + constructor( + accountService: AccountService, + biometricService: BiometricsService, + vaultTimeoutSettingsService: VaultTimeoutSettingsService, + logoutService: LogoutService, + messagingService: MessagingService, + searchService: SearchService, + folderService: FolderService, + masterPasswordService: InternalMasterPasswordServiceAbstraction, + stateEventRunnerService: StateEventRunnerService, + cipherService: CipherService, + authService: AuthService, + systemService: SystemService, + processReloadService: ProcessReloadServiceAbstraction, + logService: LogService, + keyService: KeyService, + private readonly main: MainBackground, + ) { + super( + accountService, + biometricService, + vaultTimeoutSettingsService, + logoutService, + messagingService, + searchService, + folderService, + masterPasswordService, + stateEventRunnerService, + cipherService, + authService, + systemService, + processReloadService, + logService, + keyService, + ); + } + + async runPlatformOnLockActions(): Promise { + await this.main.refreshMenu(true); + } +} diff --git a/apps/browser/src/background/commands.background.ts b/apps/browser/src/background/commands.background.ts index 3e6e86cd3d..696fd5c4f0 100644 --- a/apps/browser/src/background/commands.background.ts +++ b/apps/browser/src/background/commands.background.ts @@ -1,9 +1,13 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { firstValueFrom } from "rxjs"; + +import { LockService } 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"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ExtensionCommand, ExtensionCommandType } from "@bitwarden/common/autofill/constants"; -import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; // FIXME (PM-22628): Popup imports are forbidden in background @@ -21,9 +25,10 @@ export default class CommandsBackground { constructor( private main: MainBackground, private platformUtilsService: PlatformUtilsService, - private vaultTimeoutService: VaultTimeoutService, private authService: AuthService, private generatePasswordToClipboard: () => Promise, + private accountService: AccountService, + private lockService: LockService, ) { this.isSafari = this.platformUtilsService.isSafari(); this.isVivaldi = this.platformUtilsService.isVivaldi(); @@ -72,9 +77,11 @@ export default class CommandsBackground { case "open_popup": await this.openPopup(); break; - case "lock_vault": - await this.vaultTimeoutService.lock(); + case "lock_vault": { + const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + await this.lockService.lock(activeUserId); break; + } default: break; } diff --git a/apps/browser/src/background/idle.background.ts b/apps/browser/src/background/idle.background.ts index 0f89aa4792..66a5604a8b 100644 --- a/apps/browser/src/background/idle.background.ts +++ b/apps/browser/src/background/idle.background.ts @@ -1,6 +1,6 @@ import { firstValueFrom } from "rxjs"; -import { LogoutService } from "@bitwarden/auth/common"; +import { LockService, LogoutService } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { VaultTimeoutAction, @@ -23,6 +23,7 @@ export default class IdleBackground { private serverNotificationsService: ServerNotificationsService, private accountService: AccountService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private lockService: LockService, private logoutService: LogoutService, ) { this.idle = chrome.idle || (browser != null ? browser.idle : null); @@ -66,7 +67,7 @@ export default class IdleBackground { if (action === VaultTimeoutAction.LogOut) { await this.logoutService.logout(userId as UserId, "vaultTimeout"); } else { - await this.vaultTimeoutService.lock(userId); + await this.lockService.lock(userId as UserId); } } } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 00e5526f4e..3e6eac1f13 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -20,9 +20,9 @@ import { AuthRequestService, AuthRequestServiceAbstraction, DefaultAuthRequestApiService, - DefaultLockService, DefaultLogoutService, InternalUserDecryptionOptionsServiceAbstraction, + LockService, LoginEmailServiceAbstraction, LogoutReason, UserDecryptionOptionsService, @@ -270,6 +270,7 @@ import { } from "@bitwarden/vault-export-core"; import { AuthStatusBadgeUpdaterService } from "../auth/services/auth-status-badge-updater.service"; +import { ExtensionLockService } from "../auth/services/extension-lock.service"; import { OverlayNotificationsBackground as OverlayNotificationsBackgroundInterface } from "../autofill/background/abstractions/overlay-notifications.background"; import { OverlayBackground as OverlayBackgroundInterface } from "../autofill/background/abstractions/overlay.background"; import { AutoSubmitLoginBackground } from "../autofill/background/auto-submit-login.background"; @@ -363,6 +364,7 @@ export default class MainBackground { folderService: InternalFolderServiceAbstraction; userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction; collectionService: CollectionService; + lockService: LockService; vaultTimeoutService?: VaultTimeoutService; vaultTimeoutSettingsService: VaultTimeoutSettingsService; passwordGenerationService: PasswordGenerationServiceAbstraction; @@ -496,16 +498,6 @@ export default class MainBackground { private phishingDataService: PhishingDataService; constructor() { - // Services - const lockedCallback = async (userId: UserId) => { - await this.refreshMenu(true); - if (this.systemService != null) { - await this.systemService.clearPendingClipboard(); - await this.biometricsService.setShouldAutopromptNow(false); - await this.processReloadService.startProcessReload(this.authService); - } - }; - const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) => await this.logout(logoutReason, userId); @@ -987,27 +979,6 @@ export default class MainBackground { this.restrictedItemTypesService, ); - const logoutService = new DefaultLogoutService(this.messagingService); - this.vaultTimeoutService = new VaultTimeoutService( - this.accountService, - this.masterPasswordService, - this.cipherService, - this.folderService, - this.collectionService, - this.platformUtilsService, - this.messagingService, - this.searchService, - this.stateService, - this.tokenService, - this.authService, - this.vaultTimeoutSettingsService, - this.stateEventRunnerService, - this.taskSchedulerService, - this.logService, - this.biometricsService, - lockedCallback, - logoutService, - ); this.containerService = new ContainerService(this.keyService, this.encryptService); this.sendStateProvider = new SendStateProvider(this.stateProvider); @@ -1271,6 +1242,7 @@ export default class MainBackground { this.biometricStateService, this.accountService, this.logService, + this.authService, ); // Background @@ -1284,7 +1256,36 @@ export default class MainBackground { this.authService, ); - const lockService = new DefaultLockService(this.accountService, this.vaultTimeoutService); + const logoutService = new DefaultLogoutService(this.messagingService); + this.lockService = new ExtensionLockService( + this.accountService, + this.biometricsService, + this.vaultTimeoutSettingsService, + logoutService, + this.messagingService, + this.searchService, + this.folderService, + this.masterPasswordService, + this.stateEventRunnerService, + this.cipherService, + this.authService, + this.systemService, + this.processReloadService, + this.logService, + this.keyService, + this, + ); + + this.vaultTimeoutService = new VaultTimeoutService( + this.accountService, + this.platformUtilsService, + this.authService, + this.vaultTimeoutSettingsService, + this.taskSchedulerService, + this.logService, + this.lockService, + logoutService, + ); this.runtimeBackground = new RuntimeBackground( this, @@ -1298,7 +1299,7 @@ export default class MainBackground { this.configService, messageListener, this.accountService, - lockService, + this.lockService, this.billingAccountProfileStateService, this.browserInitialInstallService, ); @@ -1318,9 +1319,10 @@ export default class MainBackground { this.commandsBackground = new CommandsBackground( this, this.platformUtilsService, - this.vaultTimeoutService, this.authService, () => this.generatePasswordToClipboard(), + this.accountService, + this.lockService, ); this.taskService = new DefaultTaskService( @@ -1405,6 +1407,7 @@ export default class MainBackground { this.serverNotificationsService, this.accountService, this.vaultTimeoutSettingsService, + this.lockService, logoutService, ); @@ -1752,7 +1755,7 @@ export default class MainBackground { } await this.mainContextMenuHandler?.noAccess(); await this.systemService.clearPendingClipboard(); - await this.processReloadService.startProcessReload(this.authService); + await this.processReloadService.startProcessReload(); } private async needsStorageReseed(userId: UserId): Promise { diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 9dc2bff65e..de0d79a89d 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -257,7 +257,7 @@ export default class RuntimeBackground { this.lockedVaultPendingNotifications.push(msg.data); break; case "lockVault": - await this.main.vaultTimeoutService.lock(msg.userId); + await this.lockService.lock(msg.userId); break; case "lockAll": { @@ -265,6 +265,14 @@ export default class RuntimeBackground { this.messagingService.send("lockAllFinished", { requestId: msg.requestId }); } break; + case "lockUser": + { + await this.lockService.lock(msg.userId); + this.messagingService.send("lockUserFinished", { + requestId: msg.requestId, + }); + } + break; case "logout": await this.main.logout(msg.expired, msg.userId); break; diff --git a/apps/browser/src/key-management/vault-timeout/foreground-vault-timeout.service.ts b/apps/browser/src/key-management/vault-timeout/foreground-vault-timeout.service.ts index 4081ab0335..8bad50bfae 100644 --- a/apps/browser/src/key-management/vault-timeout/foreground-vault-timeout.service.ts +++ b/apps/browser/src/key-management/vault-timeout/foreground-vault-timeout.service.ts @@ -2,15 +2,10 @@ // @ts-strict-ignore import { VaultTimeoutService as BaseVaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout/abstractions/vault-timeout.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { UserId } from "@bitwarden/common/types/guid"; export class ForegroundVaultTimeoutService implements BaseVaultTimeoutService { constructor(protected messagingService: MessagingService) {} // should only ever run in background async checkVaultTimeout(): Promise {} - - async lock(userId?: UserId): Promise { - this.messagingService.send("lockVault", { userId }); - } } diff --git a/apps/cli/src/auth/commands/lock.command.ts b/apps/cli/src/auth/commands/lock.command.ts index f3b8018f40..eef85980d5 100644 --- a/apps/cli/src/auth/commands/lock.command.ts +++ b/apps/cli/src/auth/commands/lock.command.ts @@ -1,16 +1,22 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout"; +import { firstValueFrom } from "rxjs"; + +import { LockService } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { Response } from "../../models/response"; import { MessageResponse } from "../../models/response/message.response"; export class LockCommand { - constructor(private vaultTimeoutService: VaultTimeoutService) {} + constructor( + private lockService: LockService, + private accountService: AccountService, + ) {} async run() { - await this.vaultTimeoutService.lock(); - process.env.BW_SESSION = null; + const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); + await this.lockService.lock(activeUserId); + process.env.BW_SESSION = undefined; const res = new MessageResponse("Your vault is locked.", null); return Response.success(res); } diff --git a/apps/cli/src/key-management/cli-process-reload.service.ts b/apps/cli/src/key-management/cli-process-reload.service.ts new file mode 100644 index 0000000000..243de7cae4 --- /dev/null +++ b/apps/cli/src/key-management/cli-process-reload.service.ts @@ -0,0 +1,10 @@ +import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; + +/** + * CLI implementation of ProcessReloadServiceAbstraction. + * This is NOOP since there is no effective way to process reload the CLI. + */ +export class CliProcessReloadService extends ProcessReloadServiceAbstraction { + async startProcessReload(): Promise {} + async cancelProcessReload(): Promise {} +} diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index d318a44c67..bd51cf4dd9 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -160,7 +160,10 @@ export class OssServeConfigurator { this.serviceContainer.cipherService, this.serviceContainer.accountService, ); - this.lockCommand = new LockCommand(this.serviceContainer.vaultTimeoutService); + this.lockCommand = new LockCommand( + serviceContainer.lockService, + serviceContainer.accountService, + ); this.unlockCommand = new UnlockCommand( this.serviceContainer.accountService, this.serviceContainer.masterPasswordService, diff --git a/apps/cli/src/platform/services/cli-system.service.ts b/apps/cli/src/platform/services/cli-system.service.ts new file mode 100644 index 0000000000..5f647a0f88 --- /dev/null +++ b/apps/cli/src/platform/services/cli-system.service.ts @@ -0,0 +1,10 @@ +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; + +/** + * CLI implementation of SystemService. + * The implementation is NOOP since these functions are meant for GUI clients. + */ +export class CliSystemService extends SystemService { + async clearClipboard(clipboardValue: string, timeoutMs?: number): Promise {} + async clearPendingClipboard(): Promise {} +} diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index 41368269fa..a5f12b3403 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -250,7 +250,10 @@ export class Program extends BaseProgram { return; } - const command = new LockCommand(this.serviceContainer.vaultTimeoutService); + const command = new LockCommand( + this.serviceContainer.lockService, + this.serviceContainer.accountService, + ); const response = await command.run(); this.processResponse(response); }); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 3c4ee55361..c9f1d11210 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -20,6 +20,9 @@ import { SsoUrlService, AuthRequestApiServiceAbstraction, DefaultAuthRequestApiService, + DefaultLockService, + DefaultLogoutService, + LockService, } from "@bitwarden/auth/common"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; import { EventUploadService as EventUploadServiceAbstraction } from "@bitwarden/common/abstractions/event/event-upload.service"; @@ -199,9 +202,11 @@ import { } from "@bitwarden/vault-export-core"; import { CliBiometricsService } from "../key-management/cli-biometrics-service"; +import { CliProcessReloadService } from "../key-management/cli-process-reload.service"; import { flagEnabled } from "../platform/flags"; import { CliPlatformUtilsService } from "../platform/services/cli-platform-utils.service"; import { CliSdkLoadService } from "../platform/services/cli-sdk-load.service"; +import { CliSystemService } from "../platform/services/cli-system.service"; import { ConsoleLogService } from "../platform/services/console-log.service"; import { I18nService } from "../platform/services/i18n.service"; import { LowdbStorageService } from "../platform/services/lowdb-storage.service"; @@ -318,6 +323,7 @@ export class ServiceContainer { securityStateService: SecurityStateService; masterPasswordUnlockService: MasterPasswordUnlockService; cipherArchiveService: CipherArchiveService; + lockService: LockService; constructor() { let p = null; @@ -778,9 +784,6 @@ export class ServiceContainer { this.folderApiService = new FolderApiService(this.folderService, this.apiService); - const lockedCallback = async (userId: UserId) => - await this.keyService.clearStoredUserKey(userId); - this.userVerificationApiService = new UserVerificationApiService(this.apiService); this.userVerificationService = new UserVerificationService( @@ -796,25 +799,35 @@ export class ServiceContainer { ); const biometricService = new CliBiometricsService(); + const logoutService = new DefaultLogoutService(this.messagingService); + const processReloadService = new CliProcessReloadService(); + const systemService = new CliSystemService(); + this.lockService = new DefaultLockService( + this.accountService, + biometricService, + this.vaultTimeoutSettingsService, + logoutService, + this.messagingService, + this.searchService, + this.folderService, + this.masterPasswordService, + this.stateEventRunnerService, + this.cipherService, + this.authService, + systemService, + processReloadService, + this.logService, + this.keyService, + ); this.vaultTimeoutService = new DefaultVaultTimeoutService( this.accountService, - this.masterPasswordService, - this.cipherService, - this.folderService, - this.collectionService, this.platformUtilsService, - this.messagingService, - this.searchService, - this.stateService, - this.tokenService, this.authService, this.vaultTimeoutSettingsService, - this.stateEventRunnerService, this.taskSchedulerService, this.logService, - biometricService, - lockedCallback, + this.lockService, undefined, ); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 9e0a3b0473..6243ba1e53 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -32,6 +32,7 @@ import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent } from "@bitwarden/auth/angular"; import { DESKTOP_SSO_CALLBACK, + LockService, LogoutReason, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; @@ -195,6 +196,7 @@ export class AppComponent implements OnInit, OnDestroy { private pinService: PinServiceAbstraction, private readonly tokenService: TokenService, private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, + private readonly lockService: LockService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -245,7 +247,7 @@ export class AppComponent implements OnInit, OnDestroy { // eslint-disable-next-line @typescript-eslint/no-floating-promises this.updateAppMenu(); await this.systemService.clearPendingClipboard(); - await this.processReloadService.startProcessReload(this.authService); + await this.processReloadService.startProcessReload(); break; case "authBlocked": // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -258,21 +260,10 @@ export class AppComponent implements OnInit, OnDestroy { this.loading = false; break; case "lockVault": - await this.vaultTimeoutService.lock(message.userId); + await this.lockService.lock(message.userId); break; case "lockAllVaults": { - const currentUser = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a.id)), - ); - const accounts = await firstValueFrom(this.accountService.accounts$); - await this.vaultTimeoutService.lock(currentUser); - for (const account of Object.keys(accounts)) { - if (account === currentUser) { - continue; - } - - await this.vaultTimeoutService.lock(account); - } + await this.lockService.lockAll(); break; } case "locked": @@ -286,12 +277,12 @@ export class AppComponent implements OnInit, OnDestroy { } await this.updateAppMenu(); await this.systemService.clearPendingClipboard(); - await this.processReloadService.startProcessReload(this.authService); + await this.processReloadService.startProcessReload(); break; case "startProcessReload": // 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.processReloadService.startProcessReload(this.authService); + this.processReloadService.startProcessReload(); break; case "cancelProcessReload": this.processReloadService.cancelProcessReload(); @@ -812,11 +803,9 @@ export class AppComponent implements OnInit, OnDestroy { } const options = await this.getVaultTimeoutOptions(userId); if (options[0] === timeout) { - // 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 options[1] === "logOut" - ? this.logOut("vaultTimeout", userId as UserId) - : await this.vaultTimeoutService.lock(userId); + ? await this.logOut("vaultTimeout", userId as UserId) + : await this.lockService.lock(userId as UserId); } } } diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index a0ee33a459..50453f08fe 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -262,6 +262,7 @@ const safeProviders: SafeProvider[] = [ BiometricStateService, AccountServiceAbstraction, LogService, + AuthServiceAbstraction, ], }), safeProvider({ diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 4571116312..30dbee9fac 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -8,6 +8,7 @@ import { Subject, filter, firstValueFrom, map, timeout } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { DocumentLangSetter } from "@bitwarden/angular/platform/i18n"; +import { LockService } from "@bitwarden/auth/common"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -16,7 +17,6 @@ import { TokenService } from "@bitwarden/common/auth/abstractions/token.service" import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; -import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -58,8 +58,8 @@ export class AppComponent implements OnDestroy, OnInit { private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, private ngZone: NgZone, - private vaultTimeoutService: VaultTimeoutService, private keyService: KeyService, + private lockService: LockService, private collectionService: CollectionService, private searchService: SearchService, private serverNotificationsService: ServerNotificationsService, @@ -113,11 +113,13 @@ export class AppComponent implements OnDestroy, OnInit { // note: the message.logoutReason isn't consumed anymore because of the process reload clearing any toasts. await this.logOut(message.redirect); break; - case "lockVault": - await this.vaultTimeoutService.lock(); + case "lockVault": { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.lockService.lock(userId); break; + } case "locked": - await this.processReloadService.startProcessReload(this.authService); + await this.processReloadService.startProcessReload(); break; case "lockedUrl": break; @@ -267,7 +269,7 @@ export class AppComponent implements OnDestroy, OnInit { await this.router.navigate(["/"]); } - await this.processReloadService.startProcessReload(this.authService); + await this.processReloadService.startProcessReload(); // Normally we would need to reset the loading state to false or remove the layout_frontend // class from the body here, but the process reload completely reloads the app so diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index 72117f547d..72a563a77a 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -80,6 +80,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { SdkClientFactory } from "@bitwarden/common/platform/abstractions/sdk/sdk-client-factory"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { IpcService } from "@bitwarden/common/platform/ipc"; // eslint-disable-next-line no-restricted-imports -- Needed for DI import { @@ -143,6 +144,7 @@ import { WebEnvironmentService } from "../platform/web-environment.service"; import { WebMigrationRunner } from "../platform/web-migration-runner"; import { WebSdkLoadService } from "../platform/web-sdk-load.service"; import { WebStorageServiceProvider } from "../platform/web-storage-service.provider"; +import { WebSystemService } from "../platform/web-system.service"; import { EventService } from "./event.service"; import { InitService } from "./init.service"; @@ -428,6 +430,11 @@ const safeProviders: SafeProvider[] = [ useClass: WebPremiumInterestStateService, deps: [StateProvider], }), + safeProvider({ + provide: SystemService, + useClass: WebSystemService, + deps: [], + }), ]; @NgModule({ diff --git a/apps/web/src/app/key-management/services/web-process-reload.service.ts b/apps/web/src/app/key-management/services/web-process-reload.service.ts index c542c97c0e..6f055cd990 100644 --- a/apps/web/src/app/key-management/services/web-process-reload.service.ts +++ b/apps/web/src/app/key-management/services/web-process-reload.service.ts @@ -1,10 +1,9 @@ -import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; export class WebProcessReloadService implements ProcessReloadServiceAbstraction { constructor(private window: Window) {} - async startProcessReload(authService: AuthService): Promise { + async startProcessReload(): Promise { this.window.location.reload(); } diff --git a/apps/web/src/app/platform/web-system.service.ts b/apps/web/src/app/platform/web-system.service.ts new file mode 100644 index 0000000000..b614d0f924 --- /dev/null +++ b/apps/web/src/app/platform/web-system.service.ts @@ -0,0 +1,10 @@ +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; + +/** + * Web implementation of SystemService. + * The implementation is NOOP since these functions are not supported on web. + */ +export class WebSystemService extends SystemService { + async clearClipboard(clipboardValue: string, timeoutMs?: number): Promise {} + async clearPendingClipboard(): Promise {} +} diff --git a/libs/angular/src/services/injection-tokens.ts b/libs/angular/src/services/injection-tokens.ts index 6bf3ab7725..543dcaa732 100644 --- a/libs/angular/src/services/injection-tokens.ts +++ b/libs/angular/src/services/injection-tokens.ts @@ -35,9 +35,6 @@ export const SECURE_STORAGE = new SafeInjectionToken("SE export const LOGOUT_CALLBACK = new SafeInjectionToken< (logoutReason: LogoutReason, userId?: string) => Promise >("LOGOUT_CALLBACK"); -export const LOCKED_CALLBACK = new SafeInjectionToken<(userId?: string) => Promise>( - "LOCKED_CALLBACK", -); export const SUPPORTS_SECURE_STORAGE = new SafeInjectionToken("SUPPORTS_SECURE_STORAGE"); export const LOCALES_DIRECTORY = new SafeInjectionToken("LOCALES_DIRECTORY"); export const SYSTEM_LANGUAGE = new SafeInjectionToken("SYSTEM_LANGUAGE"); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index c60bc2e2f0..a608b290f0 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -40,9 +40,11 @@ import { AuthRequestService, AuthRequestServiceAbstraction, DefaultAuthRequestApiService, + DefaultLockService, DefaultLoginSuccessHandlerService, DefaultLogoutService, InternalUserDecryptionOptionsServiceAbstraction, + LockService, LoginEmailService, LoginEmailServiceAbstraction, LoginStrategyService, @@ -161,6 +163,7 @@ import { OrganizationSponsorshipApiService } from "@bitwarden/common/billing/ser import { OrganizationBillingService } from "@bitwarden/common/billing/services/organization-billing.service"; import { DefaultSubscriptionPricingService } from "@bitwarden/common/billing/services/subscription-pricing.service"; import { HibpApiService } from "@bitwarden/common/dirt/services/hibp-api.service"; +import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; import { DefaultKeyGenerationService, KeyGenerationService, @@ -219,6 +222,7 @@ import { SdkClientFactory } from "@bitwarden/common/platform/abstractions/sdk/sd import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { ValidationService as ValidationServiceAbstraction } from "@bitwarden/common/platform/abstractions/validation.service"; import { ActionsService } from "@bitwarden/common/platform/actions"; import { UnsupportedActionsService } from "@bitwarden/common/platform/actions/unsupported-actions.service"; @@ -403,7 +407,6 @@ import { HTTP_OPERATIONS, INTRAPROCESS_MESSAGING_SUBJECT, LOCALES_DIRECTORY, - LOCKED_CALLBACK, LOG_MAC_FAILURES, LOGOUT_CALLBACK, OBSERVABLE_DISK_STORAGE, @@ -459,10 +462,6 @@ const safeProviders: SafeProvider[] = [ }, deps: [MessagingServiceAbstraction], }), - safeProvider({ - provide: LOCKED_CALLBACK, - useValue: null, - }), safeProvider({ provide: LOG_MAC_FAILURES, useValue: true, @@ -889,22 +888,12 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultVaultTimeoutService, deps: [ AccountServiceAbstraction, - InternalMasterPasswordServiceAbstraction, - CipherServiceAbstraction, - FolderServiceAbstraction, - CollectionService, PlatformUtilsServiceAbstraction, - MessagingServiceAbstraction, - SearchServiceAbstraction, - StateServiceAbstraction, - TokenServiceAbstraction, AuthServiceAbstraction, VaultTimeoutSettingsService, - StateEventRunnerService, TaskSchedulerService, LogService, - BiometricsService, - LOCKED_CALLBACK, + LockService, LogoutService, ], }), @@ -1718,6 +1707,27 @@ const safeProviders: SafeProvider[] = [ InternalMasterPasswordServiceAbstraction, ], }), + safeProvider({ + provide: LockService, + useClass: DefaultLockService, + deps: [ + AccountService, + BiometricsService, + VaultTimeoutSettingsService, + LogoutService, + MessagingServiceAbstraction, + SearchServiceAbstraction, + FolderServiceAbstraction, + InternalMasterPasswordServiceAbstraction, + StateEventRunnerService, + CipherServiceAbstraction, + AuthServiceAbstraction, + SystemService, + ProcessReloadServiceAbstraction, + LogService, + KeyService, + ], + }), safeProvider({ provide: CipherArchiveService, useClass: DefaultCipherArchiveService, diff --git a/libs/auth/src/common/services/accounts/lock.service.ts b/libs/auth/src/common/services/accounts/lock.service.ts index 1f42873408..7db9331ffc 100644 --- a/libs/auth/src/common/services/accounts/lock.service.ts +++ b/libs/auth/src/common/services/accounts/lock.service.ts @@ -1,20 +1,55 @@ -import { combineLatest, firstValueFrom, map } from "rxjs"; +import { combineLatest, filter, firstValueFrom, map, timeout } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { assertNonNullish } from "@bitwarden/common/auth/utils"; +import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; +import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { UserId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; +import { BiometricsService, KeyService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; +import { StateEventRunnerService } from "@bitwarden/state"; + +import { LogoutService } from "../../abstractions"; export abstract class LockService { /** * Locks all accounts. */ abstract lockAll(): Promise; + /** + * Performs lock for a user. + * @param userId The user id to lock + */ + abstract lock(userId: UserId): Promise; + + abstract runPlatformOnLockActions(): Promise; } export class DefaultLockService implements LockService { constructor( private readonly accountService: AccountService, - private readonly vaultTimeoutService: VaultTimeoutService, + private readonly biometricService: BiometricsService, + private readonly vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private readonly logoutService: LogoutService, + private readonly messagingService: MessagingService, + private readonly searchService: SearchService, + private readonly folderService: FolderService, + private readonly masterPasswordService: InternalMasterPasswordServiceAbstraction, + private readonly stateEventRunnerService: StateEventRunnerService, + private readonly cipherService: CipherService, + private readonly authService: AuthService, + private readonly systemService: SystemService, + private readonly processReloadService: ProcessReloadServiceAbstraction, + private readonly logService: LogService, + private readonly keyService: KeyService, ) {} async lockAll() { @@ -36,14 +71,88 @@ export class DefaultLockService implements LockService { ); for (const otherAccount of accounts.otherAccounts) { - await this.vaultTimeoutService.lock(otherAccount); + await this.lock(otherAccount); } // Do the active account last in case we ever try to route the user on lock // that way this whole operation will be complete before that routing // could take place. if (accounts.activeAccount != null) { - await this.vaultTimeoutService.lock(accounts.activeAccount); + await this.lock(accounts.activeAccount); } } + + async lock(userId: UserId): Promise { + assertNonNullish(userId, "userId", "LockService"); + + this.logService.info(`[LockService] Locking user ${userId}`); + + // If user already logged out, then skip locking + if ( + (await firstValueFrom(this.authService.authStatusFor$(userId))) === + AuthenticationStatus.LoggedOut + ) { + return; + } + + // If user cannot lock, then logout instead + if (!(await this.vaultTimeoutSettingsService.canLock(userId))) { + // Logout should perform the same steps + await this.logoutService.logout(userId, "vaultTimeout"); + this.logService.info(`[LockService] User ${userId} cannot lock, logging out instead.`); + return; + } + + await this.wipeDecryptedState(userId); + await this.waitForLockedStatus(userId); + await this.systemService.clearPendingClipboard(); + await this.runPlatformOnLockActions(); + + this.logService.info(`[LockService] Locked user ${userId}`); + + // Subscribers navigate the client to the lock screen based on this lock message. + // We need to disable auto-prompting as we are just entering a locked state now. + await this.biometricService.setShouldAutopromptNow(false); + this.messagingService.send("locked", { userId }); + + // Wipe the current process to clear active secrets in memory. + await this.processReloadService.startProcessReload(); + } + + private async wipeDecryptedState(userId: UserId) { + // Manually clear state + await this.searchService.clearIndex(userId); + //! DO NOT REMOVE folderService.clearDecryptedFolderState ! For more information see PM-25660 + await this.folderService.clearDecryptedFolderState(userId); + await this.masterPasswordService.clearMasterKey(userId); + await this.cipherService.clearCache(userId); + // Clear CLI unlock state + await this.keyService.clearStoredUserKey(userId); + + // This will clear ephemeral state such as the user's user key based on the key definition's clear-on + await this.stateEventRunnerService.handleEvent("lock", userId); + } + + private async waitForLockedStatus(userId: UserId): Promise { + // HACK: Start listening for the transition of the locking user from something to the locked state. + // This is very much a hack to ensure that the authentication status to retrievable right after + // it does its work. Particularly and `"locked"` message. Instead the message should be deprecated + // and people should subscribe and react to `authStatusFor$` themselves. + await firstValueFrom( + this.authService.authStatusFor$(userId).pipe( + filter((authStatus) => authStatus === AuthenticationStatus.Locked), + timeout({ + first: 5_000, + with: () => { + throw new Error("The lock process did not complete in a reasonable amount of time."); + }, + }), + ), + ); + } + + async runPlatformOnLockActions(): Promise { + // No platform specific actions to run for this platform. + return; + } } diff --git a/libs/auth/src/common/services/accounts/lock.services.spec.ts b/libs/auth/src/common/services/accounts/lock.services.spec.ts index 2731029802..e22a6f7158 100644 --- a/libs/auth/src/common/services/accounts/lock.services.spec.ts +++ b/libs/auth/src/common/services/accounts/lock.services.spec.ts @@ -1,8 +1,23 @@ import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; -import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; +import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; import { mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; +import { BiometricsService, KeyService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; +import { StateEventRunnerService } from "@bitwarden/state"; + +import { LogoutService } from "../../abstractions"; import { DefaultLockService } from "./lock.service"; @@ -12,10 +27,57 @@ describe("DefaultLockService", () => { const mockUser3 = "user3" as UserId; const accountService = mockAccountServiceWith(mockUser1); - const vaultTimeoutService = mock(); + const biometricsService = mock(); + const vaultTimeoutSettingsService = mock(); + const logoutService = mock(); + const messagingService = mock(); + const searchService = mock(); + const folderService = mock(); + const masterPasswordService = mock(); + const stateEventRunnerService = mock(); + const cipherService = mock(); + const authService = mock(); + const systemService = mock(); + const processReloadService = mock(); + const logService = mock(); + const keyService = mock(); + const sut = new DefaultLockService( + accountService, + biometricsService, + vaultTimeoutSettingsService, + logoutService, + messagingService, + searchService, + folderService, + masterPasswordService, + stateEventRunnerService, + cipherService, + authService, + systemService, + processReloadService, + logService, + keyService, + ); - const sut = new DefaultLockService(accountService, vaultTimeoutService); describe("lockAll", () => { + const sut = new DefaultLockService( + accountService, + biometricsService, + vaultTimeoutSettingsService, + logoutService, + messagingService, + searchService, + folderService, + masterPasswordService, + stateEventRunnerService, + cipherService, + authService, + systemService, + processReloadService, + logService, + keyService, + ); + it("locks the active account last", async () => { await accountService.addAccount(mockUser2, { name: "name2", @@ -25,19 +87,49 @@ describe("DefaultLockService", () => { await accountService.addAccount(mockUser3, { name: "name3", - email: "email3@example.com", + email: "name3@example.com", emailVerified: false, }); + const lockSpy = jest.spyOn(sut, "lock").mockResolvedValue(undefined); + await sut.lockAll(); - expect(vaultTimeoutService.lock).toHaveBeenCalledTimes(3); // Non-Active users should be called first - expect(vaultTimeoutService.lock).toHaveBeenNthCalledWith(1, mockUser2); - expect(vaultTimeoutService.lock).toHaveBeenNthCalledWith(2, mockUser3); + expect(lockSpy).toHaveBeenNthCalledWith(1, mockUser2); + expect(lockSpy).toHaveBeenNthCalledWith(2, mockUser3); // Active user should be called last - expect(vaultTimeoutService.lock).toHaveBeenNthCalledWith(3, mockUser1); + expect(lockSpy).toHaveBeenNthCalledWith(3, mockUser1); + }); + }); + + describe("lock", () => { + const userId = mockUser1; + + it("returns early if user is already logged out", async () => { + authService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.LoggedOut)); + await sut.lock(userId); + // Should return early, not call logoutService.logout + expect(logoutService.logout).not.toHaveBeenCalled(); + expect(stateEventRunnerService.handleEvent).not.toHaveBeenCalled(); + }); + + it("logs out if user cannot lock", async () => { + authService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + vaultTimeoutSettingsService.canLock.mockResolvedValue(false); + await sut.lock(userId); + expect(logoutService.logout).toHaveBeenCalledWith(userId, "vaultTimeout"); + expect(stateEventRunnerService.handleEvent).not.toHaveBeenCalled(); + }); + + it("locks user", async () => { + authService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Locked)); + logoutService.logout.mockClear(); + vaultTimeoutSettingsService.canLock.mockResolvedValue(true); + await sut.lock(userId); + expect(logoutService.logout).not.toHaveBeenCalled(); + expect(stateEventRunnerService.handleEvent).toHaveBeenCalledWith("lock", userId); }); }); }); diff --git a/libs/common/src/key-management/abstractions/process-reload.service.ts b/libs/common/src/key-management/abstractions/process-reload.service.ts index e46c1e2319..4993f2212b 100644 --- a/libs/common/src/key-management/abstractions/process-reload.service.ts +++ b/libs/common/src/key-management/abstractions/process-reload.service.ts @@ -1,6 +1,4 @@ -import { AuthService } from "../../auth/abstractions/auth.service"; - export abstract class ProcessReloadServiceAbstraction { - abstract startProcessReload(authService: AuthService): Promise; + abstract startProcessReload(): Promise; abstract cancelProcessReload(): void; } diff --git a/libs/common/src/key-management/services/default-process-reload.service.ts b/libs/common/src/key-management/services/default-process-reload.service.ts index 24bf81abcd..ddbcf7e553 100644 --- a/libs/common/src/key-management/services/default-process-reload.service.ts +++ b/libs/common/src/key-management/services/default-process-reload.service.ts @@ -30,16 +30,17 @@ export class DefaultProcessReloadService implements ProcessReloadServiceAbstract private biometricStateService: BiometricStateService, private accountService: AccountService, private logService: LogService, + private authService: AuthService, ) {} - async startProcessReload(authService: AuthService): Promise { + async startProcessReload(): Promise { const accounts = await firstValueFrom(this.accountService.accounts$); if (accounts != null) { const keys = Object.keys(accounts); if (keys.length > 0) { for (const userId of keys) { - let status = await firstValueFrom(authService.authStatusFor$(userId as UserId)); - status = await authService.getAuthStatus(userId); + let status = await firstValueFrom(this.authService.authStatusFor$(userId as UserId)); + status = await this.authService.getAuthStatus(userId); if (status === AuthenticationStatus.Unlocked) { this.logService.info( "[Process Reload Service] User unlocked, preventing process reload", diff --git a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout.service.ts b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout.service.ts index 401fb8b107..d8cec9fc03 100644 --- a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout.service.ts +++ b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout.service.ts @@ -1,4 +1,3 @@ export abstract class VaultTimeoutService { abstract checkVaultTimeout(): Promise; - abstract lock(userId?: string): Promise; } diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.spec.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.spec.ts index 5ba434f718..51eec18f17 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.spec.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.spec.ts @@ -5,31 +5,17 @@ import { BehaviorSubject, from, of } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { CollectionService } from "@bitwarden/admin-console/common"; -// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { LogoutService } from "@bitwarden/auth/common"; -// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { BiometricsService } from "@bitwarden/key-management"; -import { StateService } from "@bitwarden/state"; +import { LockService, LogoutService } from "@bitwarden/auth/common"; import { FakeAccountService, mockAccountServiceWith } from "../../../../spec"; import { AccountInfo } from "../../../auth/abstractions/account.service"; import { AuthService } from "../../../auth/abstractions/auth.service"; -import { TokenService } from "../../../auth/abstractions/token.service"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { LogService } from "../../../platform/abstractions/log.service"; -import { MessagingService } from "../../../platform/abstractions/messaging.service"; import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service"; import { Utils } from "../../../platform/misc/utils"; import { TaskSchedulerService } from "../../../platform/scheduling"; -import { StateEventRunnerService } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { CipherService } from "../../../vault/abstractions/cipher.service"; -import { FolderService } from "../../../vault/abstractions/folder/folder.service.abstraction"; -import { SearchService } from "../../../vault/abstractions/search.service"; -import { FakeMasterPasswordService } from "../../master-password/services/fake-master-password.service"; import { VaultTimeoutAction } from "../enums/vault-timeout-action.enum"; import { VaultTimeout, VaultTimeoutStringType } from "../types/vault-timeout.type"; @@ -38,23 +24,13 @@ import { VaultTimeoutService } from "./vault-timeout.service"; describe("VaultTimeoutService", () => { let accountService: FakeAccountService; - let masterPasswordService: FakeMasterPasswordService; - let cipherService: MockProxy; - let folderService: MockProxy; - let collectionService: MockProxy; let platformUtilsService: MockProxy; - let messagingService: MockProxy; - let searchService: MockProxy; - let stateService: MockProxy; - let tokenService: MockProxy; let authService: MockProxy; let vaultTimeoutSettingsService: MockProxy; - let stateEventRunnerService: MockProxy; let taskSchedulerService: MockProxy; let logService: MockProxy; - let biometricsService: MockProxy; + let lockService: MockProxy; let logoutService: MockProxy; - let lockedCallback: jest.Mock, [userId: string]>; let vaultTimeoutActionSubject: BehaviorSubject; let availableVaultTimeoutActionsSubject: BehaviorSubject; @@ -65,25 +41,14 @@ describe("VaultTimeoutService", () => { beforeEach(() => { accountService = mockAccountServiceWith(userId); - masterPasswordService = new FakeMasterPasswordService(); - cipherService = mock(); - folderService = mock(); - collectionService = mock(); platformUtilsService = mock(); - messagingService = mock(); - searchService = mock(); - stateService = mock(); - tokenService = mock(); authService = mock(); vaultTimeoutSettingsService = mock(); - stateEventRunnerService = mock(); taskSchedulerService = mock(); + lockService = mock(); logService = mock(); - biometricsService = mock(); logoutService = mock(); - lockedCallback = jest.fn(); - vaultTimeoutActionSubject = new BehaviorSubject(VaultTimeoutAction.Lock); vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$.mockReturnValue( @@ -94,22 +59,12 @@ describe("VaultTimeoutService", () => { vaultTimeoutService = new VaultTimeoutService( accountService, - masterPasswordService, - cipherService, - folderService, - collectionService, platformUtilsService, - messagingService, - searchService, - stateService, - tokenService, authService, vaultTimeoutSettingsService, - stateEventRunnerService, taskSchedulerService, logService, - biometricsService, - lockedCallback, + lockService, logoutService, ); }); @@ -145,9 +100,6 @@ describe("VaultTimeoutService", () => { authService.getAuthStatus.mockImplementation((userId) => { return Promise.resolve(accounts[userId]?.authStatus); }); - tokenService.hasAccessToken$.mockImplementation((userId) => { - return of(accounts[userId]?.isAuthenticated ?? false); - }); vaultTimeoutSettingsService.getVaultTimeoutByUserId$.mockImplementation((userId) => { return new BehaviorSubject(accounts[userId]?.vaultTimeout); @@ -203,13 +155,7 @@ describe("VaultTimeoutService", () => { }; const expectUserToHaveLocked = (userId: string) => { - // This does NOT assert all the things that the lock process does - expect(tokenService.hasAccessToken$).toHaveBeenCalledWith(userId); - expect(vaultTimeoutSettingsService.availableVaultTimeoutActions$).toHaveBeenCalledWith(userId); - expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, { userId: userId }); - expect(masterPasswordService.mock.clearMasterKey).toHaveBeenCalledWith(userId); - expect(cipherService.clearCache).toHaveBeenCalledWith(userId); - expect(lockedCallback).toHaveBeenCalledWith(userId); + expect(lockService.lock).toHaveBeenCalledWith(userId); }; const expectUserToHaveLoggedOut = (userId: string) => { @@ -217,7 +163,7 @@ describe("VaultTimeoutService", () => { }; const expectNoAction = (userId: string) => { - expect(lockedCallback).not.toHaveBeenCalledWith(userId); + expect(lockService.lock).not.toHaveBeenCalledWith(userId); expect(logoutService.logout).not.toHaveBeenCalledWith(userId, "vaultTimeout"); }; @@ -347,12 +293,8 @@ describe("VaultTimeoutService", () => { expectNoAction("1"); expectUserToHaveLocked("2"); - // Active users should have additional steps ran - expect(searchService.clearIndex).toHaveBeenCalled(); - expect(folderService.clearDecryptedFolderState).toHaveBeenCalled(); - expectUserToHaveLoggedOut("3"); // They have chosen logout as their action and it's available, log them out - expectUserToHaveLoggedOut("4"); // They may have had lock as their chosen action but it's not available to them so logout + expectUserToHaveLocked("4"); // They don't have lock available. But this is handled in lock service so we do not check for logout here }); it("should lock an account if they haven't been active passed their vault timeout even if a view is open when they are not the active user.", async () => { @@ -392,70 +334,4 @@ describe("VaultTimeoutService", () => { expectNoAction("1"); }); }); - - describe("lock", () => { - const setupLock = () => { - setupAccounts( - { - user1: { - authStatus: AuthenticationStatus.Unlocked, - isAuthenticated: true, - }, - user2: { - authStatus: AuthenticationStatus.Unlocked, - isAuthenticated: true, - }, - }, - { - userId: "user1", - }, - ); - }; - - it("should call state event runner with currently active user if no user passed into lock", async () => { - setupLock(); - - await vaultTimeoutService.lock(); - - expect(stateEventRunnerService.handleEvent).toHaveBeenCalledWith("lock", "user1"); - }); - - it("should call locked callback with the locking user if no userID is passed in.", async () => { - setupLock(); - - await vaultTimeoutService.lock(); - - expect(lockedCallback).toHaveBeenCalledWith("user1"); - }); - - it("should call state event runner with user passed into lock", async () => { - setupLock(); - - const user2 = "user2" as UserId; - - await vaultTimeoutService.lock(user2); - - expect(stateEventRunnerService.handleEvent).toHaveBeenCalledWith("lock", user2); - }); - - it("should call messaging service locked message with user passed into lock", async () => { - setupLock(); - - const user2 = "user2" as UserId; - - await vaultTimeoutService.lock(user2); - - expect(messagingService.send).toHaveBeenCalledWith("locked", { userId: user2 }); - }); - - it("should call locked callback with user passed into lock", async () => { - setupLock(); - - const user2 = "user2" as UserId; - - await vaultTimeoutService.lock(user2); - - expect(lockedCallback).toHaveBeenCalledWith(user2); - }); - }); }); diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts index c0fa042369..cf9ba05b7d 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts @@ -1,32 +1,18 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { combineLatest, concatMap, filter, firstValueFrom, map, timeout } from "rxjs"; +import { combineLatest, concatMap, firstValueFrom } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports -import { CollectionService } from "@bitwarden/admin-console/common"; -// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { LogoutService } from "@bitwarden/auth/common"; -// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. -// eslint-disable-next-line no-restricted-imports -import { BiometricsService } from "@bitwarden/key-management"; +import { LockService, LogoutService } from "@bitwarden/auth/common"; import { AccountService } from "../../../auth/abstractions/account.service"; import { AuthService } from "../../../auth/abstractions/auth.service"; -import { TokenService } from "../../../auth/abstractions/token.service"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { LogService } from "../../../platform/abstractions/log.service"; -import { MessagingService } from "../../../platform/abstractions/messaging.service"; import { PlatformUtilsService } from "../../../platform/abstractions/platform-utils.service"; -import { StateService } from "../../../platform/abstractions/state.service"; import { TaskSchedulerService, ScheduledTaskNames } from "../../../platform/scheduling"; -import { StateEventRunnerService } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { CipherService } from "../../../vault/abstractions/cipher.service"; -import { FolderService } from "../../../vault/abstractions/folder/folder.service.abstraction"; -import { SearchService } from "../../../vault/abstractions/search.service"; -import { InternalMasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; import { VaultTimeoutSettingsService } from "../abstractions/vault-timeout-settings.service"; import { VaultTimeoutService as VaultTimeoutServiceAbstraction } from "../abstractions/vault-timeout.service"; import { VaultTimeoutAction } from "../enums/vault-timeout-action.enum"; @@ -36,22 +22,12 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { constructor( private accountService: AccountService, - private masterPasswordService: InternalMasterPasswordServiceAbstraction, - private cipherService: CipherService, - private folderService: FolderService, - private collectionService: CollectionService, protected platformUtilsService: PlatformUtilsService, - private messagingService: MessagingService, - private searchService: SearchService, - private stateService: StateService, - private tokenService: TokenService, private authService: AuthService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, - private stateEventRunnerService: StateEventRunnerService, private taskSchedulerService: TaskSchedulerService, protected logService: LogService, - private biometricService: BiometricsService, - private lockedCallback: (userId: UserId) => Promise = null, + private lockService: LockService, private logoutService: LogoutService, ) { this.taskSchedulerService.registerTaskHandler( @@ -104,64 +80,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { ); } - async lock(userId?: UserId): Promise { - await this.biometricService.setShouldAutopromptNow(false); - - const lockingUserId = - userId ?? (await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id)))); - - const authed = await firstValueFrom(this.tokenService.hasAccessToken$(lockingUserId)); - if (!authed) { - return; - } - - const availableActions = await firstValueFrom( - this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId), - ); - const supportsLock = availableActions.includes(VaultTimeoutAction.Lock); - if (!supportsLock) { - await this.logoutService.logout(userId, "vaultTimeout"); - } - - // HACK: Start listening for the transition of the locking user from something to the locked state. - // This is very much a hack to ensure that the authentication status to retrievable right after - // it does its work. Particularly the `lockedCallback` and `"locked"` message. Instead - // lockedCallback should be deprecated and people should subscribe and react to `authStatusFor$` themselves. - const lockPromise = firstValueFrom( - this.authService.authStatusFor$(lockingUserId).pipe( - filter((authStatus) => authStatus === AuthenticationStatus.Locked), - timeout({ - first: 5_000, - with: () => { - throw new Error("The lock process did not complete in a reasonable amount of time."); - }, - }), - ), - ); - - await this.searchService.clearIndex(lockingUserId); - - await this.folderService.clearDecryptedFolderState(lockingUserId); - await this.masterPasswordService.clearMasterKey(lockingUserId); - - await this.stateService.setUserKeyAutoUnlock(null, { userId: lockingUserId }); - - await this.cipherService.clearCache(lockingUserId); - - await this.stateEventRunnerService.handleEvent("lock", lockingUserId); - - // HACK: Sit here and wait for the the auth status to transition to `Locked` - // to ensure the message and lockedCallback will get the correct status - // if/when they call it. - await lockPromise; - - this.messagingService.send("locked", { userId: lockingUserId }); - - if (this.lockedCallback != null) { - await this.lockedCallback(lockingUserId); - } - } - private async shouldLock( userId: string, lastActive: Date, @@ -206,6 +124,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { ); timeoutAction === VaultTimeoutAction.LogOut ? await this.logoutService.logout(userId, "vaultTimeout") - : await this.lock(userId); + : await this.lockService.lock(userId); } }