bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00
Code Issues Releases Wiki Activity

[PM-25206] Inject service instead of passing as param (#16801)

Browse Source 
* Inject service instead of passing as param

* [PM-25206] Move locking logic to LockService (#16802)

* Move locking logic to lock service

* Fix tests

* Fix CLI

* Fix test

* FIx safari build

* Update call to lock service

* Remove locked callback

* Clean up lock service logic

* Add tests

* Fix cli build

* Add extension lock service

* Fix cli build

* Fix build

* Undo ac changes

* Undo ac changes

* Run prettier

* Fix build

* Remove duplicate call

* [PM-25206] Remove VaultTimeoutService lock logic (#16804)

* Move consumers off of vaulttimeoutsettingsservice lock

* Fix build

* Fix build

* Fix build

* Fix firefox build

* Fix test

* Fix ts strict errors

* Fix ts strict error

* Undo AC changes

* Cleanup

* Fix

* Fix missing service
This commit is contained in:
Bernd Schoolmann
2025-11-05 17:11:34 +01:00
committed by GitHub
parent 993637c41e
commit 3125f679d3
31 changed files with 508 additions and 365 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
libs/common/src/key-management/abstractions/process-reload.service.ts
View File

@@ -1,6 +1,4 @@
import { AuthService } from "../../auth/abstractions/auth.service";
export abstract class ProcessReloadServiceAbstraction {
abstract startProcessReload(authService: AuthService): Promise<void>;
abstract startProcessReload(): Promise<void>;
abstract cancelProcessReload(): void;
}

7
libs/common/src/key-management/services/default-process-reload.service.ts
View File

@@ -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<void> {
async startProcessReload(): Promise<void> {
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",

1
libs/common/src/key-management/vault-timeout/abstractions/vault-timeout.service.ts
View File

@@ -1,4 +1,3 @@
export abstract class VaultTimeoutService {
abstract checkVaultTimeout(): Promise<void>;
abstract lock(userId?: string): Promise<void>;
}

138
libs/common/src/key-management/vault-timeout/services/vault-timeout.service.spec.ts
View File

@@ -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<CipherService>;
let folderService: MockProxy<FolderService>;
let collectionService: MockProxy<CollectionService>;
let platformUtilsService: MockProxy<PlatformUtilsService>;
let messagingService: MockProxy<MessagingService>;
let searchService: MockProxy<SearchService>;
let stateService: MockProxy<StateService>;
let tokenService: MockProxy<TokenService>;
let authService: MockProxy<AuthService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let stateEventRunnerService: MockProxy<StateEventRunnerService>;
let taskSchedulerService: MockProxy<TaskSchedulerService>;
let logService: MockProxy<LogService>;
let biometricsService: MockProxy<BiometricsService>;
let lockService: MockProxy<LockService>;
let logoutService: MockProxy<LogoutService>;
let lockedCallback: jest.Mock<Promise<void>, [userId: string]>;
let vaultTimeoutActionSubject: BehaviorSubject<VaultTimeoutAction>;
let availableVaultTimeoutActionsSubject: BehaviorSubject<VaultTimeoutAction[]>;
@@ -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<TaskSchedulerService>();
lockService = mock<LockService>();
logService = mock<LogService>();
biometricsService = mock<BiometricsService>();
logoutService = mock<LogoutService>();
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<VaultTimeout>(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);
});
});
});

90
libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts
View File

@@ -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<void> = null,
private lockService: LockService,
private logoutService: LogoutService,
) {
this.taskSchedulerService.registerTaskHandler(
@@ -104,64 +80,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
);
}
async lock(userId?: UserId): Promise<void> {
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);
}
}