From 8d698d9d84ee1a959fed1cd593fd144e7f859bbc Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Fri, 12 Apr 2024 02:25:45 -0500 Subject: [PATCH] [PM-7169][PM-5267] Remove auth status from account info (#8539) * remove active account unlocked from state service * Remove status from account service `AccountInfo` * Fixup lingering usages of status Fixup missed factories * Fixup account info usage * fixup CLI build * Fixup current account type * Add helper for all auth statuses to auth service * Fix tests * Uncomment mistakenly commented code * Rework logged out account exclusion tests * Correct test description * Avoid getters returning observables * fixup type --- .../account-switcher.component.ts | 17 +-- .../current-account.component.ts | 7 +- .../services/account-switcher.service.spec.ts | 69 ++++++++--- .../services/account-switcher.service.ts | 21 ++-- .../browser/src/background/main.background.ts | 4 +- .../event-collection-service.factory.ts | 10 +- .../event-upload-service.factory.ts | 10 +- .../src/platform/popup/header.component.ts | 16 ++- apps/browser/src/popup/app.component.ts | 6 +- apps/cli/src/bw.ts | 4 +- libs/angular/src/auth/guards/unauth.guard.ts | 11 +- .../src/services/jslib-services.module.ts | 4 +- .../user-decryption-options.service.spec.ts | 2 - libs/common/spec/fake-account-service.ts | 14 +-- .../src/auth/abstractions/account.service.ts | 24 +--- .../src/auth/abstractions/auth.service.ts | 2 + .../src/auth/services/account.service.spec.ts | 109 +++--------------- .../src/auth/services/account.service.ts | 31 ----- .../src/auth/services/auth.service.spec.ts | 19 +++ libs/common/src/auth/services/auth.service.ts | 21 ++++ ...-enrollment.service.implementation.spec.ts | 2 - .../platform/abstractions/state.service.ts | 4 - .../platform/services/crypto.service.spec.ts | 27 ----- .../src/platform/services/crypto.service.ts | 6 +- .../default-environment.service.spec.ts | 4 - .../src/platform/services/state.service.ts | 26 +---- .../default-active-user-state.spec.ts | 2 - .../event/event-collection.service.ts | 10 +- .../services/event/event-upload.service.ts | 19 +-- .../vault-timeout.service.spec.ts | 1 - .../tools/send/services/send.service.spec.ts | 2 - 31 files changed, 200 insertions(+), 304 deletions(-) diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts index 77d9741056..9a0423fca3 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.ts @@ -6,6 +6,7 @@ import { Subject, firstValueFrom, map, switchMap, takeUntil } from "rxjs"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; 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 { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -32,6 +33,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private location: Location, private router: Router, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private authService: AuthService, ) {} get accountLimit() { @@ -42,13 +44,14 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { return this.accountSwitcherService.SPECIAL_ADD_ACCOUNT_ID; } - get availableAccounts$() { - return this.accountSwitcherService.availableAccounts$; - } - - get currentAccount$() { - return this.accountService.activeAccount$; - } + readonly availableAccounts$ = this.accountSwitcherService.availableAccounts$; + readonly currentAccount$ = this.accountService.activeAccount$.pipe( + switchMap((a) => + a == null + ? null + : this.authService.activeAccountStatus$.pipe(map((s) => ({ ...a, status: s }))), + ), + ); async ngOnInit() { const availableVaultTimeoutActions = await firstValueFrom( diff --git a/apps/browser/src/auth/popup/account-switching/current-account.component.ts b/apps/browser/src/auth/popup/account-switching/current-account.component.ts index 1c7f93bf30..643c37b9aa 100644 --- a/apps/browser/src/auth/popup/account-switching/current-account.component.ts +++ b/apps/browser/src/auth/popup/account-switching/current-account.component.ts @@ -4,6 +4,7 @@ import { ActivatedRoute, Router } from "@angular/router"; import { Observable, combineLatest, switchMap } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { UserId } from "@bitwarden/common/types/guid"; @@ -29,12 +30,14 @@ export class CurrentAccountComponent { private router: Router, private location: Location, private route: ActivatedRoute, + private authService: AuthService, ) { this.currentAccount$ = combineLatest([ this.accountService.activeAccount$, this.avatarService.avatarColor$, + this.authService.activeAccountStatus$, ]).pipe( - switchMap(async ([account, avatarColor]) => { + switchMap(async ([account, avatarColor, accountStatus]) => { if (account == null) { return null; } @@ -42,7 +45,7 @@ export class CurrentAccountComponent { id: account.id, name: account.name || account.email, email: account.email, - status: account.status, + status: accountStatus, avatarColor, }; diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index f02a8ee201..fe04bee20e 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -1,7 +1,8 @@ import { matches, mock } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom, of, timeout } from "rxjs"; +import { BehaviorSubject, ReplaySubject, firstValueFrom, of, timeout } from "rxjs"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -12,22 +13,29 @@ import { UserId } from "@bitwarden/common/types/guid"; import { AccountSwitcherService } from "./account-switcher.service"; describe("AccountSwitcherService", () => { - const accountsSubject = new BehaviorSubject>(null); - const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>(null); + let accountsSubject: BehaviorSubject>; + let activeAccountSubject: BehaviorSubject<{ id: UserId } & AccountInfo>; + let authStatusSubject: ReplaySubject>; const accountService = mock(); const avatarService = mock(); const messagingService = mock(); const environmentService = mock(); const logService = mock(); + const authService = mock(); let accountSwitcherService: AccountSwitcherService; beforeEach(() => { jest.resetAllMocks(); + accountsSubject = new BehaviorSubject>(null); + activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>(null); + authStatusSubject = new ReplaySubject>(1); + // Use subject to allow for easy updates accountService.accounts$ = accountsSubject; accountService.activeAccount$ = activeAccountSubject; + authService.authStatuses$ = authStatusSubject; accountSwitcherService = new AccountSwitcherService( accountService, @@ -35,48 +43,59 @@ describe("AccountSwitcherService", () => { messagingService, environmentService, logService, + authService, ); }); + afterEach(() => { + accountsSubject.complete(); + activeAccountSubject.complete(); + authStatusSubject.complete(); + }); + describe("availableAccounts$", () => { - it("should return all accounts and an add account option when accounts are less than 5", async () => { - const user1AccountInfo: AccountInfo = { + it("should return all logged in accounts and an add account option when accounts are less than 5", async () => { + const accountInfo: AccountInfo = { name: "Test User 1", email: "test1@email.com", - status: AuthenticationStatus.Unlocked, }; avatarService.getUserAvatarColor$.mockReturnValue(of("#cccccc")); - accountsSubject.next({ - "1": user1AccountInfo, - } as Record); - - activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "1" as UserId })); + accountsSubject.next({ ["1" as UserId]: accountInfo, ["2" as UserId]: accountInfo }); + authStatusSubject.next({ + ["1" as UserId]: AuthenticationStatus.Unlocked, + ["2" as UserId]: AuthenticationStatus.Locked, + }); + activeAccountSubject.next(Object.assign(accountInfo, { id: "1" as UserId })); const accounts = await firstValueFrom( accountSwitcherService.availableAccounts$.pipe(timeout(20)), ); - expect(accounts).toHaveLength(2); + expect(accounts).toHaveLength(3); expect(accounts[0].id).toBe("1"); expect(accounts[0].isActive).toBeTruthy(); - - expect(accounts[1].id).toBe("addAccount"); + expect(accounts[1].id).toBe("2"); expect(accounts[1].isActive).toBeFalsy(); + + expect(accounts[2].id).toBe("addAccount"); + expect(accounts[2].isActive).toBeFalsy(); }); it.each([5, 6])( "should return only accounts if there are %i accounts", async (numberOfAccounts) => { const seedAccounts: Record = {}; + const seedStatuses: Record = {}; for (let i = 0; i < numberOfAccounts; i++) { seedAccounts[`${i}` as UserId] = { email: `test${i}@email.com`, name: "Test User ${i}", - status: AuthenticationStatus.Unlocked, }; + seedStatuses[`${i}` as UserId] = AuthenticationStatus.Unlocked; } avatarService.getUserAvatarColor$.mockReturnValue(of("#cccccc")); accountsSubject.next(seedAccounts); + authStatusSubject.next(seedStatuses); activeAccountSubject.next( Object.assign(seedAccounts["1" as UserId], { id: "1" as UserId }), ); @@ -89,6 +108,26 @@ describe("AccountSwitcherService", () => { }); }, ); + + it("excludes logged out accounts", async () => { + const user1AccountInfo: AccountInfo = { + name: "Test User 1", + email: "", + }; + accountsSubject.next({ ["1" as UserId]: user1AccountInfo }); + authStatusSubject.next({ ["1" as UserId]: AuthenticationStatus.LoggedOut }); + accountsSubject.next({ + "1": user1AccountInfo, + } as Record); + + const accounts = await firstValueFrom( + accountSwitcherService.availableAccounts$.pipe(timeout(20)), + ); + + // Add account only + expect(accounts).toHaveLength(1); + expect(accounts[0].id).toBe("addAccount"); + }); }); describe("selectAccount", () => { diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts index 32ebee7c75..a73ec3e1f6 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -11,6 +11,7 @@ import { } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -48,25 +49,27 @@ export class AccountSwitcherService { private messagingService: MessagingService, private environmentService: EnvironmentService, private logService: LogService, + authService: AuthService, ) { this.availableAccounts$ = combineLatest([ - this.accountService.accounts$, + accountService.accounts$, + authService.authStatuses$, this.accountService.activeAccount$, ]).pipe( - switchMap(async ([accounts, activeAccount]) => { - const accountEntries = Object.entries(accounts).filter( - ([_, account]) => account.status !== AuthenticationStatus.LoggedOut, + switchMap(async ([accounts, accountStatuses, activeAccount]) => { + const loggedInIds = Object.keys(accounts).filter( + (id: UserId) => accountStatuses[id] !== AuthenticationStatus.LoggedOut, ); // Accounts shouldn't ever be more than ACCOUNT_LIMIT but just in case do a greater than - const hasMaxAccounts = accountEntries.length >= this.ACCOUNT_LIMIT; + const hasMaxAccounts = loggedInIds.length >= this.ACCOUNT_LIMIT; const options: AvailableAccount[] = await Promise.all( - accountEntries.map(async ([id, account]) => { + loggedInIds.map(async (id: UserId) => { return { - name: account.name ?? account.email, - email: account.email, + name: accounts[id].name ?? accounts[id].email, + email: accounts[id].email, id: id, server: (await this.environmentService.getEnvironment(id))?.getHostname(), - status: account.status, + status: accountStatuses[id], isActive: id === activeAccount?.id, avatarColor: await firstValueFrom( this.avatarService.getUserAvatarColor$(id as UserId), diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 0e43f420ab..4aecf8f585 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -779,14 +779,14 @@ export default class MainBackground { this.apiService, this.stateProvider, this.logService, - this.accountService, + this.authService, ); this.eventCollectionService = new EventCollectionService( this.cipherService, this.stateProvider, this.organizationService, this.eventUploadService, - this.accountService, + this.authService, ); this.totpService = new TotpService(this.cryptoFunctionService, this.logService); diff --git a/apps/browser/src/background/service-factories/event-collection-service.factory.ts b/apps/browser/src/background/service-factories/event-collection-service.factory.ts index ec892c73dd..b8f89c90bd 100644 --- a/apps/browser/src/background/service-factories/event-collection-service.factory.ts +++ b/apps/browser/src/background/service-factories/event-collection-service.factory.ts @@ -5,7 +5,10 @@ import { organizationServiceFactory, OrganizationServiceInitOptions, } from "../../admin-console/background/service-factories/organization-service.factory"; -import { accountServiceFactory } from "../../auth/background/service-factories/account-service.factory"; +import { + authServiceFactory, + AuthServiceInitOptions, +} from "../../auth/background/service-factories/auth-service.factory"; import { FactoryOptions, CachedServices, @@ -29,7 +32,8 @@ export type EventCollectionServiceInitOptions = EventCollectionServiceOptions & CipherServiceInitOptions & StateServiceInitOptions & OrganizationServiceInitOptions & - EventUploadServiceInitOptions; + EventUploadServiceInitOptions & + AuthServiceInitOptions; export function eventCollectionServiceFactory( cache: { eventCollectionService?: AbstractEventCollectionService } & CachedServices, @@ -45,7 +49,7 @@ export function eventCollectionServiceFactory( await stateProviderFactory(cache, opts), await organizationServiceFactory(cache, opts), await eventUploadServiceFactory(cache, opts), - await accountServiceFactory(cache, opts), + await authServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/background/service-factories/event-upload-service.factory.ts b/apps/browser/src/background/service-factories/event-upload-service.factory.ts index 4e1d7949be..b20310e8c9 100644 --- a/apps/browser/src/background/service-factories/event-upload-service.factory.ts +++ b/apps/browser/src/background/service-factories/event-upload-service.factory.ts @@ -1,7 +1,10 @@ import { EventUploadService as AbstractEventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { EventUploadService } from "@bitwarden/common/services/event/event-upload.service"; -import { accountServiceFactory } from "../../auth/background/service-factories/account-service.factory"; +import { + AuthServiceInitOptions, + authServiceFactory, +} from "../../auth/background/service-factories/auth-service.factory"; import { ApiServiceInitOptions, apiServiceFactory, @@ -23,7 +26,8 @@ type EventUploadServiceOptions = FactoryOptions; export type EventUploadServiceInitOptions = EventUploadServiceOptions & ApiServiceInitOptions & StateServiceInitOptions & - LogServiceInitOptions; + LogServiceInitOptions & + AuthServiceInitOptions; export function eventUploadServiceFactory( cache: { eventUploadService?: AbstractEventUploadService } & CachedServices, @@ -38,7 +42,7 @@ export function eventUploadServiceFactory( await apiServiceFactory(cache, opts), await stateProviderFactory(cache, opts), await logServiceFactory(cache, opts), - await accountServiceFactory(cache, opts), + await authServiceFactory(cache, opts), ), ); } diff --git a/apps/browser/src/platform/popup/header.component.ts b/apps/browser/src/platform/popup/header.component.ts index 6b9e9c9a3e..ebda12c2a4 100644 --- a/apps/browser/src/platform/popup/header.component.ts +++ b/apps/browser/src/platform/popup/header.component.ts @@ -1,8 +1,10 @@ import { Component, Input } from "@angular/core"; -import { Observable, map } from "rxjs"; +import { Observable, combineLatest, map, of, switchMap } 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 { UserId } from "@bitwarden/common/types/guid"; import { enableAccountSwitching } from "../flags"; @@ -14,14 +16,18 @@ export class HeaderComponent { @Input() noTheme = false; @Input() hideAccountSwitcher = false; authedAccounts$: Observable; - constructor(accountService: AccountService) { + constructor(accountService: AccountService, authService: AuthService) { this.authedAccounts$ = accountService.accounts$.pipe( - map((accounts) => { + switchMap((accounts) => { if (!enableAccountSwitching()) { - return false; + return of(false); } - return Object.values(accounts).some((a) => a.status !== AuthenticationStatus.LoggedOut); + return combineLatest( + Object.keys(accounts).map((id) => authService.authStatusFor$(id as UserId)), + ).pipe( + map((statuses) => statuses.some((status) => status !== AuthenticationStatus.LoggedOut)), + ); }), ); } diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 03ac1612f1..b0fdaec4fc 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -1,9 +1,10 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { ToastrService } from "ngx-toastr"; -import { filter, concatMap, Subject, takeUntil, firstValueFrom } from "rxjs"; +import { filter, concatMap, Subject, takeUntil, firstValueFrom, map } from "rxjs"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { DialogService, SimpleDialogOptions } from "@bitwarden/components"; @@ -57,8 +58,9 @@ export class AppComponent implements OnInit, OnDestroy { this.activeUserId = userId; }); - this.stateService.activeAccountUnlocked$ + this.authService.activeAccountStatus$ .pipe( + map((status) => status === AuthenticationStatus.Unlocked), filter((unlocked) => unlocked), concatMap(async () => { await this.recordActivity(); diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index fd6552e2f0..ed8c52ed94 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -678,7 +678,7 @@ export class Main { this.apiService, this.stateProvider, this.logService, - this.accountService, + this.authService, ); this.eventCollectionService = new EventCollectionService( @@ -686,7 +686,7 @@ export class Main { this.stateProvider, this.organizationService, this.eventUploadService, - this.accountService, + this.authService, ); } diff --git a/libs/angular/src/auth/guards/unauth.guard.ts b/libs/angular/src/auth/guards/unauth.guard.ts index 35c59b5744..9e1bca98ca 100644 --- a/libs/angular/src/auth/guards/unauth.guard.ts +++ b/libs/angular/src/auth/guards/unauth.guard.ts @@ -2,7 +2,6 @@ import { Injectable, inject } from "@angular/core"; import { CanActivate, CanActivateFn, Router, UrlTree } from "@angular/router"; import { Observable, 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"; @@ -43,14 +42,14 @@ const defaultRoutes: UnauthRoutes = { }; function unauthGuard(routes: UnauthRoutes): Observable { - const accountService = inject(AccountService); + const authService = inject(AuthService); const router = inject(Router); - return accountService.activeAccount$.pipe( - map((accountData) => { - if (accountData == null || accountData.status === AuthenticationStatus.LoggedOut) { + return authService.activeAccountStatus$.pipe( + map((status) => { + if (status == null || status === AuthenticationStatus.LoggedOut) { return true; - } else if (accountData.status === AuthenticationStatus.Locked) { + } else if (status === AuthenticationStatus.Locked) { return router.createUrlTree([routes.locked]); } else { return router.createUrlTree([routes.homepage()]); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 79bb6714d0..3de36020da 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -756,7 +756,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: EventUploadServiceAbstraction, useClass: EventUploadService, - deps: [ApiServiceAbstraction, StateProvider, LogService, AccountServiceAbstraction], + deps: [ApiServiceAbstraction, StateProvider, LogService, AuthServiceAbstraction], }), safeProvider({ provide: EventCollectionServiceAbstraction, @@ -766,7 +766,7 @@ const safeProviders: SafeProvider[] = [ StateProvider, OrganizationServiceAbstraction, EventUploadServiceAbstraction, - AccountServiceAbstraction, + AuthServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts index e8bb1b38ce..16479f19ea 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts @@ -1,6 +1,5 @@ import { firstValueFrom } from "rxjs"; -import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { FakeAccountService, @@ -66,7 +65,6 @@ describe("UserDecryptionOptionsService", () => { await fakeAccountService.addAccount(givenUser, { name: "Test User 1", email: "test1@email.com", - status: AuthenticationStatus.Locked, }); await fakeStateProvider.setUserState( USER_DECRYPTION_OPTIONS, diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index bd35d901c2..a8b09b7417 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -1,8 +1,7 @@ import { mock } from "jest-mock-extended"; -import { Observable, ReplaySubject } from "rxjs"; +import { ReplaySubject } from "rxjs"; import { AccountInfo, AccountService } from "../src/auth/abstractions/account.service"; -import { AuthenticationStatus } from "../src/auth/enums/authentication-status"; import { UserId } from "../src/types/guid"; export function mockAccountServiceWith( @@ -14,7 +13,6 @@ export function mockAccountServiceWith( ...{ name: "name", email: "email", - status: AuthenticationStatus.Locked, }, }; const service = new FakeAccountService({ [userId]: fullInfo }); @@ -34,8 +32,6 @@ export class FakeAccountService implements AccountService { } accounts$ = this.accountsSubject.asObservable(); activeAccount$ = this.activeAccountSubject.asObservable(); - accountLock$: Observable; - accountLogout$: Observable; constructor(initialData: Record) { this.accountsSubject.next(initialData); @@ -57,14 +53,6 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountEmail(userId, email); } - async setAccountStatus(userId: UserId, status: AuthenticationStatus): Promise { - await this.mock.setAccountStatus(userId, status); - } - - async setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise { - await this.mock.setMaxAccountStatus(userId, maxStatus); - } - async switchAccount(userId: UserId): Promise { const next = userId == null ? null : { id: userId, ...this.accountsSubject["_buffer"]?.[0]?.[userId] }; diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index 4e2a462755..fa9ad36378 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -1,27 +1,23 @@ import { Observable } from "rxjs"; import { UserId } from "../../types/guid"; -import { AuthenticationStatus } from "../enums/authentication-status"; /** * Holds information about an account for use in the AccountService * if more information is added, be sure to update the equality method. */ export type AccountInfo = { - status: AuthenticationStatus; email: string; name: string | undefined; }; export function accountInfoEqual(a: AccountInfo, b: AccountInfo) { - return a?.status === b?.status && a?.email === b?.email && a?.name === b?.name; + return a?.email === b?.email && a?.name === b?.name; } export abstract class AccountService { accounts$: Observable>; activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; - accountLock$: Observable; - accountLogout$: Observable; /** * Updates the `accounts$` observable with the new account data. * @param userId @@ -40,24 +36,6 @@ export abstract class AccountService { * @param email */ abstract setAccountEmail(userId: UserId, email: string): Promise; - /** - * Updates the `accounts$` observable with the new account status. - * Also emits the `accountLock$` or `accountLogout$` observable if the status is `Locked` or `LoggedOut` respectively. - * @param userId - * @param status - */ - abstract setAccountStatus(userId: UserId, status: AuthenticationStatus): Promise; - /** - * Updates the `accounts$` observable with the new account status if the current status is higher than the `maxStatus`. - * - * This method only downgrades status to the maximum value sent in, it will not increase authentication status. - * - * @example An account is transitioning from unlocked to logged out. If callbacks that set the status to locked occur - * after it is updated to logged out, the account will be in the incorrect state. - * @param userId The user id of the account to be updated. - * @param maxStatus The new status of the account. - */ - abstract setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise; /** * Updates the `activeAccount$` observable with the new active account. * @param userId diff --git a/libs/common/src/auth/abstractions/auth.service.ts b/libs/common/src/auth/abstractions/auth.service.ts index de08dbd4e9..36d5d219b2 100644 --- a/libs/common/src/auth/abstractions/auth.service.ts +++ b/libs/common/src/auth/abstractions/auth.service.ts @@ -6,6 +6,8 @@ import { AuthenticationStatus } from "../enums/authentication-status"; export abstract class AuthService { /** Authentication status for the active user */ abstract activeAccountStatus$: Observable; + /** Authentication status for all known users */ + abstract authStatuses$: Observable>; /** * Returns an observable authentication status for the given user id. * @note userId is a required parameter, null values will always return `AuthenticationStatus.LoggedOut` diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index e4195365f4..a9cec82c51 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -8,7 +8,6 @@ import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { UserId } from "../../types/guid"; import { AccountInfo } from "../abstractions/account.service"; -import { AuthenticationStatus } from "../enums/authentication-status"; import { ACCOUNT_ACCOUNTS, @@ -24,9 +23,7 @@ describe("accountService", () => { let accountsState: FakeGlobalState>; let activeAccountIdState: FakeGlobalState; const userId = "userId" as UserId; - function userInfo(status: AuthenticationStatus): AccountInfo { - return { status, email: "email", name: "name" }; - } + const userInfo = { email: "email", name: "name" }; beforeEach(() => { messagingService = mock(); @@ -50,61 +47,49 @@ describe("accountService", () => { expect(emissions).toEqual([undefined]); }); - it("should emit the active account and status", async () => { + it("should emit the active account", async () => { const emissions = trackEmissions(sut.activeAccount$); - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); + accountsState.stateSubject.next({ [userId]: userInfo }); activeAccountIdState.stateSubject.next(userId); expect(emissions).toEqual([ undefined, // initial value - { id: userId, ...userInfo(AuthenticationStatus.Unlocked) }, - ]); - }); - - it("should update the status if the account status changes", async () => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); - activeAccountIdState.stateSubject.next(userId); - const emissions = trackEmissions(sut.activeAccount$); - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Locked) }); - - expect(emissions).toEqual([ - { id: userId, ...userInfo(AuthenticationStatus.Unlocked) }, - { id: userId, ...userInfo(AuthenticationStatus.Locked) }, + { id: userId, ...userInfo }, ]); }); it("should remember the last emitted value", async () => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); + accountsState.stateSubject.next({ [userId]: userInfo }); activeAccountIdState.stateSubject.next(userId); expect(await firstValueFrom(sut.activeAccount$)).toEqual({ id: userId, - ...userInfo(AuthenticationStatus.Unlocked), + ...userInfo, }); }); }); describe("accounts$", () => { it("should maintain an accounts cache", async () => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Locked) }); + accountsState.stateSubject.next({ [userId]: userInfo }); + accountsState.stateSubject.next({ [userId]: userInfo }); expect(await firstValueFrom(sut.accounts$)).toEqual({ - [userId]: userInfo(AuthenticationStatus.Locked), + [userId]: userInfo, }); }); }); describe("addAccount", () => { it("should emit the new account", async () => { - await sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + await sut.addAccount(userId, userInfo); const currentValue = await firstValueFrom(sut.accounts$); - expect(currentValue).toEqual({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); + expect(currentValue).toEqual({ [userId]: userInfo }); }); }); describe("setAccountName", () => { - const initialState = { [userId]: userInfo(AuthenticationStatus.Unlocked) }; + const initialState = { [userId]: userInfo }; beforeEach(() => { accountsState.stateSubject.next(initialState); }); @@ -114,7 +99,7 @@ describe("accountService", () => { const currentState = await firstValueFrom(accountsState.state$); expect(currentState).toEqual({ - [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "new name" }, + [userId]: { ...userInfo, name: "new name" }, }); }); @@ -127,7 +112,7 @@ describe("accountService", () => { }); describe("setAccountEmail", () => { - const initialState = { [userId]: userInfo(AuthenticationStatus.Unlocked) }; + const initialState = { [userId]: userInfo }; beforeEach(() => { accountsState.stateSubject.next(initialState); }); @@ -137,7 +122,7 @@ describe("accountService", () => { const currentState = await firstValueFrom(accountsState.state$); expect(currentState).toEqual({ - [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "new email" }, + [userId]: { ...userInfo, email: "new email" }, }); }); @@ -149,49 +134,9 @@ describe("accountService", () => { }); }); - describe("setAccountStatus", () => { - const initialState = { [userId]: userInfo(AuthenticationStatus.Unlocked) }; - beforeEach(() => { - accountsState.stateSubject.next(initialState); - }); - - it("should update the account", async () => { - await sut.setAccountStatus(userId, AuthenticationStatus.Locked); - const currentState = await firstValueFrom(accountsState.state$); - - expect(currentState).toEqual({ - [userId]: { - ...userInfo(AuthenticationStatus.Unlocked), - status: AuthenticationStatus.Locked, - }, - }); - }); - - it("should not update if the status is the same", async () => { - await sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); - const currentState = await firstValueFrom(accountsState.state$); - - expect(currentState).toEqual(initialState); - }); - - it("should emit logout if the status is logged out", async () => { - const emissions = trackEmissions(sut.accountLogout$); - await sut.setAccountStatus(userId, AuthenticationStatus.LoggedOut); - - expect(emissions).toEqual([userId]); - }); - - it("should emit lock if the status is locked", async () => { - const emissions = trackEmissions(sut.accountLock$); - await sut.setAccountStatus(userId, AuthenticationStatus.Locked); - - expect(emissions).toEqual([userId]); - }); - }); - describe("switchAccount", () => { beforeEach(() => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); + accountsState.stateSubject.next({ [userId]: userInfo }); activeAccountIdState.stateSubject.next(userId); }); @@ -207,26 +152,4 @@ describe("accountService", () => { expect(sut.switchAccount("unknown" as UserId)).rejects.toThrowError("Account does not exist"); }); }); - - describe("setMaxAccountStatus", () => { - it("should update the account", async () => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.Unlocked) }); - await sut.setMaxAccountStatus(userId, AuthenticationStatus.Locked); - const currentState = await firstValueFrom(accountsState.state$); - - expect(currentState).toEqual({ - [userId]: userInfo(AuthenticationStatus.Locked), - }); - }); - - it("should not update if the new max status is higher than the current", async () => { - accountsState.stateSubject.next({ [userId]: userInfo(AuthenticationStatus.LoggedOut) }); - await sut.setMaxAccountStatus(userId, AuthenticationStatus.Locked); - const currentState = await firstValueFrom(accountsState.state$); - - expect(currentState).toEqual({ - [userId]: userInfo(AuthenticationStatus.LoggedOut), - }); - }); - }); }); diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 8ef235d815..77d61fae91 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -14,7 +14,6 @@ import { KeyDefinition, } from "../../platform/state"; import { UserId } from "../../types/guid"; -import { AuthenticationStatus } from "../enums/authentication-status"; export const ACCOUNT_ACCOUNTS = KeyDefinition.record( ACCOUNT_MEMORY, @@ -36,8 +35,6 @@ export class AccountServiceImplementation implements InternalAccountService { accounts$; activeAccount$; - accountLock$ = this.lock.asObservable(); - accountLogout$ = this.logout.asObservable(); constructor( private messagingService: MessagingService, @@ -74,34 +71,6 @@ export class AccountServiceImplementation implements InternalAccountService { await this.setAccountInfo(userId, { email }); } - async setAccountStatus(userId: UserId, status: AuthenticationStatus): Promise { - await this.setAccountInfo(userId, { status }); - - if (status === AuthenticationStatus.LoggedOut) { - this.logout.next(userId); - } else if (status === AuthenticationStatus.Locked) { - this.lock.next(userId); - } - } - - async setMaxAccountStatus(userId: UserId, maxStatus: AuthenticationStatus): Promise { - await this.accountsState.update( - (accounts) => { - accounts[userId].status = maxStatus; - return accounts; - }, - { - shouldUpdate: (accounts) => { - if (accounts?.[userId] == null) { - throw new Error("Account does not exist"); - } - - return accounts[userId].status > maxStatus; - }, - }, - ); - } - async switchAccount(userId: UserId): Promise { await this.activeAccountIdState.update( (_, accounts) => { diff --git a/libs/common/src/auth/services/auth.service.spec.ts b/libs/common/src/auth/services/auth.service.spec.ts index 07e38def4b..3bdf85d3e1 100644 --- a/libs/common/src/auth/services/auth.service.spec.ts +++ b/libs/common/src/auth/services/auth.service.spec.ts @@ -122,6 +122,25 @@ describe("AuthService", () => { }); }); + describe("authStatuses$", () => { + it("requests auth status for all known users", async () => { + const userId2 = Utils.newGuid() as UserId; + + await accountService.addAccount(userId2, { email: "email2", name: "name2" }); + + const mockFn = jest.fn().mockReturnValue(of(AuthenticationStatus.Locked)); + sut.authStatusFor$ = mockFn; + + await expect(firstValueFrom(await sut.authStatuses$)).resolves.toEqual({ + [userId]: AuthenticationStatus.Locked, + [userId2]: AuthenticationStatus.Locked, + }); + expect(mockFn).toHaveBeenCalledTimes(2); + expect(mockFn).toHaveBeenCalledWith(userId); + expect(mockFn).toHaveBeenCalledWith(userId2); + }); + }); + describe("authStatusFor$", () => { beforeEach(() => { tokenService.hasAccessToken$.mockReturnValue(of(true)); diff --git a/libs/common/src/auth/services/auth.service.ts b/libs/common/src/auth/services/auth.service.ts index de5eb66c06..7a29d313e7 100644 --- a/libs/common/src/auth/services/auth.service.ts +++ b/libs/common/src/auth/services/auth.service.ts @@ -21,6 +21,7 @@ import { AuthenticationStatus } from "../enums/authentication-status"; export class AuthService implements AuthServiceAbstraction { activeAccountStatus$: Observable; + authStatuses$: Observable>; constructor( protected accountService: AccountService, @@ -36,6 +37,26 @@ export class AuthService implements AuthServiceAbstraction { return this.authStatusFor$(userId); }), ); + + this.authStatuses$ = this.accountService.accounts$.pipe( + map((accounts) => Object.keys(accounts) as UserId[]), + switchMap((entries) => + combineLatest( + entries.map((userId) => + this.authStatusFor$(userId).pipe(map((status) => ({ userId, status }))), + ), + ), + ), + map((statuses) => { + return statuses.reduce( + (acc, { userId, status }) => { + acc[userId] = status; + return acc; + }, + {} as Record, + ); + }), + ); } authStatusFor$(userId: UserId): Observable { diff --git a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts index 408ed33c97..fc5060af5f 100644 --- a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts +++ b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts @@ -8,7 +8,6 @@ import { OrganizationAutoEnrollStatusResponse } from "../../admin-console/models import { CryptoService } from "../../platform/abstractions/crypto.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; import { AccountInfo, AccountService } from "../abstractions/account.service"; -import { AuthenticationStatus } from "../enums/authentication-status"; import { PasswordResetEnrollmentServiceImplementation } from "./password-reset-enrollment.service.implementation"; @@ -91,7 +90,6 @@ describe("PasswordResetEnrollmentServiceImplementation", () => { const user1AccountInfo: AccountInfo = { name: "Test User 1", email: "test1@email.com", - status: AuthenticationStatus.Unlocked, }; activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "userId" as UserId })); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 227cb43879..3017ae7195 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -33,10 +33,6 @@ export type InitOptions = { export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; activeAccount$: Observable; - /** - * @deprecated use accountService.activeAccount$ instead - */ - activeAccountUnlocked$: Observable; addAccount: (account: T) => Promise; setActiveUser: (userId: string) => Promise; diff --git a/libs/common/src/platform/services/crypto.service.spec.ts b/libs/common/src/platform/services/crypto.service.spec.ts index 6d0fdb1423..16e6d4aa63 100644 --- a/libs/common/src/platform/services/crypto.service.spec.ts +++ b/libs/common/src/platform/services/crypto.service.spec.ts @@ -4,7 +4,6 @@ import { firstValueFrom, of, tap } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { FakeActiveUserState, FakeSingleUserState } from "../../../spec/fake-state"; import { FakeStateProvider } from "../../../spec/fake-state-provider"; -import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { FakeMasterPasswordService } from "../../auth/services/master-password/fake-master-password.service"; import { CsprngArray } from "../../types/csprng"; import { UserId } from "../../types/guid"; @@ -273,15 +272,6 @@ describe("cryptoService", () => { await expect(cryptoService.setUserKey(null, mockUserId)).rejects.toThrow("No key provided."); }); - it("should update the user's lock state", async () => { - await cryptoService.setUserKey(mockUserKey, mockUserId); - - expect(accountService.mock.setAccountStatus).toHaveBeenCalledWith( - mockUserId, - AuthenticationStatus.Unlocked, - ); - }); - describe("Pin Key refresh", () => { let cryptoSvcMakePinKey: jest.SpyInstance; const protectedPin = @@ -353,23 +343,6 @@ describe("cryptoService", () => { accountService.activeAccount$ = accountService.activeAccountSubject.asObservable(); }); - it("sets the maximum account status of the active user id to locked when user id is not specified", async () => { - await cryptoService.clearKeys(); - expect(accountService.mock.setMaxAccountStatus).toHaveBeenCalledWith( - mockUserId, - AuthenticationStatus.Locked, - ); - }); - - it("sets the maximum account status of the specified user id to locked when user id is specified", async () => { - const userId = "someOtherUser" as UserId; - await cryptoService.clearKeys(userId); - expect(accountService.mock.setMaxAccountStatus).toHaveBeenCalledWith( - userId, - AuthenticationStatus.Locked, - ); - }); - describe.each([ USER_ENCRYPTED_ORGANIZATION_KEYS, USER_ENCRYPTED_PROVIDER_KEYS, diff --git a/libs/common/src/platform/services/crypto.service.ts b/libs/common/src/platform/services/crypto.service.ts index ae588cbc31..c091b6a5a9 100644 --- a/libs/common/src/platform/services/crypto.service.ts +++ b/libs/common/src/platform/services/crypto.service.ts @@ -7,7 +7,6 @@ import { ProfileProviderOrganizationResponse } from "../../admin-console/models/ import { ProfileProviderResponse } from "../../admin-console/models/response/profile-provider.response"; import { AccountService } from "../../auth/abstractions/account.service"; import { InternalMasterPasswordServiceAbstraction } from "../../auth/abstractions/master-password.service.abstraction"; -import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { Utils } from "../../platform/misc/utils"; import { CsprngArray } from "../../types/csprng"; @@ -152,8 +151,6 @@ export class CryptoService implements CryptoServiceAbstraction { [userId, key] = await this.stateProvider.setUserState(USER_KEY, key, userId); await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, true, userId); - await this.accountService.setAccountStatus(userId, AuthenticationStatus.Unlocked); - await this.storeAdditionalKeys(key, userId); } @@ -256,14 +253,13 @@ export class CryptoService implements CryptoServiceAbstraction { * Clears the user key. Clears all stored versions of the user keys as well, such as the biometrics key * @param userId The desired user */ - async clearUserKey(userId: UserId): Promise { + private async clearUserKey(userId: UserId): Promise { if (userId == null) { // nothing to do return; } // Set userId to ensure we have one for the account status update await this.stateProvider.setUserState(USER_KEY, null, userId); - await this.accountService.setMaxAccountStatus(userId, AuthenticationStatus.Locked); await this.clearAllStoredUserKeys(userId); } diff --git a/libs/common/src/platform/services/default-environment.service.spec.ts b/libs/common/src/platform/services/default-environment.service.spec.ts index a70ab3d179..dd504dc302 100644 --- a/libs/common/src/platform/services/default-environment.service.spec.ts +++ b/libs/common/src/platform/services/default-environment.service.spec.ts @@ -2,7 +2,6 @@ import { firstValueFrom } from "rxjs"; import { FakeStateProvider, awaitAsync } from "../../../spec"; import { FakeAccountService } from "../../../spec/fake-account-service"; -import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { UserId } from "../../types/guid"; import { CloudRegion, Region } from "../abstractions/environment.service"; @@ -32,12 +31,10 @@ describe("EnvironmentService", () => { [testUser]: { name: "name", email: "email", - status: AuthenticationStatus.Locked, }, [alternateTestUser]: { name: "name", email: "email", - status: AuthenticationStatus.Locked, }, }); stateProvider = new FakeStateProvider(accountService); @@ -50,7 +47,6 @@ describe("EnvironmentService", () => { id: userId, email: "test@example.com", name: `Test Name ${userId}`, - status: AuthenticationStatus.Unlocked, }); await awaitAsync(); }; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index b3e33cf362..7dbac2b02a 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,9 +1,8 @@ -import { BehaviorSubject, Observable, map } from "rxjs"; +import { BehaviorSubject } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; import { TokenService } from "../../auth/abstractions/token.service"; -import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; import { BiometricKey } from "../../auth/types/biometric-key"; @@ -68,8 +67,6 @@ export class StateService< protected activeAccountSubject = new BehaviorSubject(null); activeAccount$ = this.activeAccountSubject.asObservable(); - activeAccountUnlocked$: Observable; - private hasBeenInited = false; protected isRecoveredSession = false; @@ -89,13 +86,7 @@ export class StateService< protected tokenService: TokenService, private migrationRunner: MigrationRunner, protected useAccountCache: boolean = true, - ) { - this.activeAccountUnlocked$ = this.accountService.activeAccount$.pipe( - map((a) => { - return a?.status === AuthenticationStatus.Unlocked; - }), - ); - } + ) {} async init(initOptions: InitOptions = {}): Promise { // Deconstruct and apply defaults @@ -151,7 +142,6 @@ export class StateService< await this.accountService.addAccount(state.activeUserId as UserId, { name: activeDiskAccount.profile.name, email: activeDiskAccount.profile.email, - status: AuthenticationStatus.LoggedOut, }); } await this.accountService.switchAccount(state.activeUserId as UserId); @@ -177,16 +167,7 @@ export class StateService< // TODO: Temporary update to avoid routing all account status changes through account service for now. // The determination of state should be handled by the various services that control those values. - const token = await this.tokenService.getAccessToken(userId as UserId); - const autoKey = await this.getUserKeyAutoUnlock({ userId: userId }); - const accountStatus = - token == null - ? AuthenticationStatus.LoggedOut - : autoKey == null - ? AuthenticationStatus.Locked - : AuthenticationStatus.Unlocked; await this.accountService.addAccount(userId as UserId, { - status: accountStatus, name: diskAccount.profile.name, email: diskAccount.profile.email, }); @@ -206,7 +187,6 @@ export class StateService< await this.setLastActive(new Date().getTime(), { userId: account.profile.userId }); // TODO: Temporary update to avoid routing all account status changes through account service for now. await this.accountService.addAccount(account.profile.userId as UserId, { - status: AuthenticationStatus.Locked, name: account.profile.name, email: account.profile.email, }); @@ -1406,8 +1386,6 @@ export class StateService< return state; }); - // TODO: Invert this logic, we should remove accounts based on logged out emit - await this.accountService.setAccountStatus(userId as UserId, AuthenticationStatus.LoggedOut); } // settings persist even on reset, and are not affected by this method diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts index 6e01b615d7..51a972a9dc 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts @@ -9,7 +9,6 @@ import { Jsonify } from "type-fest"; import { awaitAsync, trackEmissions } from "../../../../spec"; import { FakeStorageService } from "../../../../spec/fake-storage.service"; import { AccountInfo } from "../../../auth/abstractions/account.service"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; import { StorageServiceProvider } from "../../services/storage-service.provider"; import { StateDefinition } from "../state-definition"; @@ -84,7 +83,6 @@ describe("DefaultActiveUserState", () => { id: userId, email: `test${id}@example.com`, name: `Test User ${id}`, - status: AuthenticationStatus.Unlocked, }); await awaitAsync(); }; diff --git a/libs/common/src/services/event/event-collection.service.ts b/libs/common/src/services/event/event-collection.service.ts index 2d2b553062..641c1b4d44 100644 --- a/libs/common/src/services/event/event-collection.service.ts +++ b/libs/common/src/services/event/event-collection.service.ts @@ -3,7 +3,7 @@ import { firstValueFrom, map, from, zip } from "rxjs"; import { EventCollectionService as EventCollectionServiceAbstraction } from "../../abstractions/event/event-collection.service"; import { EventUploadService } from "../../abstractions/event/event-upload.service"; import { OrganizationService } from "../../admin-console/abstractions/organization/organization.service.abstraction"; -import { AccountService } from "../../auth/abstractions/account.service"; +import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { EventType } from "../../enums"; import { EventData } from "../../models/data/event.data"; @@ -18,7 +18,7 @@ export class EventCollectionService implements EventCollectionServiceAbstraction private stateProvider: StateProvider, private organizationService: OrganizationService, private eventUploadService: EventUploadService, - private accountService: AccountService, + private authService: AuthService, ) {} /** Adds an event to the active user's event collection @@ -71,12 +71,12 @@ export class EventCollectionService implements EventCollectionServiceAbstraction const cipher$ = from(this.cipherService.get(cipherId)); - const [accountInfo, orgIds, cipher] = await firstValueFrom( - zip(this.accountService.activeAccount$, orgIds$, cipher$), + const [authStatus, orgIds, cipher] = await firstValueFrom( + zip(this.authService.activeAccountStatus$, orgIds$, cipher$), ); // The user must be authorized - if (accountInfo.status != AuthenticationStatus.Unlocked) { + if (authStatus != AuthenticationStatus.Unlocked) { return false; } diff --git a/libs/common/src/services/event/event-upload.service.ts b/libs/common/src/services/event/event-upload.service.ts index 4ee4300c39..6f229751bf 100644 --- a/libs/common/src/services/event/event-upload.service.ts +++ b/libs/common/src/services/event/event-upload.service.ts @@ -2,7 +2,7 @@ import { firstValueFrom, map } from "rxjs"; import { ApiService } from "../../abstractions/api.service"; import { EventUploadService as EventUploadServiceAbstraction } from "../../abstractions/event/event-upload.service"; -import { AccountService } from "../../auth/abstractions/account.service"; +import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { EventData } from "../../models/data/event.data"; import { EventRequest } from "../../models/request/event.request"; @@ -18,7 +18,7 @@ export class EventUploadService implements EventUploadServiceAbstraction { private apiService: ApiService, private stateProvider: StateProvider, private logService: LogService, - private accountService: AccountService, + private authService: AuthService, ) {} init(checkOnInterval: boolean) { @@ -43,13 +43,16 @@ export class EventUploadService implements EventUploadServiceAbstraction { userId = await firstValueFrom(this.stateProvider.activeUserId$); } - // Get the auth status from the provided user or the active user - const userAuth$ = this.accountService.accounts$.pipe( - map((accounts) => accounts[userId]?.status === AuthenticationStatus.Unlocked), - ); + if (!userId) { + return; + } - const isAuthenticated = await firstValueFrom(userAuth$); - if (!isAuthenticated) { + const isUnlocked = await firstValueFrom( + this.authService + .authStatusFor$(userId) + .pipe(map((status) => status === AuthenticationStatus.Unlocked)), + ); + if (!isUnlocked) { return; } diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts index 0594de741c..243b644dd8 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts @@ -138,7 +138,6 @@ describe("VaultTimeoutService", () => { if (globalSetups?.userId) { accountService.activeAccountSubject.next({ id: globalSetups.userId as UserId, - status: accounts[globalSetups.userId]?.authStatus, email: null, name: null, }); diff --git a/libs/common/src/tools/send/services/send.service.spec.ts b/libs/common/src/tools/send/services/send.service.spec.ts index fc793dba67..41183c42af 100644 --- a/libs/common/src/tools/send/services/send.service.spec.ts +++ b/libs/common/src/tools/send/services/send.service.spec.ts @@ -8,7 +8,6 @@ import { awaitAsync, mockAccountServiceWith, } from "../../../../spec"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { CryptoService } from "../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../platform/abstractions/i18n.service"; @@ -64,7 +63,6 @@ describe("SendService", () => { id: mockUserId, email: "email", name: "name", - status: AuthenticationStatus.Unlocked, }); // Initial encrypted state