diff --git a/libs/angular/src/auth/guards/lock.guard.spec.ts b/libs/angular/src/auth/guards/lock.guard.spec.ts index 32b8ecbb9dd..ed77f9bdebf 100644 --- a/libs/angular/src/auth/guards/lock.guard.spec.ts +++ b/libs/angular/src/auth/guards/lock.guard.spec.ts @@ -44,7 +44,7 @@ describe("lockGuard", () => { const keyService: MockProxy = mock(); keyService.isLegacyUser.mockResolvedValue(setupParams.isLegacyUser); - keyService.everHadUserKey$ = of(setupParams.everHadUserKey); + keyService.everHadUserKey$.mockReturnValue(of(setupParams.everHadUserKey)); const platformUtilService: MockProxy = mock(); platformUtilService.getClientType.mockReturnValue(setupParams.clientType); diff --git a/libs/angular/src/auth/guards/lock.guard.ts b/libs/angular/src/auth/guards/lock.guard.ts index 10ad4917f32..01d03dc718d 100644 --- a/libs/angular/src/auth/guards/lock.guard.ts +++ b/libs/angular/src/auth/guards/lock.guard.ts @@ -84,7 +84,7 @@ export function lockGuard(): CanActivateFn { } // If authN user with TDE directly navigates to lock, reject that navigation - const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$); + const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id)); if (tdeEnabled && !everHadUserKey) { return false; } diff --git a/libs/angular/src/auth/guards/redirect.guard.ts b/libs/angular/src/auth/guards/redirect.guard.ts index 00dd20c9909..b893614b405 100644 --- a/libs/angular/src/auth/guards/redirect.guard.ts +++ b/libs/angular/src/auth/guards/redirect.guard.ts @@ -2,8 +2,10 @@ import { inject } from "@angular/core"; import { CanActivateFn, Router } from "@angular/router"; import { firstValueFrom } from "rxjs"; +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 { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { KeyService } from "@bitwarden/key-management"; @@ -33,6 +35,7 @@ export function redirectGuard(overrides: Partial = {}): CanActiv const authService = inject(AuthService); const keyService = inject(KeyService); const deviceTrustService = inject(DeviceTrustServiceAbstraction); + const accountService = inject(AccountService); const logService = inject(LogService); const router = inject(Router); @@ -49,7 +52,8 @@ export function redirectGuard(overrides: Partial = {}): CanActiv // If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the // login decryption options component. const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$); - const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$); + const userId = await firstValueFrom(accountService.activeAccount$.pipe(getUserId)); + const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId)); if (authStatus === AuthenticationStatus.Locked && tdeEnabled && !everHadUserKey) { logService.info( "Sending user to TDE decryption options. AuthStatus is %s. TDE support is %s. Ever had user key is %s.", diff --git a/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts b/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts new file mode 100644 index 00000000000..4408452a2a2 --- /dev/null +++ b/libs/angular/src/auth/guards/tde-decryption-required.guard.spec.ts @@ -0,0 +1,107 @@ +import { TestBed } from "@angular/core/testing"; +import { Router, provideRouter } from "@angular/router"; +import { mock } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; + +import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec"; +import { Account, 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 { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { KeyService } from "@bitwarden/key-management"; + +import { tdeDecryptionRequiredGuard } from "./tde-decryption-required.guard"; + +describe("tdeDecryptionRequiredGuard", () => { + const activeUser: Account = { + id: "fake_user_id" as UserId, + email: "test@email.com", + emailVerified: true, + name: "Test User", + }; + + const setup = ( + activeUser: Account | null, + authStatus: AuthenticationStatus | null = null, + tdeEnabled: boolean = false, + everHadUserKey: boolean = false, + ) => { + const accountService = mock(); + const authService = mock(); + const keyService = mock(); + const deviceTrustService = mock(); + const logService = mock(); + + accountService.activeAccount$ = new BehaviorSubject(activeUser); + if (authStatus !== null) { + authService.getAuthStatus.mockResolvedValue(authStatus); + } + keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey)); + deviceTrustService.supportsDeviceTrust$ = of(tdeEnabled); + + const testBed = TestBed.configureTestingModule({ + providers: [ + { provide: AccountService, useValue: accountService }, + { provide: AuthService, useValue: authService }, + { provide: KeyService, useValue: keyService }, + { provide: DeviceTrustServiceAbstraction, useValue: deviceTrustService }, + { provide: LogService, useValue: logService }, + provideRouter([ + { path: "", component: EmptyComponent }, + { + path: "protected-route", + component: EmptyComponent, + canActivate: [tdeDecryptionRequiredGuard()], + }, + ]), + ], + }); + + return { + router: testBed.inject(Router), + }; + }; + + it("redirects to root when the active account is null", async () => { + const { router } = setup(null, null); + await router.navigate(["protected-route"]); + expect(router.url).toBe("/"); + }); + + test.each([AuthenticationStatus.Unlocked, AuthenticationStatus.LoggedOut])( + "redirects to root when the user isn't locked", + async (authStatus) => { + const { router } = setup(activeUser, authStatus); + + await router.navigate(["protected-route"]); + + expect(router.url).toBe("/"); + }, + ); + + it("redirects to root when TDE is not enabled", async () => { + const { router } = setup(activeUser, AuthenticationStatus.Locked, false, true); + + await router.navigate(["protected-route"]); + + expect(router.url).toBe("/"); + }); + + it("redirects to root when user has had a user key", async () => { + const { router } = setup(activeUser, AuthenticationStatus.Locked, true, true); + + await router.navigate(["protected-route"]); + + expect(router.url).toBe("/"); + }); + + it("allows access when user is locked, TDE is enabled, and user has never had a user key", async () => { + const { router } = setup(activeUser, AuthenticationStatus.Locked, true, false); + + const result = await router.navigate(["protected-route"]); + expect(result).toBe(true); + expect(router.url).toBe("/protected-route"); + }); +}); diff --git a/libs/angular/src/auth/guards/tde-decryption-required.guard.ts b/libs/angular/src/auth/guards/tde-decryption-required.guard.ts index 1d98b1fa740..13e7c6d04e1 100644 --- a/libs/angular/src/auth/guards/tde-decryption-required.guard.ts +++ b/libs/angular/src/auth/guards/tde-decryption-required.guard.ts @@ -5,8 +5,9 @@ import { RouterStateSnapshot, CanActivateFn, } from "@angular/router"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; +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 { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; @@ -24,12 +25,18 @@ export function tdeDecryptionRequiredGuard(): CanActivateFn { const authService = inject(AuthService); const keyService = inject(KeyService); const deviceTrustService = inject(DeviceTrustServiceAbstraction); + const accountService = inject(AccountService); const logService = inject(LogService); const router = inject(Router); + const userId = await firstValueFrom(accountService.activeAccount$.pipe(map((a) => a?.id))); + if (userId == null) { + return router.createUrlTree(["/"]); + } + const authStatus = await authService.getAuthStatus(); const tdeEnabled = await firstValueFrom(deviceTrustService.supportsDeviceTrust$); - const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$); + const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(userId)); // We need to determine if we should bypass the decryption options and send the user to the vault. // The ONLY time that we want to send a user to the decryption options is when: diff --git a/libs/angular/src/auth/guards/unauth.guard.spec.ts b/libs/angular/src/auth/guards/unauth.guard.spec.ts index ad0ce680a1f..c696b849558 100644 --- a/libs/angular/src/auth/guards/unauth.guard.spec.ts +++ b/libs/angular/src/auth/guards/unauth.guard.spec.ts @@ -2,7 +2,7 @@ import { TestBed } from "@angular/core/testing"; import { Router } from "@angular/router"; import { RouterTestingModule } from "@angular/router/testing"; import { MockProxy, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { EmptyComponent } from "@bitwarden/angular/platform/guard/feature-flag.guard.spec"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -43,7 +43,7 @@ describe("UnauthGuard", () => { authService.authStatusFor$.mockReturnValue(activeAccountStatusObservable); } - keyService.everHadUserKey$ = new BehaviorSubject(everHadUserKey); + keyService.everHadUserKey$.mockReturnValue(of(everHadUserKey)); deviceTrustService.supportsDeviceTrustByUserId$.mockReturnValue( new BehaviorSubject(tdeEnabled), ); diff --git a/libs/angular/src/auth/guards/unauth.guard.ts b/libs/angular/src/auth/guards/unauth.guard.ts index 6764b46843e..3fcfd38349b 100644 --- a/libs/angular/src/auth/guards/unauth.guard.ts +++ b/libs/angular/src/auth/guards/unauth.guard.ts @@ -50,7 +50,7 @@ async function unauthGuard( const tdeEnabled = await firstValueFrom( deviceTrustService.supportsDeviceTrustByUserId$(activeUser.id), ); - const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$); + const everHadUserKey = await firstValueFrom(keyService.everHadUserKey$(activeUser.id)); // If locked, TDE is enabled, and the user hasn't decrypted yet, then redirect to the // login decryption options component. diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index 95b79890c6a..51a99421967 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -85,11 +85,13 @@ export abstract class KeyService { * (such as auto, biometrics, or pin) */ abstract refreshAdditionalKeys(): Promise; + /** - * Observable value that returns whether or not the currently active user has ever had auser key, + * Observable value that returns whether or not the user has ever had a userKey, * i.e. has ever been unlocked/decrypted. This is key for differentiating between TDE locked and standard locked states. */ - abstract everHadUserKey$: Observable; + abstract everHadUserKey$(userId: UserId): Observable; + /** * Retrieves the user key * @param userId The desired user diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 6d2e8fd20ec..400d7279a30 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -34,7 +34,6 @@ import { FakeAccountService, mockAccountServiceWith, FakeStateProvider, - FakeActiveUserState, FakeSingleUserState, } from "@bitwarden/common/spec"; import { CsprngArray } from "@bitwarden/common/types/csprng"; @@ -190,28 +189,28 @@ describe("keyService", () => { }); describe("everHadUserKey$", () => { - let everHadUserKeyState: FakeActiveUserState; + let everHadUserKeyState: FakeSingleUserState; beforeEach(() => { - everHadUserKeyState = stateProvider.activeUser.getFake(USER_EVER_HAD_USER_KEY); + everHadUserKeyState = stateProvider.singleUser.getFake(mockUserId, USER_EVER_HAD_USER_KEY); }); it("should return true when stored value is true", async () => { everHadUserKeyState.nextState(true); - expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(true); + expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(true); }); it("should return false when stored value is false", async () => { everHadUserKeyState.nextState(false); - expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false); + expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false); }); it("should return false when stored value is null", async () => { everHadUserKeyState.nextState(null); - expect(await firstValueFrom(keyService.everHadUserKey$)).toBe(false); + expect(await firstValueFrom(keyService.everHadUserKey$(mockUserId))).toBe(false); }); }); diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 1d4fcc86a0c..fe288adeb88 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -41,7 +41,7 @@ import { USER_EVER_HAD_USER_KEY, USER_KEY, } from "@bitwarden/common/platform/services/key-state/user-key.state"; -import { ActiveUserState, StateProvider } from "@bitwarden/common/platform/state"; +import { StateProvider } from "@bitwarden/common/platform/state"; import { CsprngArray } from "@bitwarden/common/types/csprng"; import { OrganizationId, ProviderId, UserId } from "@bitwarden/common/types/guid"; import { @@ -63,10 +63,6 @@ import { import { KdfConfig } from "./models/kdf-config"; export class DefaultKeyService implements KeyServiceAbstraction { - private readonly activeUserEverHadUserKey: ActiveUserState; - - readonly everHadUserKey$: Observable; - readonly activeUserOrgKeys$: Observable>; constructor( @@ -82,10 +78,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { protected stateProvider: StateProvider, protected kdfConfigService: KdfConfigService, ) { - // User Key - this.activeUserEverHadUserKey = stateProvider.getActive(USER_EVER_HAD_USER_KEY); - this.everHadUserKey$ = this.activeUserEverHadUserKey.state$.pipe(map((x) => x ?? false)); - this.activeUserOrgKeys$ = this.stateProvider.activeUserId$.pipe( switchMap((userId) => (userId != null ? this.orgKeys$(userId) : NEVER)), ) as Observable>; @@ -141,6 +133,12 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.setUserKey(key, activeUserId); } + everHadUserKey$(userId: UserId): Observable { + return this.stateProvider + .getUser(userId, USER_EVER_HAD_USER_KEY) + .state$.pipe(map((x) => x ?? false)); + } + getInMemoryUserKeyFor$(userId: UserId): Observable { return this.stateProvider.getUserState$(USER_KEY, userId); }