[PM-12024] Move Lock All To Happen in Background (#11047)

* Move Lock All To Happen in Background

- Make it done serially
- Have the promise only resolve once it's complete

* Unlock Active Account Last

* Add Tests

* Update Comment
This commit is contained in:
Justin Baur 2024-09-16 16:08:03 -04:00 committed by GitHub
parent 112bad03b1
commit 1ebef296b9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 150 additions and 22 deletions

View File

@ -1,9 +1,10 @@
import { CommonModule, Location } from "@angular/common"; import { CommonModule, Location } from "@angular/common";
import { Component, OnDestroy, OnInit } from "@angular/core"; import { Component, OnDestroy, OnInit } from "@angular/core";
import { Router } from "@angular/router"; import { Router } from "@angular/router";
import { Subject, firstValueFrom, map, of, startWith, switchMap, takeUntil } from "rxjs"; import { Subject, firstValueFrom, map, of, startWith, switchMap } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { LockService } from "@bitwarden/auth/common";
import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@ -70,6 +71,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
private authService: AuthService, private authService: AuthService,
private configService: ConfigService, private configService: ConfigService,
private lockService: LockService,
) {} ) {}
get accountLimit() { get accountLimit() {
@ -131,26 +133,8 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy {
async lockAll() { async lockAll() {
this.loading = true; this.loading = true;
this.availableAccounts$ await this.lockService.lockAll();
.pipe( await this.router.navigate(["lock"]);
map((accounts) =>
accounts
.filter((account) => account.id !== this.specialAddAccountId)
.sort((a, b) => (a.isActive ? -1 : b.isActive ? 1 : 0)) // Log out of the active account first
.map((account) => account.id),
),
switchMap(async (accountIds) => {
if (accountIds.length === 0) {
return;
}
// Must lock active (first) account first, then order doesn't matter
await this.vaultTimeoutService.lock(accountIds.shift());
await Promise.all(accountIds.map((id) => this.vaultTimeoutService.lock(id)));
}),
takeUntil(this.destroy$),
)
.subscribe(() => this.router.navigate(["lock"]));
} }
async logOut(userId: UserId) { async logOut(userId: UserId) {

View File

@ -0,0 +1,32 @@
import { filter, firstValueFrom } from "rxjs";
import { LockService } from "@bitwarden/auth/common";
import {
CommandDefinition,
MessageListener,
MessageSender,
} from "@bitwarden/common/platform/messaging";
import { Utils } from "@bitwarden/common/platform/misc/utils";
const LOCK_ALL_FINISHED = new CommandDefinition<{ requestId: string }>("lockAllFinished");
const LOCK_ALL = new CommandDefinition<{ requestId: string }>("lockAll");
export class ForegroundLockService implements LockService {
constructor(
private readonly messageSender: MessageSender,
private readonly messageListener: MessageListener,
) {}
async lockAll(): Promise<void> {
const requestId = Utils.newGuid();
const finishMessage = firstValueFrom(
this.messageListener
.messages$(LOCK_ALL_FINISHED)
.pipe(filter((m) => m.requestId === requestId)),
);
this.messageSender.send(LOCK_ALL, { requestId });
await finishMessage;
}
}

View File

@ -9,6 +9,7 @@ import {
AuthRequestService, AuthRequestService,
LoginEmailServiceAbstraction, LoginEmailServiceAbstraction,
LogoutReason, LogoutReason,
DefaultLockService,
} from "@bitwarden/auth/common"; } from "@bitwarden/auth/common";
import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service";
import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service";
@ -1065,6 +1066,9 @@ export default class MainBackground {
this.scriptInjectorService, this.scriptInjectorService,
this.configService, this.configService,
); );
const lockService = new DefaultLockService(this.accountService, this.vaultTimeoutService);
this.runtimeBackground = new RuntimeBackground( this.runtimeBackground = new RuntimeBackground(
this, this,
this.autofillService, this.autofillService,
@ -1079,6 +1083,7 @@ export default class MainBackground {
this.fido2Background, this.fido2Background,
messageListener, messageListener,
this.accountService, this.accountService,
lockService,
); );
this.nativeMessagingBackground = new NativeMessagingBackground( this.nativeMessagingBackground = new NativeMessagingBackground(
this.cryptoService, this.cryptoService,

View File

@ -1,5 +1,6 @@
import { firstValueFrom, map, mergeMap, of, switchMap } from "rxjs"; import { firstValueFrom, map, mergeMap, of, switchMap } from "rxjs";
import { LockService } from "@bitwarden/auth/common";
import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AutofillOverlayVisibility, ExtensionCommand } from "@bitwarden/common/autofill/constants"; import { AutofillOverlayVisibility, ExtensionCommand } from "@bitwarden/common/autofill/constants";
@ -48,6 +49,7 @@ export default class RuntimeBackground {
private fido2Background: Fido2Background, private fido2Background: Fido2Background,
private messageListener: MessageListener, private messageListener: MessageListener,
private accountService: AccountService, private accountService: AccountService,
private readonly lockService: LockService,
) { ) {
// onInstalled listener must be wired up before anything else, so we do it in the ctor // onInstalled listener must be wired up before anything else, so we do it in the ctor
chrome.runtime.onInstalled.addListener((details: any) => { chrome.runtime.onInstalled.addListener((details: any) => {
@ -245,6 +247,12 @@ export default class RuntimeBackground {
case "lockVault": case "lockVault":
await this.main.vaultTimeoutService.lock(msg.userId); await this.main.vaultTimeoutService.lock(msg.userId);
break; break;
case "lockAll":
{
await this.lockService.lockAll();
this.messagingService.send("lockAllFinished", { requestId: msg.requestId });
}
break;
case "logout": case "logout":
await this.main.logout(msg.expired, msg.userId); await this.main.logout(msg.expired, msg.userId);
break; break;

View File

@ -17,7 +17,7 @@ import {
} from "@bitwarden/angular/services/injection-tokens"; } from "@bitwarden/angular/services/injection-tokens";
import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module";
import { AnonLayoutWrapperDataService } from "@bitwarden/auth/angular"; import { AnonLayoutWrapperDataService } from "@bitwarden/auth/angular";
import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { LockService, PinServiceAbstraction } from "@bitwarden/auth/common";
import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service";
import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service";
import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
@ -91,6 +91,7 @@ import { TotpService } from "@bitwarden/common/vault/services/totp.service";
import { DialogService, ToastService } from "@bitwarden/components"; import { DialogService, ToastService } from "@bitwarden/components";
import { PasswordRepromptService } from "@bitwarden/vault"; import { PasswordRepromptService } from "@bitwarden/vault";
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 { ExtensionAnonLayoutWrapperDataService } from "../../auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper-data.service";
import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service"; import { AutofillService as AutofillServiceAbstraction } from "../../autofill/services/abstractions/autofill.service";
import AutofillService from "../../autofill/services/autofill.service"; import AutofillService from "../../autofill/services/autofill.service";
@ -560,6 +561,11 @@ const safeProviders: SafeProvider[] = [
useClass: ExtensionAnonLayoutWrapperDataService, useClass: ExtensionAnonLayoutWrapperDataService,
deps: [], deps: [],
}), }),
safeProvider({
provide: LockService,
useClass: ForegroundLockService,
deps: [MessageSender, MessageListener],
}),
]; ];
@NgModule({ @NgModule({

View File

@ -0,0 +1,49 @@
import { combineLatest, firstValueFrom, map } from "rxjs";
import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { UserId } from "@bitwarden/common/types/guid";
export abstract class LockService {
/**
* Locks all accounts.
*/
abstract lockAll(): Promise<void>;
}
export class DefaultLockService implements LockService {
constructor(
private readonly accountService: AccountService,
private readonly vaultTimeoutService: VaultTimeoutService,
) {}
async lockAll() {
const accounts = await firstValueFrom(
combineLatest([this.accountService.activeAccount$, this.accountService.accounts$]).pipe(
map(([activeAccount, accounts]) => {
const otherAccounts = Object.keys(accounts) as UserId[];
if (activeAccount == null) {
return { activeAccount: null, otherAccounts: otherAccounts };
}
return {
activeAccount: activeAccount.id,
otherAccounts: otherAccounts.filter((accountId) => accountId !== activeAccount.id),
};
}),
),
);
for (const otherAccount of accounts.otherAccounts) {
await this.vaultTimeoutService.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);
}
}
}

View File

@ -0,0 +1,43 @@
import { mock } from "jest-mock-extended";
import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service";
import { mockAccountServiceWith } from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid";
import { DefaultLockService } from "./lock.service";
describe("DefaultLockService", () => {
const mockUser1 = "user1" as UserId;
const mockUser2 = "user2" as UserId;
const mockUser3 = "user3" as UserId;
const accountService = mockAccountServiceWith(mockUser1);
const vaultTimeoutService = mock<VaultTimeoutService>();
const sut = new DefaultLockService(accountService, vaultTimeoutService);
describe("lockAll", () => {
it("locks the active account last", async () => {
await accountService.addAccount(mockUser2, {
name: "name2",
email: "email2@example.com",
emailVerified: false,
});
await accountService.addAccount(mockUser3, {
name: "name3",
email: "email3@example.com",
emailVerified: false,
});
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);
// Active user should be called last
expect(vaultTimeoutService.lock).toHaveBeenNthCalledWith(3, mockUser1);
});
});
});

View File

@ -4,3 +4,4 @@ export * from "./login-strategies/login-strategy.service";
export * from "./user-decryption-options/user-decryption-options.service"; export * from "./user-decryption-options/user-decryption-options.service";
export * from "./auth-request/auth-request.service"; export * from "./auth-request/auth-request.service";
export * from "./register-route.service"; export * from "./register-route.service";
export * from "./accounts/lock.service";