-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auth/PM-7811 - Refactor User Auto Unlock Key Hydration Process To Rem…
…ove Race Conditions (#8979) * PM-7811 - Refactor UserKeyInitService to UserAutoUnlockKeyService - remove active account listening logic as it introduced race conditions with user key memory retrieval happening before the user auto unlock key was set into memory. * PM-7811 - CLI - (1) Fix deps (2) On CLI init (pre command execution), if there is an active account, then set the user key in memory from the user auto unlock key. * PM-7811 - Browser Extension / desktop - (1) Update deps (2) Sets user key in memory if the auto unlock key is set on account switch and background init (must act on all accounts so that account switcher displays unlock status properly). * PM-7811 - Web - (1) Update deps (2) Sets user key in memory if the auto unlock key is set on init * PM-7811 - Fix account switcher service changes not being necessary.
- Loading branch information
1 parent
24a00f1
commit 128c909
Showing
9 changed files
with
165 additions
and
246 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
libs/common/src/platform/services/user-auto-unlock-key.service.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { mock } from "jest-mock-extended"; | ||
|
|
||
| import { CsprngArray } from "../../types/csprng"; | ||
| import { UserId } from "../../types/guid"; | ||
| import { UserKey } from "../../types/key"; | ||
| import { KeySuffixOptions } from "../enums"; | ||
| import { Utils } from "../misc/utils"; | ||
| import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; | ||
|
|
||
| import { CryptoService } from "./crypto.service"; | ||
| import { UserAutoUnlockKeyService } from "./user-auto-unlock-key.service"; | ||
|
|
||
| describe("UserAutoUnlockKeyService", () => { | ||
| let userAutoUnlockKeyService: UserAutoUnlockKeyService; | ||
|
|
||
| const mockUserId = Utils.newGuid() as UserId; | ||
|
|
||
| const cryptoService = mock<CryptoService>(); | ||
|
|
||
| beforeEach(() => { | ||
| userAutoUnlockKeyService = new UserAutoUnlockKeyService(cryptoService); | ||
| }); | ||
|
|
||
| describe("setUserKeyInMemoryIfAutoUserKeySet", () => { | ||
| it("does nothing if the userId is null", async () => { | ||
| // Act | ||
| await (userAutoUnlockKeyService as any).setUserKeyInMemoryIfAutoUserKeySet(null); | ||
|
|
||
| // Assert | ||
| expect(cryptoService.getUserKeyFromStorage).not.toHaveBeenCalled(); | ||
| expect(cryptoService.setUserKey).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("does nothing if the autoUserKey is null", async () => { | ||
| // Arrange | ||
| const userId = mockUserId; | ||
|
|
||
| cryptoService.getUserKeyFromStorage.mockResolvedValue(null); | ||
|
|
||
| // Act | ||
| await (userAutoUnlockKeyService as any).setUserKeyInMemoryIfAutoUserKeySet(userId); | ||
|
|
||
| // Assert | ||
| expect(cryptoService.getUserKeyFromStorage).toHaveBeenCalledWith( | ||
| KeySuffixOptions.Auto, | ||
| userId, | ||
| ); | ||
| expect(cryptoService.setUserKey).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("sets the user key in memory if the autoUserKey is not null", async () => { | ||
| // Arrange | ||
| const userId = mockUserId; | ||
|
|
||
| const mockRandomBytes = new Uint8Array(64) as CsprngArray; | ||
| const mockAutoUserKey: UserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; | ||
|
|
||
| cryptoService.getUserKeyFromStorage.mockResolvedValue(mockAutoUserKey); | ||
|
|
||
| // Act | ||
| await (userAutoUnlockKeyService as any).setUserKeyInMemoryIfAutoUserKeySet(userId); | ||
|
|
||
| // Assert | ||
| expect(cryptoService.getUserKeyFromStorage).toHaveBeenCalledWith( | ||
| KeySuffixOptions.Auto, | ||
| userId, | ||
| ); | ||
| expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockAutoUserKey, userId); | ||
| }); | ||
| }); | ||
| }); |
36 changes: 36 additions & 0 deletions
36
libs/common/src/platform/services/user-auto-unlock-key.service.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { UserId } from "../../types/guid"; | ||
| import { CryptoService } from "../abstractions/crypto.service"; | ||
| import { KeySuffixOptions } from "../enums"; | ||
|
|
||
| // TODO: this is a half measure improvement which allows us to reduce some side effects today (cryptoService.getUserKey setting user key in memory if auto key exists) | ||
| // but ideally, in the future, we would be able to put this logic into the cryptoService | ||
| // after the vault timeout settings service is transitioned to state provider so that | ||
| // the getUserKey logic can simply go to the correct location based on the vault timeout settings | ||
| // similar to the TokenService (it would either go to secure storage for the auto user key or memory for the user key) | ||
|
|
||
| export class UserAutoUnlockKeyService { | ||
| constructor(private cryptoService: CryptoService) {} | ||
|
|
||
| /** | ||
| * The presence of the user key in memory dictates whether the user's vault is locked or unlocked. | ||
| * However, for users that have the auto unlock user key set, we need to set the user key in memory | ||
| * on application bootstrap and on active account changes so that the user's vault loads unlocked. | ||
| * @param userId - The user id to check for an auto user key. | ||
| */ | ||
| async setUserKeyInMemoryIfAutoUserKeySet(userId: UserId): Promise<void> { | ||
| if (userId == null) { | ||
| return; | ||
| } | ||
|
|
||
| const autoUserKey = await this.cryptoService.getUserKeyFromStorage( | ||
| KeySuffixOptions.Auto, | ||
| userId, | ||
| ); | ||
|
|
||
| if (autoUserKey == null) { | ||
| return; | ||
| } | ||
|
|
||
| await this.cryptoService.setUserKey(autoUserKey, userId); | ||
| } | ||
| } |
Oops, something went wrong.