1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-12 06:13:38 +00:00

Only Keep Active User Alive When A View is Open (#7045)

This commit is contained in:
Justin Baur
2023-12-05 10:01:41 -05:00
committed by GitHub
parent 2fd30304d3
commit 3deb6ea0c8
2 changed files with 75 additions and 36 deletions

View File

@@ -91,7 +91,10 @@ describe("VaultTimeoutService", () => {
availableTimeoutActions?: VaultTimeoutAction[]; availableTimeoutActions?: VaultTimeoutAction[];
} }
>, >,
userId?: string, globalSetups?: {
userId?: string;
isViewOpen?: boolean;
},
) => { ) => {
// Both are available by default and the specific test can change this per test // Both are available by default and the specific test can change this per test
availableVaultTimeoutActionsSubject.next([VaultTimeoutAction.Lock, VaultTimeoutAction.LogOut]); availableVaultTimeoutActionsSubject.next([VaultTimeoutAction.Lock, VaultTimeoutAction.LogOut]);
@@ -111,7 +114,11 @@ describe("VaultTimeoutService", () => {
return Promise.resolve(accounts[options.userId]?.lastActive); return Promise.resolve(accounts[options.userId]?.lastActive);
}); });
stateService.getUserId.mockResolvedValue(userId); stateService.getUserId.mockResolvedValue(globalSetups?.userId);
stateService.activeAccount$ = new BehaviorSubject<string>(globalSetups?.userId);
platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false);
vaultTimeoutSettingsService.vaultTimeoutAction$.mockImplementation((userId) => { vaultTimeoutSettingsService.vaultTimeoutAction$.mockImplementation((userId) => {
return new BehaviorSubject<VaultTimeoutAction>(accounts[userId]?.timeoutAction); return new BehaviorSubject<VaultTimeoutAction>(accounts[userId]?.timeoutAction);
@@ -217,22 +224,25 @@ describe("VaultTimeoutService", () => {
it("should lock an account that isn't active and has immediate as their timeout when view is not open", async () => { it("should lock an account that isn't active and has immediate as their timeout when view is not open", async () => {
// Arrange // Arrange
platformUtilsService.isViewOpen.mockResolvedValue(false); setupAccounts(
{
setupAccounts({ 1: {
1: { authStatus: AuthenticationStatus.Unlocked,
authStatus: AuthenticationStatus.Unlocked, isAuthenticated: true,
isAuthenticated: true, vaultTimeout: 0, // Immediately
vaultTimeout: 0, // Immediately lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago },
2: {
authStatus: AuthenticationStatus.Unlocked,
isAuthenticated: true,
vaultTimeout: 1, // One minute
lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
},
}, },
2: { {
authStatus: AuthenticationStatus.Unlocked, isViewOpen: false,
isAuthenticated: true,
vaultTimeout: 1, // One minute
lastActive: new Date().getTime() - 10 * 1000, // Last active 10 seconds ago
}, },
}); );
// Act // Act
await vaultTimeoutService.checkVaultTimeout(); await vaultTimeoutService.checkVaultTimeout();
@@ -243,8 +253,6 @@ describe("VaultTimeoutService", () => {
}); });
it("should run action on an account that hasn't been active for greater than 1 minute and has a vault timeout for 1 minutes", async () => { it("should run action on an account that hasn't been active for greater than 1 minute and has a vault timeout for 1 minutes", async () => {
platformUtilsService.isViewOpen.mockResolvedValue(false);
setupAccounts( setupAccounts(
{ {
1: { 1: {
@@ -276,7 +284,7 @@ describe("VaultTimeoutService", () => {
availableTimeoutActions: [VaultTimeoutAction.LogOut], availableTimeoutActions: [VaultTimeoutAction.LogOut],
}, },
}, },
"2", // Treat user 2 as the active user { userId: "2", isViewOpen: false }, // Treat user 2 as the active user
); );
await vaultTimeoutService.checkVaultTimeout(); await vaultTimeoutService.checkVaultTimeout();
@@ -292,18 +300,37 @@ describe("VaultTimeoutService", () => {
expectUserToHaveLoggedOut("4"); // They may have had lock as their chosen action but it's not available to them so logout expectUserToHaveLoggedOut("4"); // They may have had lock as their chosen action but it's not available to them so logout
}); });
it("should not lock any accounts as long as a view is known to be open, no matter if they haven't been active since before their timeout", async () => { 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 () => {
platformUtilsService.isViewOpen.mockResolvedValue(true); setupAccounts(
{
setupAccounts({ 1: {
1: { // Neither of these setup values ever get called
// Neither of these setup values ever get called authStatus: AuthenticationStatus.Unlocked,
authStatus: AuthenticationStatus.Unlocked, isAuthenticated: true,
isAuthenticated: true, lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago
lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago vaultTimeout: 1, // Vault timeout of 1 minute
vaultTimeout: 1, // Vault timeout of 1 minute ago },
}, },
}); { userId: "2", isViewOpen: true },
);
await vaultTimeoutService.checkVaultTimeout();
expectUserToHaveLocked("1");
});
it("should not lock an account that is active and we know that a view is open, even if they haven't been active passed their timeout", async () => {
setupAccounts(
{
1: {
authStatus: AuthenticationStatus.Unlocked,
isAuthenticated: true,
lastActive: new Date().getTime() - 80 * 1000, // Last active 80 seconds ago
vaultTimeout: 1, // Vault timeout of 1 minute
},
},
{ userId: "1", isViewOpen: true }, // They are the currently active user
);
await vaultTimeoutService.checkVaultTimeout(); await vaultTimeoutService.checkVaultTimeout();

View File

@@ -1,4 +1,4 @@
import { firstValueFrom } from "rxjs"; import { firstValueFrom, timeout } from "rxjs";
import { SearchService } from "../../abstractions/search.service"; import { SearchService } from "../../abstractions/search.service";
import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service";
@@ -52,13 +52,14 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
} }
async checkVaultTimeout(): Promise<void> { async checkVaultTimeout(): Promise<void> {
if (await this.platformUtilsService.isViewOpen()) { // Get whether or not the view is open a single time so it can be compared for each user
return; const isViewOpen = await this.platformUtilsService.isViewOpen();
}
const activeUserId = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500)));
const accounts = await firstValueFrom(this.stateService.accounts$); const accounts = await firstValueFrom(this.stateService.accounts$);
for (const userId in accounts) { for (const userId in accounts) {
if (userId != null && (await this.shouldLock(userId))) { if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) {
await this.executeTimeoutAction(userId); await this.executeTimeoutAction(userId);
} }
} }
@@ -108,7 +109,18 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
} }
} }
private async shouldLock(userId: string): Promise<boolean> { private async shouldLock(
userId: string,
activeUserId: string,
isViewOpen: boolean,
): Promise<boolean> {
if (isViewOpen && userId === activeUserId) {
// We know a view is open and this is the currently active user
// which means they are likely looking at their vault
// and they should not lock.
return false;
}
const authStatus = await this.authService.getAuthStatus(userId); const authStatus = await this.authService.getAuthStatus(userId);
if ( if (
authStatus === AuthenticationStatus.Locked || authStatus === AuthenticationStatus.Locked ||