diff --git a/apps/browser/gulpfile.js b/apps/browser/gulpfile.js index d5b29ffc38..3fe2c44dd1 100644 --- a/apps/browser/gulpfile.js +++ b/apps/browser/gulpfile.js @@ -30,6 +30,19 @@ const filters = { safari: ["!build/safari/**/*"], }; +/** + * Converts a number to a tuple containing two Uint16's + * @param num {number} This number is expected to be a integer style number with no decimals + * + * @returns {number[]} A tuple containing two elements that are both numbers. + */ +function numToUint16s(num) { + var arr = new ArrayBuffer(4); + var view = new DataView(arr); + view.setUint32(0, num, false); + return [view.getUint16(0), view.getUint16(2)]; +} + function buildString() { var build = ""; if (process.env.MANIFEST_VERSION) { @@ -258,8 +271,19 @@ function applyBetaLabels(manifest) { manifest.short_name = "Bitwarden BETA"; manifest.description = "THIS EXTENSION IS FOR BETA TESTING BITWARDEN."; if (process.env.GITHUB_RUN_ID) { - manifest.version_name = `${manifest.version} beta - ${process.env.GITHUB_SHA.slice(0, 8)}`; - manifest.version = `${manifest.version}.${parseInt(process.env.GITHUB_RUN_ID.slice(-4))}`; + const existingVersionParts = manifest.version.split("."); // 3 parts expected 2024.4.0 + + // GITHUB_RUN_ID is a number like: 8853654662 + // which will convert to [ 4024, 3206 ] + // and a single incremented id of 8853654663 will become [ 4024, 3207 ] + const runIdParts = numToUint16s(parseInt(process.env.GITHUB_RUN_ID)); + + // Only use the first 2 parts from the given version number and base the other 2 numbers from the GITHUB_RUN_ID + // Example: 2024.4.4024.3206 + const betaVersion = `${existingVersionParts[0]}.${existingVersionParts[1]}.${runIdParts[0]}.${runIdParts[1]}`; + + manifest.version_name = `${betaVersion} beta - ${process.env.GITHUB_SHA.slice(0, 8)}`; + manifest.version = betaVersion; } else { manifest.version = `${manifest.version}.0`; } diff --git a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html index aebf2219ff..806dae084d 100644 --- a/apps/browser/src/auth/popup/account-switching/account-switcher.component.html +++ b/apps/browser/src/auth/popup/account-switching/account-switcher.component.html @@ -49,7 +49,7 @@ + + + + + + + + + + + + + + `, standalone: true, + imports: [CommonModule, ItemModule, BadgeModule, IconButtonModule], }) -class VaultComponent {} +class VaultComponent { + protected data = Array.from(Array(20).keys()); +} @Component({ selector: "generator-placeholder", diff --git a/apps/browser/src/platform/services/browser-state.service.spec.ts b/apps/browser/src/platform/services/browser-state.service.spec.ts index f06126dcf5..a0a52ff622 100644 --- a/apps/browser/src/platform/services/browser-state.service.spec.ts +++ b/apps/browser/src/platform/services/browser-state.service.spec.ts @@ -1,5 +1,4 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { firstValueFrom } from "rxjs"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -50,7 +49,6 @@ describe("Browser State Service", () => { state.accounts[userId] = new Account({ profile: { userId: userId }, }); - state.activeUserId = userId; }); afterEach(() => { @@ -78,18 +76,8 @@ describe("Browser State Service", () => { ); }); - describe("add Account", () => { - it("should add account", async () => { - const newUserId = "newUserId" as UserId; - const newAcct = new Account({ - profile: { userId: newUserId }, - }); - - await sut.addAccount(newAcct); - - const accts = await firstValueFrom(sut.accounts$); - expect(accts[newUserId]).toBeDefined(); - }); + it("exists", () => { + expect(sut).toBeDefined(); }); }); }); diff --git a/apps/browser/src/platform/services/default-browser-state.service.ts b/apps/browser/src/platform/services/default-browser-state.service.ts index b9cd219076..f717ab96d8 100644 --- a/apps/browser/src/platform/services/default-browser-state.service.ts +++ b/apps/browser/src/platform/services/default-browser-state.service.ts @@ -29,8 +29,6 @@ export class DefaultBrowserStateService initializeAs: "record", }) protected accountsSubject: BehaviorSubject<{ [userId: string]: Account }>; - @sessionSync({ initializer: (s: string) => s }) - protected activeAccountSubject: BehaviorSubject; protected accountDeserializer = Account.fromJSON; diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index 0fa359181d..c29b9c69dc 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -200,26 +200,29 @@ export class LocalBackedSessionStorageService } private compareValues(value1: T, value2: T): boolean { - if (value1 == null && value2 == null) { + try { + if (value1 == null && value2 == null) { + return true; + } + + if (value1 && value2 == null) { + return false; + } + + if (value1 == null && value2) { + return false; + } + + if (typeof value1 !== "object" || typeof value2 !== "object") { + return value1 === value2; + } + + return JSON.stringify(value1) === JSON.stringify(value2); + } catch (e) { + this.logService.error( + `error comparing values\n${JSON.stringify(value1)}\n${JSON.stringify(value2)}`, + ); return true; } - - if (value1 && value2 == null) { - return false; - } - - if (value1 == null && value2) { - return false; - } - - if (typeof value1 !== "object" || typeof value2 !== "object") { - return value1 === value2; - } - - if (JSON.stringify(value1) === JSON.stringify(value2)) { - return true; - } - - return Object.entries(value1).sort().toString() === Object.entries(value2).sort().toString(); } } diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 7acaf1ba93..25fac44450 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -1,12 +1,14 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; -import { filter, concatMap, Subject, takeUntil, firstValueFrom, tap, map } from "rxjs"; +import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } 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 { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { MessageListener } from "@bitwarden/common/platform/messaging"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService, SimpleDialogOptions, ToastService } from "@bitwarden/components"; @@ -27,8 +29,9 @@ import { DesktopSyncVerificationDialogComponent } from "./components/desktop-syn `, }) export class AppComponent implements OnInit, OnDestroy { - private lastActivity: number = null; - private activeUserId: string; + private lastActivity: Date; + private activeUserId: UserId; + private recordActivitySubject = new Subject(); private destroy$ = new Subject(); @@ -46,6 +49,7 @@ export class AppComponent implements OnInit, OnDestroy { private dialogService: DialogService, private messageListener: MessageListener, private toastService: ToastService, + private accountService: AccountService, ) {} async ngOnInit() { @@ -53,14 +57,13 @@ export class AppComponent implements OnInit, OnDestroy { // Clear them aggressively to make sure this doesn't occur await this.clearComponentStates(); - this.stateService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((userId) => { - this.activeUserId = userId; + this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { + this.activeUserId = account?.id; }); this.authService.activeAccountStatus$ .pipe( - map((status) => status === AuthenticationStatus.Unlocked), - filter((unlocked) => unlocked), + filter((status) => status === AuthenticationStatus.Unlocked), concatMap(async () => { await this.recordActivity(); }), @@ -200,13 +203,13 @@ export class AppComponent implements OnInit, OnDestroy { return; } - const now = new Date().getTime(); - if (this.lastActivity != null && now - this.lastActivity < 250) { + const now = new Date(); + if (this.lastActivity != null && now.getTime() - this.lastActivity.getTime() < 250) { return; } this.lastActivity = now; - await this.stateService.setLastActive(now, { userId: this.activeUserId }); + await this.accountService.setAccountActivity(this.activeUserId, now); } private showToast(msg: any) { diff --git a/apps/browser/src/tools/popup/send/send-add-edit.component.ts b/apps/browser/src/tools/popup/send/send-add-edit.component.ts index baf985b6e9..c20bf7cb8d 100644 --- a/apps/browser/src/tools/popup/send/send-add-edit.component.ts +++ b/apps/browser/src/tools/popup/send/send-add-edit.component.ts @@ -6,6 +6,7 @@ import { first } from "rxjs/operators"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -51,6 +52,7 @@ export class SendAddEditComponent extends BaseAddEditComponent { formBuilder: FormBuilder, private filePopoutUtilsService: FilePopoutUtilsService, billingAccountProfileStateService: BillingAccountProfileStateService, + accountService: AccountService, ) { super( i18nService, @@ -66,6 +68,7 @@ export class SendAddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); } diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index b3fb68fe63..665701639e 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -3,6 +3,7 @@ import * as path from "path"; import { program } from "commander"; import * as jsdom from "jsdom"; +import { firstValueFrom } from "rxjs"; import { InternalUserDecryptionOptionsServiceAbstraction, @@ -79,7 +80,7 @@ import { MigrationBuilderService } from "@bitwarden/common/platform/services/mig import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; import { StateService } from "@bitwarden/common/platform/services/state.service"; import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; -import { UserKeyInitService } from "@bitwarden/common/platform/services/user-key-init.service"; +import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { ActiveUserStateProvider, DerivedStateProvider, @@ -236,7 +237,7 @@ export class Main { biometricStateService: BiometricStateService; billingAccountProfileStateService: BillingAccountProfileStateService; providerApiService: ProviderApiServiceAbstraction; - userKeyInitService: UserKeyInitService; + userAutoUnlockKeyService: UserAutoUnlockKeyService; kdfConfigService: KdfConfigServiceAbstraction; constructor() { @@ -709,11 +710,7 @@ export class Main { this.providerApiService = new ProviderApiService(this.apiService); - this.userKeyInitService = new UserKeyInitService( - this.accountService, - this.cryptoService, - this.logService, - ); + this.userAutoUnlockKeyService = new UserAutoUnlockKeyService(this.cryptoService); } async run() { @@ -734,7 +731,7 @@ export class Main { this.authService.logOut(() => { /* Do nothing */ }); - const userId = await this.stateService.getUserId(); + const userId = (await this.stateService.getUserId()) as UserId; await Promise.all([ this.eventUploadService.uploadEvents(userId as UserId), this.syncService.setLastSync(new Date(0)), @@ -745,9 +742,10 @@ export class Main { this.passwordGenerationService.clear(), ]); - await this.stateEventRunnerService.handleEvent("logout", userId as UserId); + await this.stateEventRunnerService.handleEvent("logout", userId); await this.stateService.clean(); + await this.accountService.clean(userId); process.env.BW_SESSION = null; } @@ -757,7 +755,11 @@ export class Main { this.containerService.attachToGlobal(global); await this.i18nService.init(); this.twoFactorService.init(); - this.userKeyInitService.listenForActiveUserChangesToSetUserKey(); + + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + if (activeAccount) { + await this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(activeAccount.id); + } } } diff --git a/apps/desktop/src/app/app-routing.module.ts b/apps/desktop/src/app/app-routing.module.ts index 4fc19c8433..bb8deb2339 100644 --- a/apps/desktop/src/app/app-routing.module.ts +++ b/apps/desktop/src/app/app-routing.module.ts @@ -9,7 +9,7 @@ import { } from "@bitwarden/angular/auth/guards"; import { AccessibilityCookieComponent } from "../auth/accessibility-cookie.component"; -import { LoginGuard } from "../auth/guards/login.guard"; +import { maxAccountsGuardFn } from "../auth/guards/max-accounts.guard"; import { HintComponent } from "../auth/hint.component"; import { LockComponent } from "../auth/lock.component"; import { LoginDecryptionOptionsComponent } from "../auth/login/login-decryption-options/login-decryption-options.component"; @@ -40,7 +40,7 @@ const routes: Routes = [ { path: "login", component: LoginComponent, - canActivate: [LoginGuard], + canActivate: [maxAccountsGuardFn()], }, { path: "login-with-device", diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index ad99a3a447..4e540efdc6 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -8,7 +8,7 @@ import { ViewContainerRef, } from "@angular/core"; import { Router } from "@angular/router"; -import { firstValueFrom, Subject, takeUntil } from "rxjs"; +import { firstValueFrom, map, Subject, takeUntil } from "rxjs"; import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; import { ModalService } from "@bitwarden/angular/services/modal.service"; @@ -18,9 +18,9 @@ import { NotificationsService } from "@bitwarden/common/abstractions/notificatio import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; -import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { ProviderService } from "@bitwarden/common/admin-console/abstractions/provider.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -107,11 +107,11 @@ export class AppComponent implements OnInit, OnDestroy { loading = false; - private lastActivity: number = null; + private lastActivity: Date = null; private modal: ModalRef = null; private idleTimer: number = null; private isIdle = false; - private activeUserId: string = null; + private activeUserId: UserId = null; private destroy$ = new Subject(); @@ -150,12 +150,12 @@ export class AppComponent implements OnInit, OnDestroy { private biometricStateService: BiometricStateService, private stateEventRunnerService: StateEventRunnerService, private providerService: ProviderService, - private organizationService: InternalOrganizationServiceAbstraction, + private accountService: AccountService, ) {} ngOnInit() { - this.stateService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((userId) => { - this.activeUserId = userId; + this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { + this.activeUserId = account?.id; }); this.ngZone.runOutsideAngular(() => { @@ -400,7 +400,8 @@ export class AppComponent implements OnInit, OnDestroy { break; case "switchAccount": { if (message.userId != null) { - await this.stateService.setActiveUser(message.userId); + await this.stateService.clearDecryptedData(message.userId); + await this.accountService.switchAccount(message.userId); } const locked = (await this.authService.getAuthStatus(message.userId)) === @@ -522,7 +523,7 @@ export class AppComponent implements OnInit, OnDestroy { private async updateAppMenu() { let updateRequest: MenuUpdateRequest; - const stateAccounts = await firstValueFrom(this.stateService.accounts$); + const stateAccounts = await firstValueFrom(this.accountService.accounts$); if (stateAccounts == null || Object.keys(stateAccounts).length < 1) { updateRequest = { accounts: null, @@ -531,32 +532,32 @@ export class AppComponent implements OnInit, OnDestroy { } else { const accounts: { [userId: string]: MenuAccount } = {}; for (const i in stateAccounts) { + const userId = i as UserId; if ( i != null && - stateAccounts[i]?.profile?.userId != null && - !this.isAccountCleanUpInProgress(stateAccounts[i].profile.userId) // skip accounts that are being cleaned up + userId != null && + !this.isAccountCleanUpInProgress(userId) // skip accounts that are being cleaned up ) { - const userId = stateAccounts[i].profile.userId; const availableTimeoutActions = await firstValueFrom( this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId), ); + const authStatus = await firstValueFrom(this.authService.authStatusFor$(userId)); accounts[userId] = { - isAuthenticated: await this.stateService.getIsAuthenticated({ - userId: userId, - }), - isLocked: - (await this.authService.getAuthStatus(userId)) === AuthenticationStatus.Locked, + isAuthenticated: authStatus >= AuthenticationStatus.Locked, + isLocked: authStatus === AuthenticationStatus.Locked, isLockable: availableTimeoutActions.includes(VaultTimeoutAction.Lock), - email: stateAccounts[i].profile.email, - userId: stateAccounts[i].profile.userId, + email: stateAccounts[userId].email, + userId: userId, hasMasterPassword: await this.userVerificationService.hasMasterPassword(userId), }; } } updateRequest = { accounts: accounts, - activeUserId: await this.stateService.getUserId(), + activeUserId: await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ), }; } @@ -564,7 +565,9 @@ export class AppComponent implements OnInit, OnDestroy { } private async logOut(expired: boolean, userId?: string) { - const userBeingLoggedOut = await this.stateService.getUserId({ userId: userId }); + const userBeingLoggedOut = + (userId as UserId) ?? + (await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id)))); // Mark account as being cleaned up so that the updateAppMenu logic (executed on syncCompleted) // doesn't attempt to update a user that is being logged out as we will manually @@ -572,9 +575,10 @@ export class AppComponent implements OnInit, OnDestroy { this.startAccountCleanUp(userBeingLoggedOut); let preLogoutActiveUserId; + const nextUpAccount = await firstValueFrom(this.accountService.nextUpAccount$); try { // Provide the userId of the user to upload events for - await this.eventUploadService.uploadEvents(userBeingLoggedOut as UserId); + await this.eventUploadService.uploadEvents(userBeingLoggedOut); await this.syncService.setLastSync(new Date(0), userBeingLoggedOut); await this.cryptoService.clearKeys(userBeingLoggedOut); await this.cipherService.clear(userBeingLoggedOut); @@ -582,22 +586,23 @@ export class AppComponent implements OnInit, OnDestroy { await this.collectionService.clear(userBeingLoggedOut); await this.passwordGenerationService.clear(userBeingLoggedOut); await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut); - await this.biometricStateService.logout(userBeingLoggedOut as UserId); + await this.biometricStateService.logout(userBeingLoggedOut); - await this.stateEventRunnerService.handleEvent("logout", userBeingLoggedOut as UserId); + await this.stateEventRunnerService.handleEvent("logout", userBeingLoggedOut); preLogoutActiveUserId = this.activeUserId; await this.stateService.clean({ userId: userBeingLoggedOut }); + await this.accountService.clean(userBeingLoggedOut); } finally { this.finishAccountCleanUp(userBeingLoggedOut); } - if (this.activeUserId == null) { + if (nextUpAccount == null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.router.navigate(["login"]); - } else if (preLogoutActiveUserId !== this.activeUserId) { - this.messagingService.send("switchAccount"); + } else if (preLogoutActiveUserId !== nextUpAccount.id) { + this.messagingService.send("switchAccount", { userId: nextUpAccount.id }); } await this.updateAppMenu(); @@ -622,13 +627,13 @@ export class AppComponent implements OnInit, OnDestroy { return; } - const now = new Date().getTime(); - if (this.lastActivity != null && now - this.lastActivity < 250) { + const now = new Date(); + if (this.lastActivity != null && now.getTime() - this.lastActivity.getTime() < 250) { return; } this.lastActivity = now; - await this.stateService.setLastActive(now, { userId: this.activeUserId }); + await this.accountService.setAccountActivity(this.activeUserId, now); // Idle states if (this.isIdle) { diff --git a/apps/desktop/src/app/layout/account-switcher.component.html b/apps/desktop/src/app/layout/account-switcher.component.html index eedafbcfe0..b5741a1a1b 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.html +++ b/apps/desktop/src/app/layout/account-switcher.component.html @@ -1,110 +1,112 @@ - - - - - -
- - - - - {{ "accountSwitcherLimitReached" | i18n }} - + + + - - + + {{ "switchAccount" | i18n }} + + + + + + + + diff --git a/apps/desktop/src/app/layout/account-switcher.component.ts b/apps/desktop/src/app/layout/account-switcher.component.ts index 4e39ab0029..c8a26065c1 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.ts +++ b/apps/desktop/src/app/layout/account-switcher.component.ts @@ -1,19 +1,17 @@ import { animate, state, style, transition, trigger } from "@angular/animations"; import { ConnectedPosition } from "@angular/cdk/overlay"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component } from "@angular/core"; import { Router } from "@angular/router"; -import { concatMap, firstValueFrom, Subject, takeUntil } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common"; +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 { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { Account } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; type ActiveAccount = { @@ -52,12 +50,18 @@ type InactiveAccount = ActiveAccount & { ]), ], }) -export class AccountSwitcherComponent implements OnInit, OnDestroy { - activeAccount?: ActiveAccount; - inactiveAccounts: { [userId: string]: InactiveAccount } = {}; - +export class AccountSwitcherComponent { + activeAccount$: Observable; + inactiveAccounts$: Observable<{ [userId: string]: InactiveAccount }>; authStatus = AuthenticationStatus; + view$: Observable<{ + activeAccount: ActiveAccount | null; + inactiveAccounts: { [userId: string]: InactiveAccount }; + numberOfAccounts: number; + showSwitcher: boolean; + }>; + isOpen = false; overlayPosition: ConnectedPosition[] = [ { @@ -68,21 +72,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { }, ]; - private destroy$ = new Subject(); + showSwitcher$: Observable; - get showSwitcher() { - const userIsInAVault = !Utils.isNullOrWhitespace(this.activeAccount?.email); - const userIsAddingAnAdditionalAccount = Object.keys(this.inactiveAccounts).length > 0; - return userIsInAVault || userIsAddingAnAdditionalAccount; - } - - get numberOfAccounts() { - if (this.inactiveAccounts == null) { - this.isOpen = false; - return 0; - } - return Object.keys(this.inactiveAccounts).length; - } + numberOfAccounts$: Observable; constructor( private stateService: StateService, @@ -90,37 +82,65 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private avatarService: AvatarService, private messagingService: MessagingService, private router: Router, - private tokenService: TokenService, private environmentService: EnvironmentService, private loginEmailService: LoginEmailServiceAbstraction, - ) {} + private accountService: AccountService, + ) { + this.activeAccount$ = this.accountService.activeAccount$.pipe( + switchMap(async (active) => { + if (active == null) { + return null; + } - async ngOnInit(): Promise { - this.stateService.accounts$ - .pipe( - concatMap(async (accounts: { [userId: string]: Account }) => { - this.inactiveAccounts = await this.createInactiveAccounts(accounts); + return { + id: active.id, + name: active.name, + email: active.email, + avatarColor: await firstValueFrom(this.avatarService.avatarColor$), + server: (await this.environmentService.getEnvironment())?.getHostname(), + }; + }), + ); + this.inactiveAccounts$ = combineLatest([ + this.activeAccount$, + this.accountService.accounts$, + this.authService.authStatuses$, + ]).pipe( + switchMap(async ([activeAccount, accounts, accountStatuses]) => { + // Filter out logged out accounts and active account + accounts = Object.fromEntries( + Object.entries(accounts).filter( + ([id]: [UserId, AccountInfo]) => + accountStatuses[id] !== AuthenticationStatus.LoggedOut || id === activeAccount?.id, + ), + ); + return this.createInactiveAccounts(accounts); + }), + ); + this.showSwitcher$ = combineLatest([this.activeAccount$, this.inactiveAccounts$]).pipe( + map(([activeAccount, inactiveAccounts]) => { + const hasActiveUser = activeAccount != null; + const userIsAddingAnAdditionalAccount = Object.keys(inactiveAccounts).length > 0; + return hasActiveUser || userIsAddingAnAdditionalAccount; + }), + ); + this.numberOfAccounts$ = this.inactiveAccounts$.pipe( + map((accounts) => Object.keys(accounts).length), + ); - try { - this.activeAccount = { - id: await this.tokenService.getUserId(), - name: (await this.tokenService.getName()) ?? (await this.tokenService.getEmail()), - email: await this.tokenService.getEmail(), - avatarColor: await firstValueFrom(this.avatarService.avatarColor$), - server: (await this.environmentService.getEnvironment())?.getHostname(), - }; - } catch { - this.activeAccount = undefined; - } - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } - - ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); + this.view$ = combineLatest([ + this.activeAccount$, + this.inactiveAccounts$, + this.numberOfAccounts$, + this.showSwitcher$, + ]).pipe( + map(([activeAccount, inactiveAccounts, numberOfAccounts, showSwitcher]) => ({ + activeAccount, + inactiveAccounts, + numberOfAccounts, + showSwitcher, + })), + ); } toggle() { @@ -144,11 +164,13 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { await this.loginEmailService.saveEmailSettings(); await this.router.navigate(["/login"]); - await this.stateService.setActiveUser(null); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + await this.stateService.clearDecryptedData(activeAccount?.id as UserId); + await this.accountService.switchAccount(null); } private async createInactiveAccounts(baseAccounts: { - [userId: string]: Account; + [userId: string]: AccountInfo; }): Promise<{ [userId: string]: InactiveAccount }> { const inactiveAccounts: { [userId: string]: InactiveAccount } = {}; @@ -159,8 +181,8 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { inactiveAccounts[userId] = { id: userId, - name: baseAccounts[userId].profile.name, - email: baseAccounts[userId].profile.email, + name: baseAccounts[userId].name, + email: baseAccounts[userId].email, authenticationStatus: await this.authService.getAuthStatus(userId), avatarColor: await firstValueFrom(this.avatarService.getUserAvatarColor$(userId as UserId)), server: (await this.environmentService.getEnvironment(userId))?.getHostname(), diff --git a/apps/desktop/src/app/layout/search/search.component.ts b/apps/desktop/src/app/layout/search/search.component.ts index 9a7226218a..06c67d8af2 100644 --- a/apps/desktop/src/app/layout/search/search.component.ts +++ b/apps/desktop/src/app/layout/search/search.component.ts @@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { UntypedFormControl } from "@angular/forms"; import { Subscription } from "rxjs"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { SearchBarService, SearchBarState } from "./search-bar.service"; @@ -18,7 +18,7 @@ export class SearchComponent implements OnInit, OnDestroy { constructor( private searchBarService: SearchBarService, - private stateService: StateService, + private accountService: AccountService, ) { // eslint-disable-next-line rxjs-angular/prefer-takeuntil this.searchBarService.state$.subscribe((state) => { @@ -33,7 +33,7 @@ export class SearchComponent implements OnInit, OnDestroy { ngOnInit() { // eslint-disable-next-line rxjs-angular/prefer-takeuntil - this.activeAccountSubscription = this.stateService.activeAccount$.subscribe((value) => { + this.activeAccountSubscription = this.accountService.activeAccount$.subscribe((_) => { this.searchBarService.setSearchText(""); this.searchText.patchValue(""); }); diff --git a/apps/desktop/src/app/services/init.service.ts b/apps/desktop/src/app/services/init.service.ts index ae2e1ba97c..0452e9be83 100644 --- a/apps/desktop/src/app/services/init.service.ts +++ b/apps/desktop/src/app/services/init.service.ts @@ -1,10 +1,12 @@ import { DOCUMENT } from "@angular/common"; import { Inject, Injectable } from "@angular/core"; +import { firstValueFrom } from "rxjs"; import { AbstractThemingService } from "@bitwarden/angular/platform/services/theming/theming.service.abstraction"; import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { EventUploadService as EventUploadServiceAbstraction } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService as NotificationsServiceAbstraction } from "@bitwarden/common/abstractions/notifications.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { TwoFactorService as TwoFactorServiceAbstraction } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; @@ -12,9 +14,10 @@ import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platfor import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; -import { UserKeyInitService } from "@bitwarden/common/platform/services/user-key-init.service"; +import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { EventUploadService } from "@bitwarden/common/services/event/event-upload.service"; import { VaultTimeoutService } from "@bitwarden/common/services/vault-timeout/vault-timeout.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { SyncService as SyncServiceAbstraction } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { I18nRendererService } from "../../platform/services/i18n.renderer.service"; @@ -36,7 +39,8 @@ export class InitService { private nativeMessagingService: NativeMessagingService, private themingService: AbstractThemingService, private encryptService: EncryptService, - private userKeyInitService: UserKeyInitService, + private userAutoUnlockKeyService: UserAutoUnlockKeyService, + private accountService: AccountService, @Inject(DOCUMENT) private document: Document, ) {} @@ -44,7 +48,18 @@ export class InitService { return async () => { this.nativeMessagingService.init(); await this.stateService.init({ runMigrations: false }); // Desktop will run them in main process - this.userKeyInitService.listenForActiveUserChangesToSetUserKey(); + + const accounts = await firstValueFrom(this.accountService.accounts$); + const setUserKeyInMemoryPromises = []; + for (const userId of Object.keys(accounts) as UserId[]) { + // For each acct, we must await the process of setting the user key in memory + // if the auto user key is set to avoid race conditions of any code trying to access + // the user key from mem. + setUserKeyInMemoryPromises.push( + this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(userId), + ); + } + await Promise.all(setUserKeyInMemoryPromises); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index a8301b3b78..b896799b47 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -61,7 +61,6 @@ import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/ge import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService } from "@bitwarden/components"; -import { LoginGuard } from "../../auth/guards/login.guard"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { Account } from "../../models/account"; import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; @@ -104,7 +103,6 @@ const safeProviders: SafeProvider[] = [ safeProvider(InitService), safeProvider(NativeMessagingService), safeProvider(SearchBarService), - safeProvider(LoginGuard), safeProvider(DialogService), safeProvider({ provide: APP_INITIALIZER as SafeInjectionToken<() => void>, @@ -194,6 +192,7 @@ const safeProviders: SafeProvider[] = [ AutofillSettingsServiceAbstraction, VaultTimeoutSettingsService, BiometricStateService, + AccountServiceAbstraction, TaskSchedulerService, ], }), diff --git a/apps/desktop/src/app/tools/generator.component.spec.ts b/apps/desktop/src/app/tools/generator.component.spec.ts index 51b5bf93a2..d908de8ef7 100644 --- a/apps/desktop/src/app/tools/generator.component.spec.ts +++ b/apps/desktop/src/app/tools/generator.component.spec.ts @@ -4,6 +4,7 @@ import { ActivatedRoute } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -59,6 +60,10 @@ describe("GeneratorComponent", () => { provide: CipherService, useValue: mock(), }, + { + provide: AccountService, + useValue: mock(), + }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/desktop/src/app/tools/send/add-edit.component.ts b/apps/desktop/src/app/tools/send/add-edit.component.ts index 7bdd5efbba..804a390438 100644 --- a/apps/desktop/src/app/tools/send/add-edit.component.ts +++ b/apps/desktop/src/app/tools/send/add-edit.component.ts @@ -4,6 +4,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -34,6 +35,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService: DialogService, formBuilder: FormBuilder, billingAccountProfileStateService: BillingAccountProfileStateService, + accountService: AccountService, ) { super( i18nService, @@ -49,6 +51,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); } diff --git a/apps/desktop/src/auth/guards/login.guard.ts b/apps/desktop/src/auth/guards/login.guard.ts deleted file mode 100644 index f6c67d5af9..0000000000 --- a/apps/desktop/src/auth/guards/login.guard.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Injectable } from "@angular/core"; -import { CanActivate } from "@angular/router"; -import { firstValueFrom } from "rxjs"; - -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; - -const maxAllowedAccounts = 5; - -@Injectable() -export class LoginGuard implements CanActivate { - protected homepage = "vault"; - constructor( - private stateService: StateService, - private platformUtilsService: PlatformUtilsService, - private i18nService: I18nService, - ) {} - - async canActivate() { - const accounts = await firstValueFrom(this.stateService.accounts$); - if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { - this.platformUtilsService.showToast("error", null, this.i18nService.t("accountLimitReached")); - return false; - } - - return true; - } -} diff --git a/apps/desktop/src/auth/guards/max-accounts.guard.ts b/apps/desktop/src/auth/guards/max-accounts.guard.ts new file mode 100644 index 0000000000..65c4ac99d0 --- /dev/null +++ b/apps/desktop/src/auth/guards/max-accounts.guard.ts @@ -0,0 +1,38 @@ +import { inject } from "@angular/core"; +import { CanActivateFn } from "@angular/router"; +import { Observable, map } from "rxjs"; + +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +const maxAllowedAccounts = 5; + +function maxAccountsGuard(): Observable { + const authService = inject(AuthService); + const toastService = inject(ToastService); + const i18nService = inject(I18nService); + + return authService.authStatuses$.pipe( + map((statuses) => + Object.values(statuses).filter((status) => status != AuthenticationStatus.LoggedOut), + ), + map((accounts) => { + if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { + toastService.showToast({ + variant: "error", + title: null, + message: i18nService.t("accountLimitReached"), + }); + return false; + } + + return true; + }), + ); +} + +export function maxAccountsGuardFn(): CanActivateFn { + return () => maxAccountsGuard(); +} diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts index f998e75d7a..2137b707f6 100644 --- a/apps/desktop/src/auth/lock.component.spec.ts +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -13,6 +13,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -50,7 +51,7 @@ describe("LockComponent", () => { let component: LockComponent; let fixture: ComponentFixture; let stateServiceMock: MockProxy; - const biometricStateService = mock(); + let biometricStateService: MockProxy; let messagingServiceMock: MockProxy; let broadcasterServiceMock: MockProxy; let platformUtilsServiceMock: MockProxy; @@ -62,7 +63,6 @@ describe("LockComponent", () => { beforeEach(async () => { stateServiceMock = mock(); - stateServiceMock.activeAccount$ = of(null); messagingServiceMock = mock(); broadcasterServiceMock = mock(); @@ -73,6 +73,7 @@ describe("LockComponent", () => { mockMasterPasswordService = new FakeMasterPasswordService(); + biometricStateService = mock(); biometricStateService.dismissedRequirePasswordOnStartCallout$ = of(false); biometricStateService.promptAutomatically$ = of(false); biometricStateService.promptCancelled$ = of(false); @@ -165,6 +166,10 @@ describe("LockComponent", () => { provide: AccountService, useValue: accountService, }, + { + provide: AuthService, + useValue: mock(), + }, { provide: KdfConfigService, useValue: mock(), diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 8e87b6663f..d95df419e1 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -10,6 +10,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -64,6 +65,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService: PinCryptoServiceAbstraction, biometricStateService: BiometricStateService, accountService: AccountService, + authService: AuthService, kdfConfigService: KdfConfigService, ) { super( @@ -89,6 +91,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService, biometricStateService, accountService, + authService, kdfConfigService, ); } diff --git a/apps/desktop/src/main/menu/menubar.ts b/apps/desktop/src/main/menu/menubar.ts index eb1dacf825..b71774c5af 100644 --- a/apps/desktop/src/main/menu/menubar.ts +++ b/apps/desktop/src/main/menu/menubar.ts @@ -65,9 +65,10 @@ export class Menubar { isLocked = updateRequest.accounts[updateRequest.activeUserId]?.isLocked ?? true; } - const isLockable = !isLocked && updateRequest?.accounts[updateRequest.activeUserId]?.isLockable; + const isLockable = + !isLocked && updateRequest?.accounts?.[updateRequest.activeUserId]?.isLockable; const hasMasterPassword = - updateRequest?.accounts[updateRequest.activeUserId]?.hasMasterPassword ?? false; + updateRequest?.accounts?.[updateRequest.activeUserId]?.hasMasterPassword ?? false; this.items = [ new FileMenu( diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index 3cccd6e28f..a67bea39c0 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -619,7 +619,7 @@ export class MemberDialogComponent implements OnDestroy { } function mapCollectionToAccessItemView( - collection: CollectionView, + collection: CollectionAdminView, organization: Organization, flexibleCollectionsV1Enabled: boolean, accessSelection?: CollectionAccessSelectionView, @@ -631,7 +631,8 @@ function mapCollectionToAccessItemView( labelName: collection.name, listName: collection.name, readonly: - group !== undefined || !collection.canEdit(organization, flexibleCollectionsV1Enabled), + group !== undefined || + !collection.canEditUserAccess(organization, flexibleCollectionsV1Enabled), readonlyPermission: accessSelection ? convertToPermission(accessSelection) : undefined, viaGroupName: group?.name, }; diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 1da2d94c15..1939bb11f5 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -2,7 +2,7 @@ import { DOCUMENT } from "@angular/common"; import { Component, Inject, NgZone, OnDestroy, OnInit } from "@angular/core"; import { NavigationEnd, Router } from "@angular/router"; import * as jq from "jquery"; -import { Subject, switchMap, takeUntil, timer } from "rxjs"; +import { Subject, firstValueFrom, map, switchMap, takeUntil, timer } from "rxjs"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; @@ -10,6 +10,7 @@ import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { PaymentMethodWarningsServiceAbstraction as PaymentMethodWarningService } from "@bitwarden/common/billing/abstractions/payment-method-warnings-service.abstraction"; @@ -51,7 +52,7 @@ const PaymentMethodWarningsRefresh = 60000; // 1 Minute templateUrl: "app.component.html", }) export class AppComponent implements OnDestroy, OnInit { - private lastActivity: number = null; + private lastActivity: Date = null; private idleTimer: number = null; private isIdle = false; private destroy$ = new Subject(); @@ -86,6 +87,7 @@ export class AppComponent implements OnDestroy, OnInit { private stateEventRunnerService: StateEventRunnerService, private paymentMethodWarningService: PaymentMethodWarningService, private organizationService: InternalOrganizationServiceAbstraction, + private accountService: AccountService, ) {} ngOnInit() { @@ -298,15 +300,16 @@ export class AppComponent implements OnDestroy, OnInit { } private async recordActivity() { - const now = new Date().getTime(); - if (this.lastActivity != null && now - this.lastActivity < 250) { + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); + const now = new Date(); + if (this.lastActivity != null && now.getTime() - this.lastActivity.getTime() < 250) { return; } this.lastActivity = now; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.stateService.setLastActive(now); + await this.accountService.setAccountActivity(activeUserId, now); // Idle states if (this.isIdle) { this.isIdle = false; diff --git a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html index 6d6691f336..b4c1224db9 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html +++ b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html @@ -42,7 +42,10 @@ : subscription.expirationWithGracePeriod ) | date: "mediumDate" }} -
+
{{ "selfHostGracePeriodHelp" | i18n: (subscription.expirationWithGracePeriod | date: "mediumDate") diff --git a/apps/web/src/app/core/init.service.ts b/apps/web/src/app/core/init.service.ts index dab6ed5e3d..55dc1544ff 100644 --- a/apps/web/src/app/core/init.service.ts +++ b/apps/web/src/app/core/init.service.ts @@ -1,17 +1,19 @@ import { DOCUMENT } from "@angular/common"; import { Inject, Injectable } from "@angular/core"; +import { firstValueFrom } from "rxjs"; import { AbstractThemingService } from "@bitwarden/angular/platform/services/theming/theming.service.abstraction"; import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { EventUploadService as EventUploadServiceAbstraction } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService as NotificationsServiceAbstraction } from "@bitwarden/common/abstractions/notifications.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { TwoFactorService as TwoFactorServiceAbstraction } from "@bitwarden/common/auth/abstractions/two-factor.service"; import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; -import { UserKeyInitService } from "@bitwarden/common/platform/services/user-key-init.service"; +import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { EventUploadService } from "@bitwarden/common/services/event/event-upload.service"; import { VaultTimeoutService } from "@bitwarden/common/services/vault-timeout/vault-timeout.service"; @@ -28,14 +30,21 @@ export class InitService { private cryptoService: CryptoServiceAbstraction, private themingService: AbstractThemingService, private encryptService: EncryptService, - private userKeyInitService: UserKeyInitService, + private userAutoUnlockKeyService: UserAutoUnlockKeyService, + private accountService: AccountService, @Inject(DOCUMENT) private document: Document, ) {} init() { return async () => { await this.stateService.init(); - this.userKeyInitService.listenForActiveUserChangesToSetUserKey(); + + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + if (activeAccount) { + // If there is an active account, we must await the process of setting the user key in memory + // if the auto user key is set to avoid race conditions of any code trying to access the user key from mem. + await this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(activeAccount.id); + } setTimeout(() => this.notificationsService.init(), 3000); await this.vaultTimeoutService.init(true); diff --git a/apps/web/src/app/layouts/header/web-header.component.html b/apps/web/src/app/layouts/header/web-header.component.html index e24013de6f..e2b3e7910a 100644 --- a/apps/web/src/app/layouts/header/web-header.component.html +++ b/apps/web/src/app/layouts/header/web-header.component.html @@ -58,7 +58,7 @@ [bitMenuTriggerFor]="accountMenu" class="tw-border-0 tw-bg-transparent tw-p-0" > - + @@ -67,7 +67,7 @@ class="tw-flex tw-items-center tw-px-4 tw-py-1 tw-leading-tight tw-text-info" appStopProp > - +
{{ "loggedInAs" | i18n }} diff --git a/apps/web/src/app/layouts/header/web-header.component.ts b/apps/web/src/app/layouts/header/web-header.component.ts index 1f012e52dd..9906bd53ba 100644 --- a/apps/web/src/app/layouts/header/web-header.component.ts +++ b/apps/web/src/app/layouts/header/web-header.component.ts @@ -1,16 +1,17 @@ import { Component, Input } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { combineLatest, map, Observable } from "rxjs"; +import { map, Observable } from "rxjs"; +import { User } from "@bitwarden/angular/pipes/user-name.pipe"; import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { AccountProfile } from "@bitwarden/common/platform/models/domain/account"; +import { UserId } from "@bitwarden/common/types/guid"; @Component({ selector: "app-header", @@ -28,7 +29,7 @@ export class WebHeaderComponent { @Input() icon: string; protected routeData$: Observable<{ titleId: string }>; - protected account$: Observable; + protected account$: Observable; protected canLock$: Observable; protected selfHosted: boolean; protected hostname = location.hostname; @@ -38,12 +39,12 @@ export class WebHeaderComponent { constructor( private route: ActivatedRoute, - private stateService: StateService, private platformUtilsService: PlatformUtilsService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private messagingService: MessagingService, protected unassignedItemsBannerService: UnassignedItemsBannerService, private configService: ConfigService, + private accountService: AccountService, ) { this.routeData$ = this.route.data.pipe( map((params) => { @@ -55,14 +56,7 @@ export class WebHeaderComponent { this.selfHosted = this.platformUtilsService.isSelfHost(); - this.account$ = combineLatest([ - this.stateService.activeAccount$, - this.stateService.accounts$, - ]).pipe( - map(([activeAccount, accounts]) => { - return accounts[activeAccount]?.profile; - }), - ); + this.account$ = this.accountService.activeAccount$; this.canLock$ = this.vaultTimeoutSettingsService .availableVaultTimeoutActions$() .pipe(map((actions) => actions.includes(VaultTimeoutAction.Lock))); diff --git a/apps/web/src/app/tools/send/add-edit.component.ts b/apps/web/src/app/tools/send/add-edit.component.ts index ee4be41488..cca416db9c 100644 --- a/apps/web/src/app/tools/send/add-edit.component.ts +++ b/apps/web/src/app/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -40,6 +41,7 @@ export class AddEditComponent extends BaseAddEditComponent { billingAccountProfileStateService: BillingAccountProfileStateService, protected dialogRef: DialogRef, @Inject(DIALOG_DATA) params: { sendId: string }, + accountService: AccountService, ) { super( i18nService, @@ -55,6 +57,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); this.sendId = params.sendId; diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index ad80c9f4e5..41aa766e3a 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -55,7 +55,6 @@ export default { { provide: StateService, useValue: { - activeAccount$: new BehaviorSubject("1").asObservable(), accounts$: new BehaviorSubject({ "1": { profile: { name: "Foo" } } }).asObservable(), async getShowFavicon() { return true; diff --git a/apps/web/src/app/vault/core/views/collection-admin.view.ts b/apps/web/src/app/vault/core/views/collection-admin.view.ts index d942d42fb8..369c78636c 100644 --- a/apps/web/src/app/vault/core/views/collection-admin.view.ts +++ b/apps/web/src/app/vault/core/views/collection-admin.view.ts @@ -31,6 +31,9 @@ export class CollectionAdminView extends CollectionView { this.assigned = response.assigned; } + /** + * Whether the current user can edit the collection, including user and group access + */ override canEdit(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean { return org?.flexibleCollections ? org?.canEditAnyCollection(flexibleCollectionsV1Enabled) || this.manage @@ -43,4 +46,11 @@ export class CollectionAdminView extends CollectionView { ? org?.canDeleteAnyCollection || (!org?.limitCollectionCreationDeletion && this.manage) : org?.canDeleteAnyCollection || (org?.canDeleteAssignedCollections && this.assigned); } + + /** + * Whether the user can modify user access to this collection + */ + canEditUserAccess(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean { + return this.canEdit(org, flexibleCollectionsV1Enabled) || org.canManageUsers; + } } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html index 9a47f42686..3f107486e2 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html @@ -1,4 +1,4 @@ -
+

{{ "projectPeopleDescription" | i18n }} @@ -19,3 +19,9 @@

+ + +
+ +
+ diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts index b52a8938b4..835d3825a0 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core"; import { FormControl, FormGroup } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; -import { combineLatest, Subject, switchMap, takeUntil, catchError, EMPTY } from "rxjs"; +import { combineLatest, Subject, switchMap, takeUntil, catchError } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -37,11 +37,9 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { return convertToAccessPolicyItemViews(policies); }), ), - catchError(() => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["/sm", this.organizationId, "projects"]); - return EMPTY; + catchError(async () => { + await this.router.navigate(["/sm", this.organizationId, "projects"]); + return []; }), ); @@ -99,17 +97,20 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { if (this.formGroup.invalid) { return; } + const formValues = this.formGroup.value.accessPolicies; + this.formGroup.disable(); const showAccessRemovalWarning = await this.accessPolicySelectorService.showAccessRemovalWarning( this.organizationId, - this.formGroup.value.accessPolicies, + formValues, ); if (showAccessRemovalWarning) { const confirmed = await this.showWarning(); if (!confirmed) { this.setSelected(this.currentAccessPolicies); + this.formGroup.enable(); return; } } @@ -117,7 +118,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { try { const projectPeopleView = convertToProjectPeopleAccessPoliciesView( this.projectId, - this.formGroup.value.accessPolicies, + formValues, ); const peoplePoliciesViews = await this.accessPolicyService.putProjectPeopleAccessPolicies( this.projectId, @@ -126,9 +127,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { this.currentAccessPolicies = convertToAccessPolicyItemViews(peoplePoliciesViews); if (showAccessRemovalWarning) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["sm", this.organizationId, "projects"]); + await this.router.navigate(["sm", this.organizationId, "projects"]); } this.platformUtilsService.showToast( "success", @@ -139,6 +138,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { this.validationService.showError(e); this.setSelected(this.currentAccessPolicies); } + this.formGroup.enable(); }; private setSelected(policiesToSelect: ApItemViewType[]) { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html index 074fa8ca00..96f7ae4d2b 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html @@ -1,4 +1,4 @@ -
+

{{ "machineAccountPeopleDescription" | i18n }} @@ -20,3 +20,9 @@

+ + +
+ +
+ diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts index aeb124aa6a..a3d3984ea8 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core"; import { FormControl, FormGroup } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; -import { catchError, combineLatest, EMPTY, Subject, switchMap, takeUntil } from "rxjs"; +import { combineLatest, Subject, switchMap, takeUntil } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -40,12 +40,6 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy { return convertToAccessPolicyItemViews(policies); }), ), - catchError(() => { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["/sm", this.organizationId, "machine-accounts"]); - return EMPTY; - }), ); private potentialGrantees$ = combineLatest([this.route.params]).pipe( @@ -101,29 +95,32 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy { if (this.isFormInvalid()) { return; } + const formValues = this.formGroup.value.accessPolicies; + this.formGroup.disable(); const showAccessRemovalWarning = await this.accessPolicySelectorService.showAccessRemovalWarning( this.organizationId, - this.formGroup.value.accessPolicies, + formValues, ); if ( await this.handleAccessRemovalWarning(showAccessRemovalWarning, this.currentAccessPolicies) ) { + this.formGroup.enable(); return; } try { const peoplePoliciesViews = await this.updateServiceAccountPeopleAccessPolicies( this.serviceAccountId, - this.formGroup.value.accessPolicies, + formValues, ); await this.handleAccessTokenAvailableWarning( showAccessRemovalWarning, this.currentAccessPolicies, - this.formGroup.value.accessPolicies, + formValues, ); this.currentAccessPolicies = convertToAccessPolicyItemViews(peoplePoliciesViews); @@ -137,6 +134,7 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy { this.validationService.showError(e); this.setSelected(this.currentAccessPolicies); } + this.formGroup.enable(); }; private setSelected(policiesToSelect: ApItemViewType[]) { @@ -198,9 +196,7 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy { selectedPolicies: ApItemValueType[], ): Promise { if (showAccessRemovalWarning) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["sm", this.organizationId, "machine-accounts"]); + await this.router.navigate(["sm", this.organizationId, "machine-accounts"]); } else if ( this.accessPolicySelectorService.isAccessRemoval(currentAccessPolicies, selectedPolicies) ) { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.component.html index 4b3c839264..e1faf2a185 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/access-policies/access-policy-selector/access-policy-selector.component.html @@ -55,6 +55,7 @@ bitIconButton="bwi-close" buttonType="main" size="default" + [disabled]="disabled" [attr.title]="'remove' | i18n" [attr.aria-label]="'remove' | i18n" (click)="selectionList.deselectItem(item.id); handleBlur()" @@ -84,7 +85,14 @@
-
diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 89af31da81..7eb30d759a 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -1,7 +1,7 @@ import { Directive, NgZone, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; import { firstValueFrom, Subject } from "rxjs"; -import { concatMap, take, takeUntil } from "rxjs/operators"; +import { concatMap, map, take, takeUntil } from "rxjs/operators"; import { PinCryptoServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -11,10 +11,12 @@ import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abs import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SecretVerificationRequest } from "@bitwarden/common/auth/models/request/secret-verification.request"; import { MasterPasswordPolicyResponse } from "@bitwarden/common/auth/models/response/master-password-policy.response"; @@ -30,6 +32,7 @@ import { BiometricStateService } from "@bitwarden/common/platform/biometrics/bio import { HashPurpose, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { PinLockType } from "@bitwarden/common/services/vault-timeout/vault-timeout-settings.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { DialogService } from "@bitwarden/components"; @@ -46,6 +49,7 @@ export class LockComponent implements OnInit, OnDestroy { supportsBiometric: boolean; biometricLock: boolean; + private activeUserId: UserId; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; protected onSuccessfulSubmit: () => Promise; @@ -80,14 +84,16 @@ export class LockComponent implements OnInit, OnDestroy { protected pinCryptoService: PinCryptoServiceAbstraction, protected biometricStateService: BiometricStateService, protected accountService: AccountService, + protected authService: AuthService, protected kdfConfigService: KdfConfigService, ) {} async ngOnInit() { - this.stateService.activeAccount$ + this.accountService.activeAccount$ .pipe( - concatMap(async () => { - await this.load(); + concatMap(async (account) => { + this.activeUserId = account?.id; + await this.load(account?.id); }), takeUntil(this.destroy$), ) @@ -116,7 +122,7 @@ export class LockComponent implements OnInit, OnDestroy { }); if (confirmed) { - this.messagingService.send("logout"); + this.messagingService.send("logout", { userId: this.activeUserId }); } } @@ -321,23 +327,35 @@ export class LockComponent implements OnInit, OnDestroy { } } - private async load() { + private async load(userId: UserId) { // TODO: Investigate PM-3515 // The loading of the lock component works as follows: - // 1. First, is locking a valid timeout action? If not, we will log the user out. - // 2. If locking IS a valid timeout action, we proceed to show the user the lock screen. + // 1. If the user is unlocked, we're here in error so we navigate to the home page + // 2. First, is locking a valid timeout action? If not, we will log the user out. + // 3. If locking IS a valid timeout action, we proceed to show the user the lock screen. // The user will be able to unlock as follows: // - If they have a PIN set, they will be presented with the PIN input // - If they have a master password and no PIN, they will be presented with the master password input // - If they have biometrics enabled, they will be presented with the biometric prompt + const isUnlocked = await firstValueFrom( + this.authService + .authStatusFor$(userId) + .pipe(map((status) => status === AuthenticationStatus.Unlocked)), + ); + if (isUnlocked) { + // navigate to home + await this.router.navigate(["/"]); + return; + } + const availableVaultTimeoutActions = await firstValueFrom( - this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId), ); const supportsLock = availableVaultTimeoutActions.includes(VaultTimeoutAction.Lock); if (!supportsLock) { - return await this.vaultTimeoutService.logOut(); + return await this.vaultTimeoutService.logOut(userId); } this.pinStatus = await this.vaultTimeoutSettingsService.isPinLockSet(); diff --git a/libs/angular/src/pipes/user-name.pipe.ts b/libs/angular/src/pipes/user-name.pipe.ts index 88b088a7e2..f007f4ad87 100644 --- a/libs/angular/src/pipes/user-name.pipe.ts +++ b/libs/angular/src/pipes/user-name.pipe.ts @@ -1,6 +1,6 @@ import { Pipe, PipeTransform } from "@angular/core"; -interface User { +export interface User { name?: string; email?: string; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index fc3b861ff9..1d5ab72d01 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -53,7 +53,6 @@ import { ProviderApiService } from "@bitwarden/common/admin-console/services/pro import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountApiService as AccountApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/account-api.service"; import { - AccountService, AccountService as AccountServiceAbstraction, InternalAccountService, } from "@bitwarden/common/auth/abstractions/account.service"; @@ -164,7 +163,7 @@ import { MigrationRunner } from "@bitwarden/common/platform/services/migration-r import { NoopNotificationsService } from "@bitwarden/common/platform/services/noop-notifications.service"; import { StateService } from "@bitwarden/common/platform/services/state.service"; import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider"; -import { UserKeyInitService } from "@bitwarden/common/platform/services/user-key-init.service"; +import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; import { ValidationService } from "@bitwarden/common/platform/services/validation.service"; import { WebCryptoFunctionService } from "@bitwarden/common/platform/services/web-crypto-function.service"; import { @@ -1138,9 +1137,9 @@ const safeProviders: SafeProvider[] = [ deps: [StateProvider], }), safeProvider({ - provide: UserKeyInitService, - useClass: UserKeyInitService, - deps: [AccountService, CryptoServiceAbstraction, LogService], + provide: UserAutoUnlockKeyService, + useClass: UserAutoUnlockKeyService, + deps: [CryptoServiceAbstraction], }), safeProvider({ provide: ErrorHandler, diff --git a/libs/angular/src/tools/send/add-edit.component.ts b/libs/angular/src/tools/send/add-edit.component.ts index da859e50bf..b4f7ec171a 100644 --- a/libs/angular/src/tools/send/add-edit.component.ts +++ b/libs/angular/src/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { Subject, firstValueFrom, takeUntil, map, BehaviorSubject, concatMap } f import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -118,6 +119,7 @@ export class AddEditComponent implements OnInit, OnDestroy { protected dialogService: DialogService, protected formBuilder: FormBuilder, protected billingAccountProfileStateService: BillingAccountProfileStateService, + protected accountService: AccountService, ) { this.typeOptions = [ { name: i18nService.t("sendTypeFile"), value: SendType.File, premium: true }, @@ -215,7 +217,9 @@ export class AddEditComponent implements OnInit, OnDestroy { } async load() { - this.emailVerified = await this.stateService.getEmailVerified(); + this.emailVerified = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.emailVerified ?? false)), + ); this.type = !this.canAccessPremium || !this.emailVerified ? SendType.Text : SendType.File; if (this.send == null) { diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index a123e30053..0efb9569eb 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -128,6 +128,7 @@ describe("AuthRequestLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: mockUserId }); await authRequestLoginStrategy.logIn(credentials); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 3284f6e947..c3a8f61d78 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -218,7 +218,7 @@ describe("LoginStrategy", () => { expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); }); - it("throws if active account isn't found after being initialized", async () => { + it("throws if new account isn't active after being initialized", async () => { const idTokenResponse = identityTokenResponseFactory(); apiService.postIdentityToken.mockResolvedValue(idTokenResponse); @@ -228,7 +228,8 @@ describe("LoginStrategy", () => { stateService.getVaultTimeoutAction.mockResolvedValue(mockVaultTimeoutAction); stateService.getVaultTimeout.mockResolvedValue(mockVaultTimeout); - accountService.activeAccountSubject.next(null); + accountService.switchAccount = jest.fn(); // block internal switch to new account + accountService.activeAccountSubject.next(null); // simulate no active account await expect(async () => await passwordLoginStrategy.logIn(credentials)).rejects.toThrow(); }); diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index fd268d955e..3a3109349e 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -169,6 +169,12 @@ export abstract class LoginStrategy { const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId }); const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); + await this.accountService.addAccount(userId, { + name: accountInformation.name, + email: accountInformation.email, + emailVerified: accountInformation.email_verified, + }); + // set access token and refresh token before account initialization so authN status can be accurate // User id will be derived from the access token. await this.tokenService.setTokens( @@ -178,6 +184,8 @@ export abstract class LoginStrategy { tokenResponse.refreshToken, // Note: CLI login via API key sends undefined for refresh token. ); + await this.accountService.switchAccount(userId); + await this.stateService.addAccount( new Account({ profile: { diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index 5c1fe9b1fe..c97639f102 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -164,6 +164,7 @@ describe("PasswordLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); await passwordLoginStrategy.logIn(credentials); @@ -199,6 +200,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const result = await passwordLoginStrategy.logIn(credentials); @@ -213,6 +215,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const token2FAResponse = new IdentityTwoFactorResponse({ TwoFactorProviders: ["0"], 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 16479f19ea..ae1813d3d7 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 @@ -65,6 +65,7 @@ describe("UserDecryptionOptionsService", () => { await fakeAccountService.addAccount(givenUser, { name: "Test User 1", email: "test1@email.com", + emailVerified: false, }); 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 a8b09b7417..649a158d75 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { ReplaySubject } from "rxjs"; +import { ReplaySubject, combineLatest, map } from "rxjs"; import { AccountInfo, AccountService } from "../src/auth/abstractions/account.service"; import { UserId } from "../src/types/guid"; @@ -7,15 +7,20 @@ import { UserId } from "../src/types/guid"; export function mockAccountServiceWith( userId: UserId, info: Partial = {}, + activity: Record = {}, ): FakeAccountService { const fullInfo: AccountInfo = { ...info, ...{ name: "name", email: "email", + emailVerified: true, }, }; - const service = new FakeAccountService({ [userId]: fullInfo }); + + const fullActivity = { [userId]: new Date(), ...activity }; + + const service = new FakeAccountService({ [userId]: fullInfo }, fullActivity); service.activeAccountSubject.next({ id: userId, ...fullInfo }); return service; } @@ -26,17 +31,46 @@ export class FakeAccountService implements AccountService { accountsSubject = new ReplaySubject>(1); // eslint-disable-next-line rxjs/no-exposed-subjects -- test class activeAccountSubject = new ReplaySubject<{ id: UserId } & AccountInfo>(1); + // eslint-disable-next-line rxjs/no-exposed-subjects -- test class + accountActivitySubject = new ReplaySubject>(1); private _activeUserId: UserId; get activeUserId() { return this._activeUserId; } accounts$ = this.accountsSubject.asObservable(); activeAccount$ = this.activeAccountSubject.asObservable(); + accountActivity$ = this.accountActivitySubject.asObservable(); + get sortedUserIds$() { + return this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => a.lastActive.getTime() - b.lastActive.getTime()) + .map((a) => a.userId); + }), + ); + } + get nextUpAccount$() { + return combineLatest([this.accounts$, this.activeAccount$, this.sortedUserIds$]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); + } - constructor(initialData: Record) { + constructor(initialData: Record, accountActivity?: Record) { this.accountsSubject.next(initialData); this.activeAccountSubject.subscribe((data) => (this._activeUserId = data?.id)); this.activeAccountSubject.next(null); + this.accountActivitySubject.next(accountActivity); + } + setAccountActivity(userId: UserId, lastActivity: Date): Promise { + this.accountActivitySubject.next({ + ...this.accountActivitySubject["_buffer"][0], + [userId]: lastActivity, + }); + return this.mock.setAccountActivity(userId, lastActivity); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { @@ -53,10 +87,27 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountEmail(userId, email); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.mock.setAccountEmailVerified(userId, emailVerified); + } + async switchAccount(userId: UserId): Promise { const next = userId == null ? null : { id: userId, ...this.accountsSubject["_buffer"]?.[0]?.[userId] }; this.activeAccountSubject.next(next); await this.mock.switchAccount(userId); } + + async clean(userId: UserId): Promise { + const current = this.accountsSubject["_buffer"][0] ?? {}; + const updated = { ...current, [userId]: loggedOutInfo }; + this.accountsSubject.next(updated); + await this.mock.clean(userId); + } } + +const loggedOutInfo: AccountInfo = { + name: undefined, + email: "", + emailVerified: false, +}; diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index fa9ad36378..b7fd6d9bb9 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -8,18 +8,44 @@ import { UserId } from "../../types/guid"; */ export type AccountInfo = { email: string; + emailVerified: boolean; name: string | undefined; }; export function accountInfoEqual(a: AccountInfo, b: AccountInfo) { - return a?.email === b?.email && a?.name === b?.name; + if (a == null && b == null) { + return true; + } + + if (a == null || b == null) { + return false; + } + + const keys = new Set([...Object.keys(a), ...Object.keys(b)]) as Set; + for (const key of keys) { + if (a[key] !== b[key]) { + return false; + } + } + return true; } export abstract class AccountService { accounts$: Observable>; activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; + + /** + * Observable of the last activity time for each account. + */ + accountActivity$: Observable>; + /** Account list in order of descending recency */ + sortedUserIds$: Observable; + /** Next account that is not the current active account */ + nextUpAccount$: Observable<{ id: UserId } & AccountInfo>; /** * Updates the `accounts$` observable with the new account data. + * + * @note Also sets the last active date of the account to `now`. * @param userId * @param accountData */ @@ -36,11 +62,30 @@ export abstract class AccountService { * @param email */ abstract setAccountEmail(userId: UserId, email: string): Promise; + /** + * updates the `accounts$` observable with the new email verification status for the account. + * @param userId + * @param emailVerified + */ + abstract setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise; /** * Updates the `activeAccount$` observable with the new active account. * @param userId */ abstract switchAccount(userId: UserId): Promise; + /** + * Cleans personal information for the given account from the `accounts$` observable. Does not remove the userId from the observable. + * + * @note Also sets the last active date of the account to `null`. + * @param userId + */ + abstract clean(userId: UserId): Promise; + /** + * Updates the given user's last activity time. + * @param userId + * @param lastActivity + */ + abstract setAccountActivity(userId: UserId, lastActivity: Date): Promise; } export abstract class InternalAccountService extends AccountService { diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index a9cec82c51..0ae14b0cc1 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -1,3 +1,8 @@ +/** + * need to update test environment so structuredClone works appropriately + * @jest-environment ../../libs/shared/test.environment.ts + */ + import { MockProxy, mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; @@ -6,15 +11,57 @@ import { FakeGlobalStateProvider } from "../../../spec/fake-state-provider"; import { trackEmissions } from "../../../spec/utils"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; -import { AccountInfo } from "../abstractions/account.service"; +import { AccountInfo, accountInfoEqual } from "../abstractions/account.service"; import { ACCOUNT_ACCOUNTS, ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, AccountServiceImplementation, } from "./account.service"; +describe("accountInfoEqual", () => { + const accountInfo: AccountInfo = { name: "name", email: "email", emailVerified: true }; + + it("compares nulls", () => { + expect(accountInfoEqual(null, null)).toBe(true); + expect(accountInfoEqual(null, accountInfo)).toBe(false); + expect(accountInfoEqual(accountInfo, null)).toBe(false); + }); + + it("compares all keys, not just those defined in AccountInfo", () => { + const different = { ...accountInfo, extra: "extra" }; + + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares name", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, name: "name2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares email", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, email: "email2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares emailVerified", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, emailVerified: false }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); +}); + describe("accountService", () => { let messagingService: MockProxy; let logService: MockProxy; @@ -22,8 +69,8 @@ describe("accountService", () => { let sut: AccountServiceImplementation; let accountsState: FakeGlobalState>; let activeAccountIdState: FakeGlobalState; - const userId = "userId" as UserId; - const userInfo = { email: "email", name: "name" }; + const userId = Utils.newGuid() as UserId; + const userInfo = { email: "email", name: "name", emailVerified: true }; beforeEach(() => { messagingService = mock(); @@ -86,6 +133,25 @@ describe("accountService", () => { expect(currentValue).toEqual({ [userId]: userInfo }); }); + + it("sets the last active date of the account to now", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await sut.addAccount(userId, userInfo); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: expect.any(Date) }); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await expect(sut.addAccount(userId as UserId, userInfo)).rejects.toThrow( + "userId is required", + ); + }, + ); }); describe("setAccountName", () => { @@ -134,6 +200,58 @@ describe("accountService", () => { }); }); + describe("setAccountEmailVerified", () => { + const initialState = { [userId]: userInfo }; + initialState[userId].emailVerified = false; + beforeEach(() => { + accountsState.stateSubject.next(initialState); + }); + + it("should update the account", async () => { + await sut.setAccountEmailVerified(userId, true); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { ...userInfo, emailVerified: true }, + }); + }); + + it("should not update if the email is the same", async () => { + await sut.setAccountEmailVerified(userId, false); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual(initialState); + }); + }); + + describe("clean", () => { + beforeEach(() => { + accountsState.stateSubject.next({ [userId]: userInfo }); + }); + + it("removes account info of the given user", async () => { + await sut.clean(userId); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + }); + + it("removes account activity of the given user", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({ [userId]: new Date() }); + + await sut.clean(userId); + + expect(state.nextMock).toHaveBeenCalledWith({}); + }); + }); + describe("switchAccount", () => { beforeEach(() => { accountsState.stateSubject.next({ [userId]: userInfo }); @@ -152,4 +270,83 @@ describe("accountService", () => { expect(sut.switchAccount("unknown" as UserId)).rejects.toThrowError("Account does not exist"); }); }); + + describe("account activity", () => { + let state: FakeGlobalState>; + + beforeEach(() => { + state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + }); + describe("accountActivity$", () => { + it("returns the account activity state", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + }); + + it("returns an empty object when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({}); + }); + }); + + describe("sortedUserIds$", () => { + it("returns the sorted user ids by date with most recent first", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(3), + [toId("user2")]: new Date(2), + [toId("user3")]: new Date(1), + }); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([ + "user1" as UserId, + "user2" as UserId, + "user3" as UserId, + ]); + }); + + it("returns an empty array when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([]); + }); + }); + + describe("setAccountActivity", () => { + const userId = Utils.newGuid() as UserId; + it("sets the account activity", async () => { + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: new Date(1) }); + }); + + it("does not update if the activity is the same", async () => { + state.stateSubject.next({ [userId]: new Date(1) }); + + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + await sut.setAccountActivity(userId as UserId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }, + ); + }); + }); }); + +function toId(userId: string) { + return userId as UserId; +} diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 77d61fae91..6740387ded 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -1,4 +1,4 @@ -import { Subject, combineLatestWith, map, distinctUntilChanged, shareReplay } from "rxjs"; +import { combineLatestWith, map, distinctUntilChanged, shareReplay, combineLatest } from "rxjs"; import { AccountInfo, @@ -7,8 +7,9 @@ import { } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { - ACCOUNT_MEMORY, + ACCOUNT_DISK, GlobalState, GlobalStateProvider, KeyDefinition, @@ -16,25 +17,36 @@ import { import { UserId } from "../../types/guid"; export const ACCOUNT_ACCOUNTS = KeyDefinition.record( - ACCOUNT_MEMORY, + ACCOUNT_DISK, "accounts", { deserializer: (accountInfo) => accountInfo, }, ); -export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_MEMORY, "activeAccountId", { +export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_DISK, "activeAccountId", { deserializer: (id: UserId) => id, }); +export const ACCOUNT_ACTIVITY = KeyDefinition.record(ACCOUNT_DISK, "activity", { + deserializer: (activity) => new Date(activity), +}); + +const LOGGED_OUT_INFO: AccountInfo = { + email: "", + emailVerified: false, + name: undefined, +}; + export class AccountServiceImplementation implements InternalAccountService { - private lock = new Subject(); - private logout = new Subject(); private accountsState: GlobalState>; private activeAccountIdState: GlobalState; accounts$; activeAccount$; + accountActivity$; + sortedUserIds$; + nextUpAccount$; constructor( private messagingService: MessagingService, @@ -53,14 +65,40 @@ export class AccountServiceImplementation implements InternalAccountService { distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)), shareReplay({ bufferSize: 1, refCount: false }), ); + this.accountActivity$ = this.globalStateProvider + .get(ACCOUNT_ACTIVITY) + .state$.pipe(map((activity) => activity ?? {})); + this.sortedUserIds$ = this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => b.lastActive.getTime() - a.lastActive.getTime()) // later dates first + .map((a) => a.userId); + }), + ); + this.nextUpAccount$ = combineLatest([ + this.accounts$, + this.activeAccount$, + this.sortedUserIds$, + ]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { + if (!Utils.isGuid(userId)) { + throw new Error("userId is required"); + } + await this.accountsState.update((accounts) => { accounts ||= {}; accounts[userId] = accountData; return accounts; }); + await this.setAccountActivity(userId, new Date()); } async setAccountName(userId: UserId, name: string): Promise { @@ -71,6 +109,15 @@ export class AccountServiceImplementation implements InternalAccountService { await this.setAccountInfo(userId, { email }); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.setAccountInfo(userId, { emailVerified }); + } + + async clean(userId: UserId) { + await this.setAccountInfo(userId, LOGGED_OUT_INFO); + await this.removeAccountActivity(userId); + } + async switchAccount(userId: UserId): Promise { await this.activeAccountIdState.update( (_, accounts) => { @@ -94,6 +141,37 @@ export class AccountServiceImplementation implements InternalAccountService { ); } + async setAccountActivity(userId: UserId, lastActivity: Date): Promise { + if (!Utils.isGuid(userId)) { + // only store for valid userIds + return; + } + + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + activity ||= {}; + activity[userId] = lastActivity; + return activity; + }, + { + shouldUpdate: (oldActivity) => oldActivity?.[userId]?.getTime() !== lastActivity?.getTime(), + }, + ); + } + + async removeAccountActivity(userId: UserId): Promise { + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + if (activity == null) { + return activity; + } + delete activity[userId]; + return activity; + }, + { shouldUpdate: (oldActivity) => oldActivity?.[userId] != null }, + ); + } + // TODO: update to use our own account status settings. Requires inverting direction of state service accounts flow async delete(): Promise { try { diff --git a/libs/common/src/auth/services/auth.service.spec.ts b/libs/common/src/auth/services/auth.service.spec.ts index 3bdf85d3e1..9a93a4207b 100644 --- a/libs/common/src/auth/services/auth.service.spec.ts +++ b/libs/common/src/auth/services/auth.service.spec.ts @@ -56,6 +56,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: userId, email: "email", + emailVerified: false, name: "name", }; @@ -109,6 +110,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: Utils.newGuid() as UserId, email: "email2", + emailVerified: false, name: "name2", }; @@ -126,7 +128,11 @@ describe("AuthService", () => { it("requests auth status for all known users", async () => { const userId2 = Utils.newGuid() as UserId; - await accountService.addAccount(userId2, { email: "email2", name: "name2" }); + await accountService.addAccount(userId2, { + email: "email2", + emailVerified: false, + name: "name2", + }); const mockFn = jest.fn().mockReturnValue(of(AuthenticationStatus.Locked)); sut.authStatusFor$ = mockFn; @@ -147,11 +153,14 @@ describe("AuthService", () => { cryptoService.getInMemoryUserKeyFor$.mockReturnValue(of(undefined)); }); - it("emits LoggedOut when userId is null", async () => { - expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( - AuthenticationStatus.LoggedOut, - ); - }); + it.each([null, undefined, "not a userId"])( + "emits LoggedOut when userId is invalid (%s)", + async () => { + expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( + AuthenticationStatus.LoggedOut, + ); + }, + ); it("emits LoggedOut when there is no access token", async () => { tokenService.hasAccessToken$.mockReturnValue(of(false)); diff --git a/libs/common/src/auth/services/auth.service.ts b/libs/common/src/auth/services/auth.service.ts index c9e711b4cc..a4529084a2 100644 --- a/libs/common/src/auth/services/auth.service.ts +++ b/libs/common/src/auth/services/auth.service.ts @@ -2,6 +2,7 @@ import { Observable, combineLatest, distinctUntilChanged, + firstValueFrom, map, of, shareReplay, @@ -12,6 +13,7 @@ import { ApiService } from "../../abstractions/api.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { StateService } from "../../platform/abstractions/state.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; import { AccountService } from "../abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "../abstractions/auth.service"; @@ -39,13 +41,16 @@ export class AuthService implements AuthServiceAbstraction { this.authStatuses$ = this.accountService.accounts$.pipe( map((accounts) => Object.keys(accounts) as UserId[]), - switchMap((entries) => - combineLatest( + switchMap((entries) => { + if (entries.length === 0) { + return of([] as { userId: UserId; status: AuthenticationStatus }[]); + } + return combineLatest( entries.map((userId) => this.authStatusFor$(userId).pipe(map((status) => ({ userId, status }))), ), - ), - ), + ); + }), map((statuses) => { return statuses.reduce( (acc, { userId, status }) => { @@ -59,7 +64,7 @@ export class AuthService implements AuthServiceAbstraction { } authStatusFor$(userId: UserId): Observable { - if (userId == null) { + if (!Utils.isGuid(userId)) { return of(AuthenticationStatus.LoggedOut); } @@ -84,17 +89,8 @@ export class AuthService implements AuthServiceAbstraction { } async getAuthStatus(userId?: string): Promise { - // If we don't have an access token or userId, we're logged out - const isAuthenticated = await this.stateService.getIsAuthenticated({ userId: userId }); - if (!isAuthenticated) { - return AuthenticationStatus.LoggedOut; - } - - // Note: since we aggresively set the auto user key to memory if it exists on app init (see InitService) - // we only need to check if the user key is in memory. - const hasUserKey = await this.cryptoService.hasUserKeyInMemory(userId as UserId); - - return hasUserKey ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; + userId ??= await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); + return await firstValueFrom(this.authStatusFor$(userId as UserId)); } logOut(callback: () => void) { 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 fc5060af5f..19b29f0593 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 @@ -90,6 +90,7 @@ describe("PasswordResetEnrollmentServiceImplementation", () => { const user1AccountInfo: AccountInfo = { name: "Test User 1", email: "test1@email.com", + emailVerified: true, }; activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "userId" as UserId })); diff --git a/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts b/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts index c1f5640207..7b49688294 100644 --- a/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts +++ b/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts @@ -58,4 +58,16 @@ export class SelfHostedOrganizationSubscriptionView implements View { get isExpiredAndOutsideGracePeriod() { return this.hasExpiration && this.expirationWithGracePeriod < new Date(); } + + /** + * In the case of a trial, where there is no grace period, the expirationWithGracePeriod and expirationWithoutGracePeriod will + * be exactly the same. This can be used to hide the grace period note. + */ + get isInTrial() { + return ( + this.expirationWithGracePeriod && + this.expirationWithoutGracePeriod && + this.expirationWithGracePeriod.getTime() === this.expirationWithoutGracePeriod.getTime() + ); + } } diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 13c33305d1..5ca604b526 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -25,11 +25,10 @@ export type InitOptions = { export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; - activeAccount$: Observable; addAccount: (account: T) => Promise; - setActiveUser: (userId: string) => Promise; - clean: (options?: StorageOptions) => Promise; + clearDecryptedData: (userId: UserId) => Promise; + clean: (options?: StorageOptions) => Promise; init: (initOptions?: InitOptions) => Promise; /** @@ -122,8 +121,6 @@ export abstract class StateService { setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; getEmail: (options?: StorageOptions) => Promise; setEmail: (value: string, options?: StorageOptions) => Promise; - getEmailVerified: (options?: StorageOptions) => Promise; - setEmailVerified: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegration: (options?: StorageOptions) => Promise; setEnableBrowserIntegration: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegrationFingerprint: (options?: StorageOptions) => Promise; @@ -147,8 +144,6 @@ export abstract class StateService { */ setEncryptedPinProtected: (value: string, options?: StorageOptions) => Promise; getIsAuthenticated: (options?: StorageOptions) => Promise; - getLastActive: (options?: StorageOptions) => Promise; - setLastActive: (value: number, options?: StorageOptions) => Promise; getLastSync: (options?: StorageOptions) => Promise; setLastSync: (value: string, options?: StorageOptions) => Promise; getMinimizeOnCopyToClipboard: (options?: StorageOptions) => Promise; @@ -180,5 +175,4 @@ export abstract class StateService { setVaultTimeout: (value: number, options?: StorageOptions) => Promise; getVaultTimeoutAction: (options?: StorageOptions) => Promise; setVaultTimeoutAction: (value: string, options?: StorageOptions) => Promise; - nextUpActiveUser: () => Promise; } diff --git a/libs/common/src/platform/misc/utils.spec.ts b/libs/common/src/platform/misc/utils.spec.ts index a7a520a77c..964a2a1941 100644 --- a/libs/common/src/platform/misc/utils.spec.ts +++ b/libs/common/src/platform/misc/utils.spec.ts @@ -3,6 +3,33 @@ import * as path from "path"; import { Utils } from "./utils"; describe("Utils Service", () => { + describe("isGuid", () => { + it("is false when null", () => { + expect(Utils.isGuid(null)).toBe(false); + }); + + it("is false when undefined", () => { + expect(Utils.isGuid(undefined)).toBe(false); + }); + + it("is false when empty", () => { + expect(Utils.isGuid("")).toBe(false); + }); + + it("is false when not a string", () => { + expect(Utils.isGuid(123 as any)).toBe(false); + }); + + it("is false when not a guid", () => { + expect(Utils.isGuid("not a guid")).toBe(false); + }); + + it("is true when a guid", () => { + // we use a limited guid scope in which all zeroes is invalid + expect(Utils.isGuid("00000000-0000-1000-8000-000000000000")).toBe(true); + }); + }); + describe("getDomain", () => { it("should fail for invalid urls", () => { expect(Utils.getDomain(null)).toBeNull(); diff --git a/libs/common/src/platform/models/domain/state.ts b/libs/common/src/platform/models/domain/state.ts index 95557e082a..5dde49f99d 100644 --- a/libs/common/src/platform/models/domain/state.ts +++ b/libs/common/src/platform/models/domain/state.ts @@ -9,9 +9,6 @@ export class State< > { accounts: { [userId: string]: TAccount } = {}; globals: TGlobalState; - activeUserId: string; - authenticatedAccounts: string[] = []; - accountActivity: { [userId: string]: number } = {}; constructor(globals: TGlobalState) { this.globals = globals; 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 dd504dc302..7d266e93fc 100644 --- a/libs/common/src/platform/services/default-environment.service.spec.ts +++ b/libs/common/src/platform/services/default-environment.service.spec.ts @@ -31,10 +31,12 @@ describe("EnvironmentService", () => { [testUser]: { name: "name", email: "email", + emailVerified: false, }, [alternateTestUser]: { name: "name", email: "email", + emailVerified: false, }, }); stateProvider = new FakeStateProvider(accountService); @@ -47,6 +49,7 @@ describe("EnvironmentService", () => { id: userId, email: "test@example.com", name: `Test Name ${userId}`, + emailVerified: false, }); await awaitAsync(); }; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index cab5768d2a..9479d64710 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, firstValueFrom, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; @@ -33,10 +33,7 @@ const keys = { state: "state", stateVersion: "stateVersion", global: "global", - authenticatedAccounts: "authenticatedAccounts", - activeUserId: "activeUserId", tempAccountSettings: "tempAccountSettings", // used to hold account specific settings (i.e clear clipboard) between initial migration and first account authentication - accountActivity: "accountActivity", }; const partialKeys = { @@ -58,9 +55,6 @@ export class StateService< protected accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({}); accounts$ = this.accountsSubject.asObservable(); - protected activeAccountSubject = new BehaviorSubject(null); - activeAccount$ = this.activeAccountSubject.asObservable(); - private hasBeenInited = false; protected isRecoveredSession = false; @@ -112,36 +106,16 @@ export class StateService< } // Get all likely authenticated accounts - const authenticatedAccounts = ( - (await this.storageService.get(keys.authenticatedAccounts)) ?? [] - ).filter((account) => account != null); + const authenticatedAccounts = await firstValueFrom( + this.accountService.accounts$.pipe(map((accounts) => Object.keys(accounts))), + ); await this.updateState(async (state) => { for (const i in authenticatedAccounts) { state = await this.syncAccountFromDisk(authenticatedAccounts[i]); } - // After all individual accounts have been added - state.authenticatedAccounts = authenticatedAccounts; - - const storedActiveUser = await this.storageService.get(keys.activeUserId); - if (storedActiveUser != null) { - state.activeUserId = storedActiveUser; - } await this.pushAccounts(); - this.activeAccountSubject.next(state.activeUserId); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - // account service tracks logged out accounts, but State service does not, so we need to add the active account - // if it's not in the accounts list. - if (state.activeUserId != null && this.accountsSubject.value[state.activeUserId] == null) { - const activeDiskAccount = await this.getAccountFromDisk({ userId: state.activeUserId }); - await this.accountService.addAccount(state.activeUserId as UserId, { - name: activeDiskAccount.profile.name, - email: activeDiskAccount.profile.email, - }); - } - await this.accountService.switchAccount(state.activeUserId as UserId); - // End TODO return state; }); @@ -161,61 +135,25 @@ export class StateService< return state; }); - // 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. - await this.accountService.addAccount(userId as UserId, { - name: diskAccount.profile.name, - email: diskAccount.profile.email, - }); - return state; } async addAccount(account: TAccount) { await this.environmentService.seedUserEnvironment(account.profile.userId as UserId); await this.updateState(async (state) => { - state.authenticatedAccounts.push(account.profile.userId); - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); state.accounts[account.profile.userId] = account; return state; }); await this.scaffoldNewAccountStorage(account); - 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, { - name: account.profile.name, - email: account.profile.email, - }); - await this.setActiveUser(account.profile.userId); } - async setActiveUser(userId: string): Promise { - await this.clearDecryptedDataForActiveUser(); - await this.updateState(async (state) => { - state.activeUserId = userId; - await this.storageService.save(keys.activeUserId, userId); - this.activeAccountSubject.next(state.activeUserId); - // TODO: temporary update to avoid routing all account status changes through account service for now. - await this.accountService.switchAccount(userId as UserId); - - return state; - }); - - await this.pushAccounts(); - } - - async clean(options?: StorageOptions): Promise { + async clean(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultInMemoryOptions()); await this.deAuthenticateAccount(options.userId); - let currentUser = (await this.state())?.activeUserId; - if (options.userId === currentUser) { - currentUser = await this.dynamicallySetActiveUser(); - } await this.removeAccountFromDisk(options?.userId); await this.removeAccountFromMemory(options?.userId); await this.pushAccounts(); - return currentUser as UserId; } /** @@ -515,24 +453,6 @@ export class StateService< ); } - async getEmailVerified(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.profile.emailVerified ?? false - ); - } - - async setEmailVerified(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.profile.emailVerified = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getEnableBrowserIntegration(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -642,35 +562,6 @@ export class StateService< ); } - async getLastActive(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - - const accountActivity = await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - ); - - if (accountActivity == null || Object.keys(accountActivity).length < 1) { - return null; - } - - return accountActivity[options.userId]; - } - - async setLastActive(value: number, options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - if (options.userId == null) { - return; - } - const accountActivity = - (await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - )) ?? {}; - accountActivity[options.userId] = value; - await this.storageService.save(keys.accountActivity, accountActivity, options); - } - async getLastSync(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions())) @@ -910,24 +801,28 @@ export class StateService< } protected async getAccountFromMemory(options: StorageOptions): Promise { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + return await this.state().then(async (state) => { if (state.accounts == null) { return null; } - return state.accounts[await this.getUserIdFromMemory(options)]; - }); - } - - protected async getUserIdFromMemory(options: StorageOptions): Promise { - return await this.state().then((state) => { - return options?.userId != null - ? state.accounts[options.userId]?.profile?.userId - : state.activeUserId; + return state.accounts[userId]; }); } protected async getAccountFromDisk(options: StorageOptions): Promise { - if (options?.userId == null && (await this.state())?.activeUserId == null) { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + + if (userId == null) { return null; } @@ -1086,53 +981,76 @@ export class StateService< } protected async defaultInMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Memory, - userId: (await this.state()).activeUserId, + userId, }; } protected async defaultOnDiskOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Session, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskLocalOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Local, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Memory, - userId: (await this.state())?.activeUserId ?? (await this.getUserId()), + userId, useSecureStorage: false, }; } protected async defaultSecureStorageOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, useSecureStorage: true, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, }; } protected async getActiveUserIdFromStorage(): Promise { - return await this.storageService.get(keys.activeUserId); + return await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); } protected async removeAccountFromLocalStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskLocalOptions()), ); @@ -1143,7 +1061,10 @@ export class StateService< } protected async removeAccountFromSessionStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskOptions()), ); @@ -1154,7 +1075,10 @@ export class StateService< } protected async removeAccountFromSecureStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId }); await this.setCryptoMasterKeyAuto(null, { userId: userId }); @@ -1163,8 +1087,11 @@ export class StateService< } protected async removeAccountFromMemory(userId: string = null): Promise { + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.updateState(async (state) => { - userId = userId ?? state.activeUserId; delete state.accounts[userId]; return state; }); @@ -1178,15 +1105,16 @@ export class StateService< return Object.assign(this.createAccount(), persistentAccountInformation); } - protected async clearDecryptedDataForActiveUser(): Promise { + async clearDecryptedData(userId: UserId): Promise { await this.updateState(async (state) => { - const userId = state?.activeUserId; if (userId != null && state?.accounts[userId]?.data != null) { state.accounts[userId].data = new AccountData(); } return state; }); + + await this.pushAccounts(); } protected createAccount(init: Partial = null): TAccount { @@ -1201,14 +1129,6 @@ export class StateService< // We must have a manual call to clear tokens as we can't leverage state provider to clean // up our data as we have secure storage in the mix. await this.tokenService.clearTokens(userId as UserId); - await this.setLastActive(null, { userId: userId }); - await this.updateState(async (state) => { - state.authenticatedAccounts = state.authenticatedAccounts.filter((id) => id !== userId); - - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); - - return state; - }); } protected async removeAccountFromDisk(userId: string) { @@ -1217,32 +1137,6 @@ export class StateService< await this.removeAccountFromSecureStorage(userId); } - async nextUpActiveUser() { - const accounts = (await this.state())?.accounts; - if (accounts == null || Object.keys(accounts).length < 1) { - return null; - } - - let newActiveUser; - for (const userId in accounts) { - if (userId == null) { - continue; - } - if (await this.getIsAuthenticated({ userId: userId })) { - newActiveUser = userId; - break; - } - newActiveUser = null; - } - return newActiveUser as UserId; - } - - protected async dynamicallySetActiveUser() { - const newActiveUser = await this.nextUpActiveUser(); - await this.setActiveUser(newActiveUser); - return newActiveUser; - } - protected async saveSecureStorageKey( key: string, value: T, diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index c11463d210..d7cf79cc38 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -1,10 +1,12 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { firstValueFrom, map, timeout } from "rxjs"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AutofillSettingsServiceAbstraction } from "../../autofill/services/autofill-settings.service"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; +import { UserId } from "../../types/guid"; import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; @@ -27,6 +29,7 @@ export class SystemService implements SystemServiceAbstraction { private autofillSettingsService: AutofillSettingsServiceAbstraction, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private biometricStateService: BiometricStateService, + private accountService: AccountService, private taskSchedulerService?: TaskSchedulerService, ) { void this.taskSchedulerService?.registerTaskHandler( @@ -36,12 +39,14 @@ export class SystemService implements SystemServiceAbstraction { } async startProcessReload(authService: AuthService): Promise { - const accounts = await firstValueFrom(this.stateService.accounts$); + const accounts = await firstValueFrom(this.accountService.accounts$); if (accounts != null) { const keys = Object.keys(accounts); if (keys.length > 0) { for (const userId of keys) { - if ((await authService.getAuthStatus(userId)) === AuthenticationStatus.Unlocked) { + let status = await firstValueFrom(authService.authStatusFor$(userId as UserId)); + status = await authService.getAuthStatus(userId); + if (status === AuthenticationStatus.Unlocked) { return; } } @@ -71,15 +76,24 @@ export class SystemService implements SystemServiceAbstraction { clearInterval(this.reloadInterval); this.reloadInterval = null; - const currentUser = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); + const currentUser = await firstValueFrom( + this.accountService.activeAccount$.pipe( + map((a) => a?.id), + timeout(500), + ), + ); // Replace current active user if they will be logged out on reload if (currentUser != null) { const timeoutAction = await firstValueFrom( this.vaultTimeoutSettingsService.vaultTimeoutAction$().pipe(timeout(500)), ); if (timeoutAction === VaultTimeoutAction.LogOut) { - const nextUser = await this.stateService.nextUpActiveUser(); - await this.stateService.setActiveUser(nextUser); + const nextUser = await firstValueFrom( + this.accountService.nextUpAccount$.pipe(map((account) => account?.id ?? null)), + ); + // Can be removed once we migrate password generation history to state providers + await this.stateService.clearDecryptedData(currentUser); + await this.accountService.switchAccount(nextUser); } } diff --git a/libs/common/src/platform/services/user-auto-unlock-key.service.spec.ts b/libs/common/src/platform/services/user-auto-unlock-key.service.spec.ts new file mode 100644 index 0000000000..f0d60158c1 --- /dev/null +++ b/libs/common/src/platform/services/user-auto-unlock-key.service.spec.ts @@ -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(); + + 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); + }); + }); +}); diff --git a/libs/common/src/platform/services/user-auto-unlock-key.service.ts b/libs/common/src/platform/services/user-auto-unlock-key.service.ts new file mode 100644 index 0000000000..6c8ce3f048 --- /dev/null +++ b/libs/common/src/platform/services/user-auto-unlock-key.service.ts @@ -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 { + if (userId == null) { + return; + } + + const autoUserKey = await this.cryptoService.getUserKeyFromStorage( + KeySuffixOptions.Auto, + userId, + ); + + if (autoUserKey == null) { + return; + } + + await this.cryptoService.setUserKey(autoUserKey, userId); + } +} diff --git a/libs/common/src/platform/services/user-key-init.service.spec.ts b/libs/common/src/platform/services/user-key-init.service.spec.ts deleted file mode 100644 index 567320ded6..0000000000 --- a/libs/common/src/platform/services/user-key-init.service.spec.ts +++ /dev/null @@ -1,162 +0,0 @@ -import { mock } from "jest-mock-extended"; - -import { FakeAccountService, mockAccountServiceWith } from "../../../spec"; -import { CsprngArray } from "../../types/csprng"; -import { UserId } from "../../types/guid"; -import { UserKey } from "../../types/key"; -import { LogService } from "../abstractions/log.service"; -import { KeySuffixOptions } from "../enums"; -import { Utils } from "../misc/utils"; -import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; - -import { CryptoService } from "./crypto.service"; -import { UserKeyInitService } from "./user-key-init.service"; - -describe("UserKeyInitService", () => { - let userKeyInitService: UserKeyInitService; - - const mockUserId = Utils.newGuid() as UserId; - - const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); - - const cryptoService = mock(); - const logService = mock(); - - beforeEach(() => { - userKeyInitService = new UserKeyInitService(accountService, cryptoService, logService); - }); - - describe("listenForActiveUserChangesToSetUserKey()", () => { - it("calls setUserKeyInMemoryIfAutoUserKeySet if there is an active user", () => { - // Arrange - accountService.activeAccountSubject.next({ - id: mockUserId, - name: "name", - email: "email", - }); - - (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet = jest.fn(); - - // Act - - const subscription = userKeyInitService.listenForActiveUserChangesToSetUserKey(); - - // Assert - - expect(subscription).not.toBeFalsy(); - - expect((userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet).toHaveBeenCalledWith( - mockUserId, - ); - }); - - it("calls setUserKeyInMemoryIfAutoUserKeySet if there is an active user and tracks subsequent emissions", () => { - // Arrange - accountService.activeAccountSubject.next({ - id: mockUserId, - name: "name", - email: "email", - }); - - const mockUser2Id = Utils.newGuid() as UserId; - - jest - .spyOn(userKeyInitService as any, "setUserKeyInMemoryIfAutoUserKeySet") - .mockImplementation(() => Promise.resolve()); - - // Act - - const subscription = userKeyInitService.listenForActiveUserChangesToSetUserKey(); - - accountService.activeAccountSubject.next({ - id: mockUser2Id, - name: "name", - email: "email", - }); - - // Assert - - expect(subscription).not.toBeFalsy(); - - expect((userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet).toHaveBeenCalledTimes( - 2, - ); - - expect( - (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet, - ).toHaveBeenNthCalledWith(1, mockUserId); - expect( - (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet, - ).toHaveBeenNthCalledWith(2, mockUser2Id); - - subscription.unsubscribe(); - }); - - it("does not call setUserKeyInMemoryIfAutoUserKeySet if there is not an active user", () => { - // Arrange - accountService.activeAccountSubject.next(null); - - (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet = jest.fn(); - - // Act - - const subscription = userKeyInitService.listenForActiveUserChangesToSetUserKey(); - - // Assert - - expect(subscription).not.toBeFalsy(); - - expect( - (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet, - ).not.toHaveBeenCalledWith(mockUserId); - }); - }); - - describe("setUserKeyInMemoryIfAutoUserKeySet", () => { - it("does nothing if the userId is null", async () => { - // Act - await (userKeyInitService 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 (userKeyInitService 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 (userKeyInitService as any).setUserKeyInMemoryIfAutoUserKeySet(userId); - - // Assert - expect(cryptoService.getUserKeyFromStorage).toHaveBeenCalledWith( - KeySuffixOptions.Auto, - userId, - ); - expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockAutoUserKey, userId); - }); - }); -}); diff --git a/libs/common/src/platform/services/user-key-init.service.ts b/libs/common/src/platform/services/user-key-init.service.ts deleted file mode 100644 index 1f6aacce8f..0000000000 --- a/libs/common/src/platform/services/user-key-init.service.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { EMPTY, Subscription, catchError, filter, from, switchMap } from "rxjs"; - -import { AccountService } from "../../auth/abstractions/account.service"; -import { UserId } from "../../types/guid"; -import { CryptoService } from "../abstractions/crypto.service"; -import { LogService } from "../abstractions/log.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 UserKeyInitService { - constructor( - private accountService: AccountService, - private cryptoService: CryptoService, - private logService: LogService, - ) {} - - // Note: must listen for changes to support account switching - listenForActiveUserChangesToSetUserKey(): Subscription { - return this.accountService.activeAccount$ - .pipe( - filter((activeAccount) => activeAccount != null), - switchMap((activeAccount) => - from(this.setUserKeyInMemoryIfAutoUserKeySet(activeAccount?.id)).pipe( - catchError((err: unknown) => { - this.logService.warning( - `setUserKeyInMemoryIfAutoUserKeySet failed with error: ${err}`, - ); - // Returning EMPTY to protect observable chain from cancellation in case of error - return EMPTY; - }), - ), - ), - ) - .subscribe(); - } - - private async setUserKeyInMemoryIfAutoUserKeySet(userId: UserId) { - if (userId == null) { - return; - } - - const autoUserKey = await this.cryptoService.getUserKeyFromStorage( - KeySuffixOptions.Auto, - userId, - ); - if (autoUserKey == null) { - return; - } - - await this.cryptoService.setUserKey(autoUserKey, userId); - } -} diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts index c1cc15a176..681963f823 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts @@ -1,7 +1,6 @@ import { mock } from "jest-mock-extended"; import { mockAccountServiceWith, trackEmissions } from "../../../../spec"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; import { SingleUserStateProvider } from "../user-state.provider"; @@ -14,7 +13,7 @@ describe("DefaultActiveUserStateProvider", () => { id: userId, name: "name", email: "email", - status: AuthenticationStatus.Locked, + emailVerified: false, }; const accountService = mockAccountServiceWith(userId, accountInfo); let sut: DefaultActiveUserStateProvider; 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 51a972a9dc..c652136a0d 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 @@ -82,6 +82,7 @@ describe("DefaultActiveUserState", () => { activeAccountSubject.next({ id: userId, email: `test${id}@example.com`, + emailVerified: false, name: `Test User ${id}`, }); await awaitAsync(); diff --git a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts index 3243b53d67..98d423cf48 100644 --- a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts @@ -69,7 +69,12 @@ describe("DefaultStateProvider", () => { userId?: UserId, ) => Observable, ) => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); @@ -114,7 +119,12 @@ describe("DefaultStateProvider", () => { ); describe("getUserState$", () => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index 4b8d435724..b85eccecc8 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -38,6 +38,7 @@ export const BILLING_DISK = new StateDefinition("billing", "disk"); export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); +export const ACCOUNT_DISK = new StateDefinition("account", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const TWO_FACTOR_MEMORY = new StateDefinition("twoFactor", "memory"); 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 5344093a25..12c24dcdef 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 @@ -1,9 +1,10 @@ import { MockProxy, any, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountInfo } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { FakeMasterPasswordService } from "../../auth/services/master-password/fake-master-password.service"; @@ -13,7 +14,6 @@ import { MessagingService } from "../../platform/abstractions/messaging.service" import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { StateService } from "../../platform/abstractions/state.service"; import { Utils } from "../../platform/misc/utils"; -import { Account } from "../../platform/models/domain/account"; import { StateEventRunnerService } from "../../platform/state"; import { UserId } from "../../types/guid"; import { CipherService } from "../../vault/abstractions/cipher.service"; @@ -39,7 +39,6 @@ describe("VaultTimeoutService", () => { let lockedCallback: jest.Mock, [userId: string]>; let loggedOutCallback: jest.Mock, [expired: boolean, userId?: string]>; - let accountsSubject: BehaviorSubject>; let vaultTimeoutActionSubject: BehaviorSubject; let availableVaultTimeoutActionsSubject: BehaviorSubject; @@ -65,10 +64,6 @@ describe("VaultTimeoutService", () => { lockedCallback = jest.fn(); loggedOutCallback = jest.fn(); - accountsSubject = new BehaviorSubject(null); - - stateService.accounts$ = accountsSubject; - vaultTimeoutActionSubject = new BehaviorSubject(VaultTimeoutAction.Lock); vaultTimeoutSettingsService.vaultTimeoutAction$.mockReturnValue(vaultTimeoutActionSubject); @@ -127,21 +122,39 @@ describe("VaultTimeoutService", () => { return Promise.resolve(accounts[userId]?.vaultTimeout); }); - stateService.getLastActive.mockImplementation((options) => { - return Promise.resolve(accounts[options.userId]?.lastActive); - }); - stateService.getUserId.mockResolvedValue(globalSetups?.userId); - stateService.activeAccount$ = new BehaviorSubject(globalSetups?.userId); - + // Set desired user active and known users on accounts service : note the only thing that matters here is that the ID are set if (globalSetups?.userId) { accountService.activeAccountSubject.next({ id: globalSetups.userId as UserId, email: null, + emailVerified: false, name: null, }); } + accountService.accounts$ = of( + Object.entries(accounts).reduce( + (agg, [id]) => { + agg[id] = { + email: "", + emailVerified: true, + name: "", + }; + return agg; + }, + {} as Record, + ), + ); + accountService.accountActivity$ = of( + Object.entries(accounts).reduce( + (agg, [id, info]) => { + agg[id] = info.lastActive ? new Date(info.lastActive) : null; + return agg; + }, + {} as Record, + ), + ); platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false); @@ -158,16 +171,6 @@ describe("VaultTimeoutService", () => { ], ); }); - - const accountsSubjectValue: Record = Object.keys(accounts).reduce( - (agg, key) => { - const newPartial: Record = {}; - newPartial[key] = null; // No values actually matter on this other than the key - return Object.assign(agg, newPartial); - }, - {} as Record, - ); - accountsSubject.next(accountsSubjectValue); }; const expectUserToHaveLocked = (userId: string) => { diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.ts index 8baf6c04c4..8e0978d07d 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { combineLatest, firstValueFrom, switchMap } from "rxjs"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; @@ -64,14 +64,25 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { // Get whether or not the view is open a single time so it can be compared for each user const isViewOpen = await this.platformUtilsService.isViewOpen(); - const activeUserId = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); - - const accounts = await firstValueFrom(this.stateService.accounts$); - for (const userId in accounts) { - if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) { - await this.executeTimeoutAction(userId); - } - } + await firstValueFrom( + combineLatest([ + this.accountService.activeAccount$, + this.accountService.accountActivity$, + ]).pipe( + switchMap(async ([activeAccount, accountActivity]) => { + const activeUserId = activeAccount?.id; + for (const userIdString in accountActivity) { + const userId = userIdString as UserId; + if ( + userId != null && + (await this.shouldLock(userId, accountActivity[userId], activeUserId, isViewOpen)) + ) { + await this.executeTimeoutAction(userId); + } + } + }), + ), + ); } async lock(userId?: string): Promise { @@ -123,6 +134,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { private async shouldLock( userId: string, + lastActive: Date, activeUserId: string, isViewOpen: boolean, ): Promise { @@ -146,13 +158,12 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { return false; } - const lastActive = await this.stateService.getLastActive({ userId: userId }); if (lastActive == null) { return false; } const vaultTimeoutSeconds = vaultTimeout * 60; - const diffSeconds = (new Date().getTime() - lastActive) / 1000; + const diffSeconds = (new Date().getTime() - lastActive.getTime()) / 1000; return diffSeconds >= vaultTimeoutSeconds; } diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 31bc5460b4..0a1f4b1d11 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -57,13 +57,14 @@ import { CipherServiceMigrator } from "./migrations/57-move-cipher-service-to-st import { RemoveRefreshTokenMigratedFlagMigrator } from "./migrations/58-remove-refresh-token-migrated-state-provider-flag"; import { KdfConfigMigrator } from "./migrations/59-move-kdf-config-to-state-provider"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; +import { KnownAccountsMigrator } from "./migrations/60-known-accounts"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-settings-to-global"; import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 59; +export const CURRENT_VERSION = 60; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -124,7 +125,8 @@ export function createMigrationBuilder() { .with(AuthRequestMigrator, 55, 56) .with(CipherServiceMigrator, 56, 57) .with(RemoveRefreshTokenMigratedFlagMigrator, 57, 58) - .with(KdfConfigMigrator, 58, CURRENT_VERSION); + .with(KdfConfigMigrator, 58, 59) + .with(KnownAccountsMigrator, 59, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index 5f366f2597..162fac2fab 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -27,6 +27,14 @@ const exampleJSON = { }, global_serviceName_key: "global_serviceName_key", user_userId_serviceName_key: "user_userId_serviceName_key", + global_account_accounts: { + "c493ed01-4e08-4e88-abc7-332f380ca760": { + otherStuff: "otherStuff3", + }, + "23e61a5f-2ece-4f5e-b499-f0bc489482a9": { + otherStuff: "otherStuff4", + }, + }, }; describe("RemoveLegacyEtmKeyMigrator", () => { @@ -81,6 +89,41 @@ describe("RemoveLegacyEtmKeyMigrator", () => { const accounts = await sut.getAccounts(); expect(accounts).toEqual([]); }); + + it("handles global scoped known accounts for version 60 and after", async () => { + sut.currentVersion = 60; + const accounts = await sut.getAccounts(); + expect(accounts).toEqual([ + // Note, still gets values stored in state service objects, just grabs user ids from global + { + userId: "c493ed01-4e08-4e88-abc7-332f380ca760", + account: { otherStuff: "otherStuff1" }, + }, + { + userId: "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + account: { otherStuff: "otherStuff2" }, + }, + ]); + }); + }); + + describe("getKnownUserIds", () => { + it("returns all user ids", async () => { + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); + + it("returns all user ids when version is 60 or greater", async () => { + sut.currentVersion = 60; + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); }); describe("getFromGlobal", () => { diff --git a/libs/common/src/state-migrations/migration-helper.ts b/libs/common/src/state-migrations/migration-helper.ts index 2505e2b264..5d1de8dd49 100644 --- a/libs/common/src/state-migrations/migration-helper.ts +++ b/libs/common/src/state-migrations/migration-helper.ts @@ -162,7 +162,7 @@ export class MigrationHelper { async getAccounts(): Promise< { userId: string; account: ExpectedAccountType }[] > { - const userIds = (await this.get("authenticatedAccounts")) ?? []; + const userIds = await this.getKnownUserIds(); return Promise.all( userIds.map(async (userId) => ({ userId, @@ -171,6 +171,17 @@ export class MigrationHelper { ); } + /** + * Helper method to read known users ids. + */ + async getKnownUserIds(): Promise { + if (this.currentVersion < 61) { + return knownAccountUserIdsBuilderPre61(this.storageService); + } else { + return knownAccountUserIdsBuilder(this.storageService); + } + } + /** * Builds a user storage key appropriate for the current version. * @@ -233,3 +244,18 @@ function globalKeyBuilder(keyDefinition: KeyDefinitionLike): string { function globalKeyBuilderPre9(): string { throw Error("No key builder should be used for versions prior to 9."); } + +async function knownAccountUserIdsBuilderPre61( + storageService: AbstractStorageService, +): Promise { + return (await storageService.get("authenticatedAccounts")) ?? []; +} + +async function knownAccountUserIdsBuilder( + storageService: AbstractStorageService, +): Promise { + const accounts = await storageService.get>( + globalKeyBuilder({ stateDefinition: { name: "account" }, key: "accounts" }), + ); + return Object.keys(accounts ?? {}); +} diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts new file mode 100644 index 0000000000..28dedb3c39 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts @@ -0,0 +1,145 @@ +import { MockProxy } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { + ACCOUNT_ACCOUNTS, + ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, + KnownAccountsMigrator, +} from "./60-known-accounts"; + +const migrateJson = () => { + return { + authenticatedAccounts: ["user1", "user2"], + activeUserId: "user1", + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + accountActivity: { + user1: 1609459200000, // 2021-01-01 + user2: 1609545600000, // 2021-01-02 + }, + }; +}; + +const rollbackJson = () => { + return { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + global_account_accounts: { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + }, + global_account_activeAccountId: "user1", + global_account_activity: { + user1: "2021-01-01T00:00:00.000Z", + user2: "2021-01-02T00:00:00.000Z", + }, + }; +}; + +describe("ReplicateKnownAccounts", () => { + let helper: MockProxy; + let sut: KnownAccountsMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(migrateJson(), 59); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("migrates accounts", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS, { + user1: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + user2: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + expect(helper.remove).toHaveBeenCalledWith("authenticatedAccounts"); + }); + + it("migrates active account it", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID, "user1"); + expect(helper.remove).toHaveBeenCalledWith("activeUserId"); + }); + + it("migrates account activity", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY, { + user1: '"2021-01-01T00:00:00.000Z"', + user2: '"2021-01-02T00:00:00.000Z"', + }); + expect(helper.remove).toHaveBeenCalledWith("accountActivity"); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJson(), 60); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("rolls back authenticated accounts", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("authenticatedAccounts", ["user1", "user2"]); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS); + }); + + it("rolls back active account id", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("activeUserId", "user1"); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID); + }); + + it("rolls back account activity", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("accountActivity", { + user1: 1609459200000, + user2: 1609545600000, + }); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.ts new file mode 100644 index 0000000000..75117da5b4 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.ts @@ -0,0 +1,111 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +export const ACCOUNT_ACCOUNTS: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "accounts", +}; + +export const ACCOUNT_ACTIVE_ACCOUNT_ID: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activeAccountId", +}; + +export const ACCOUNT_ACTIVITY: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activity", +}; + +type ExpectedAccountType = { + profile?: { + email?: string; + name?: string; + emailVerified?: boolean; + }; +}; + +export class KnownAccountsMigrator extends Migrator<59, 60> { + async migrate(helper: MigrationHelper): Promise { + await this.migrateAuthenticatedAccounts(helper); + await this.migrateActiveAccountId(helper); + await this.migrateAccountActivity(helper); + } + async rollback(helper: MigrationHelper): Promise { + // authenticated account are removed, but the accounts record also contains logged out accounts. Best we can do is to add them all back + const accounts = (await helper.getFromGlobal>(ACCOUNT_ACCOUNTS)) ?? {}; + await helper.set("authenticatedAccounts", Object.keys(accounts)); + await helper.removeFromGlobal(ACCOUNT_ACCOUNTS); + + // Active Account Id + const activeAccountId = await helper.getFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + if (activeAccountId) { + await helper.set("activeUserId", activeAccountId); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + + // Account Activity + const accountActivity = await helper.getFromGlobal>(ACCOUNT_ACTIVITY); + if (accountActivity) { + const toStore = Object.entries(accountActivity).reduce( + (agg, [userId, dateString]) => { + agg[userId] = new Date(dateString).getTime(); + return agg; + }, + {} as Record, + ); + await helper.set("accountActivity", toStore); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVITY); + } + + private async migrateAuthenticatedAccounts(helper: MigrationHelper) { + const authenticatedAccounts = (await helper.get("authenticatedAccounts")) ?? []; + const accounts = await Promise.all( + authenticatedAccounts.map(async (userId) => { + const account = await helper.get(userId); + return { userId, account }; + }), + ); + const accountsToStore = accounts.reduce( + (agg, { userId, account }) => { + if (account?.profile) { + agg[userId] = { + email: account.profile.email ?? "", + emailVerified: account.profile.emailVerified ?? false, + name: account.profile.name, + }; + } + return agg; + }, + {} as Record, + ); + + await helper.setToGlobal(ACCOUNT_ACCOUNTS, accountsToStore); + await helper.remove("authenticatedAccounts"); + } + + private async migrateAccountActivity(helper: MigrationHelper) { + const stored = await helper.get>("accountActivity"); + const accountActivity = Object.entries(stored ?? {}).reduce( + (agg, [userId, dateMs]) => { + agg[userId] = JSON.stringify(new Date(dateMs)); + return agg; + }, + {} as Record, + ); + await helper.setToGlobal(ACCOUNT_ACTIVITY, accountActivity); + await helper.remove("accountActivity"); + } + + private async migrateActiveAccountId(helper: MigrationHelper) { + const activeAccountId = await helper.get("activeUserId"); + await helper.setToGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID, activeAccountId); + await helper.remove("activeUserId"); + } +} 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 41183c42af..2f0f50c616 100644 --- a/libs/common/src/tools/send/services/send.service.spec.ts +++ b/libs/common/src/tools/send/services/send.service.spec.ts @@ -62,6 +62,7 @@ describe("SendService", () => { accountService.activeAccountSubject.next({ id: mockUserId, email: "email", + emailVerified: false, name: "name", }); diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 73869ff488..995ab7319b 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -326,7 +326,10 @@ export class SyncService implements SyncServiceAbstraction { await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations); await this.avatarService.setSyncAvatarColor(response.id as UserId, response.avatarColor); await this.tokenService.setSecurityStamp(response.securityStamp, response.id as UserId); - await this.stateService.setEmailVerified(response.emailVerified); + await this.accountService.setAccountEmailVerified( + response.id as UserId, + response.emailVerified, + ); await this.billingAccountProfileStateService.setHasPremium( response.premiumPersonally, diff --git a/libs/components/src/a11y/a11y-cell.directive.ts b/libs/components/src/a11y/a11y-cell.directive.ts new file mode 100644 index 0000000000..fdd75c076f --- /dev/null +++ b/libs/components/src/a11y/a11y-cell.directive.ts @@ -0,0 +1,33 @@ +import { ContentChild, Directive, ElementRef, HostBinding } from "@angular/core"; + +import { FocusableElement } from "../shared/focusable-element"; + +@Directive({ + selector: "bitA11yCell", + standalone: true, + providers: [{ provide: FocusableElement, useExisting: A11yCellDirective }], +}) +export class A11yCellDirective implements FocusableElement { + @HostBinding("attr.role") + role: "gridcell" | null; + + @ContentChild(FocusableElement) + private focusableChild: FocusableElement; + + getFocusTarget() { + let focusTarget: HTMLElement; + if (this.focusableChild) { + focusTarget = this.focusableChild.getFocusTarget(); + } else { + focusTarget = this.elementRef.nativeElement.querySelector("button, a"); + } + + if (!focusTarget) { + return this.elementRef.nativeElement; + } + + return focusTarget; + } + + constructor(private elementRef: ElementRef) {} +} diff --git a/libs/components/src/a11y/a11y-grid.directive.ts b/libs/components/src/a11y/a11y-grid.directive.ts new file mode 100644 index 0000000000..c632376f4f --- /dev/null +++ b/libs/components/src/a11y/a11y-grid.directive.ts @@ -0,0 +1,145 @@ +import { + AfterViewInit, + ContentChildren, + Directive, + HostBinding, + HostListener, + Input, + QueryList, +} from "@angular/core"; + +import type { A11yCellDirective } from "./a11y-cell.directive"; +import { A11yRowDirective } from "./a11y-row.directive"; + +@Directive({ + selector: "bitA11yGrid", + standalone: true, +}) +export class A11yGridDirective implements AfterViewInit { + @HostBinding("attr.role") + role = "grid"; + + @ContentChildren(A11yRowDirective) + rows: QueryList; + + /** The number of pages to navigate on `PageUp` and `PageDown` */ + @Input() pageSize = 5; + + private grid: A11yCellDirective[][]; + + /** The row that currently has focus */ + private activeRow = 0; + + /** The cell that currently has focus */ + private activeCol = 0; + + @HostListener("keydown", ["$event"]) + onKeyDown(event: KeyboardEvent) { + switch (event.code) { + case "ArrowUp": + this.updateCellFocusByDelta(-1, 0); + break; + case "ArrowRight": + this.updateCellFocusByDelta(0, 1); + break; + case "ArrowDown": + this.updateCellFocusByDelta(1, 0); + break; + case "ArrowLeft": + this.updateCellFocusByDelta(0, -1); + break; + case "Home": + this.updateCellFocusByDelta(-this.activeRow, -this.activeCol); + break; + case "End": + this.updateCellFocusByDelta(this.grid.length, this.grid[this.grid.length - 1].length); + break; + case "PageUp": + this.updateCellFocusByDelta(-this.pageSize, 0); + break; + case "PageDown": + this.updateCellFocusByDelta(this.pageSize, 0); + break; + default: + return; + } + + /** Prevent default scrolling behavior */ + event.preventDefault(); + } + + ngAfterViewInit(): void { + this.initializeGrid(); + } + + private initializeGrid(): void { + try { + this.grid = this.rows.map((listItem) => { + listItem.role = "row"; + return [...listItem.cells]; + }); + this.grid.flat().forEach((cell) => { + cell.role = "gridcell"; + cell.getFocusTarget().tabIndex = -1; + }); + + this.getActiveCellContent().tabIndex = 0; + } catch (error) { + // eslint-disable-next-line no-console + console.error("Unable to initialize grid"); + } + } + + /** Get the focusable content of the active cell */ + private getActiveCellContent(): HTMLElement { + return this.grid[this.activeRow][this.activeCol].getFocusTarget(); + } + + /** Move focus via a delta against the currently active gridcell */ + private updateCellFocusByDelta(rowDelta: number, colDelta: number) { + const prevActive = this.getActiveCellContent(); + + this.activeCol += colDelta; + this.activeRow += rowDelta; + + // Row upper bound + if (this.activeRow >= this.grid.length) { + this.activeRow = this.grid.length - 1; + } + + // Row lower bound + if (this.activeRow < 0) { + this.activeRow = 0; + } + + // Column upper bound + if (this.activeCol >= this.grid[this.activeRow].length) { + if (this.activeRow < this.grid.length - 1) { + // Wrap to next row on right arrow + this.activeCol = 0; + this.activeRow += 1; + } else { + this.activeCol = this.grid[this.activeRow].length - 1; + } + } + + // Column lower bound + if (this.activeCol < 0) { + if (this.activeRow > 0) { + // Wrap to prev row on left arrow + this.activeRow -= 1; + this.activeCol = this.grid[this.activeRow].length - 1; + } else { + this.activeCol = 0; + } + } + + const nextActive = this.getActiveCellContent(); + nextActive.tabIndex = 0; + nextActive.focus(); + + if (nextActive !== prevActive) { + prevActive.tabIndex = -1; + } + } +} diff --git a/libs/components/src/a11y/a11y-row.directive.ts b/libs/components/src/a11y/a11y-row.directive.ts new file mode 100644 index 0000000000..e062eb2b5a --- /dev/null +++ b/libs/components/src/a11y/a11y-row.directive.ts @@ -0,0 +1,31 @@ +import { + AfterViewInit, + ContentChildren, + Directive, + HostBinding, + QueryList, + ViewChildren, +} from "@angular/core"; + +import { A11yCellDirective } from "./a11y-cell.directive"; + +@Directive({ + selector: "bitA11yRow", + standalone: true, +}) +export class A11yRowDirective implements AfterViewInit { + @HostBinding("attr.role") + role: "row" | null; + + cells: A11yCellDirective[]; + + @ViewChildren(A11yCellDirective) + private viewCells: QueryList; + + @ContentChildren(A11yCellDirective) + private contentCells: QueryList; + + ngAfterViewInit(): void { + this.cells = [...this.viewCells, ...this.contentCells]; + } +} diff --git a/libs/components/src/badge/badge.directive.ts b/libs/components/src/badge/badge.directive.ts index b81b9f80e2..acce4a18aa 100644 --- a/libs/components/src/badge/badge.directive.ts +++ b/libs/components/src/badge/badge.directive.ts @@ -1,5 +1,7 @@ import { Directive, ElementRef, HostBinding, Input } from "@angular/core"; +import { FocusableElement } from "../shared/focusable-element"; + export type BadgeVariant = "primary" | "secondary" | "success" | "danger" | "warning" | "info"; const styles: Record = { @@ -22,8 +24,9 @@ const hoverStyles: Record = { @Directive({ selector: "span[bitBadge], a[bitBadge], button[bitBadge]", + providers: [{ provide: FocusableElement, useExisting: BadgeDirective }], }) -export class BadgeDirective { +export class BadgeDirective implements FocusableElement { @HostBinding("class") get classList() { return [ "tw-inline-block", @@ -62,6 +65,10 @@ export class BadgeDirective { */ @Input() truncate = true; + getFocusTarget() { + return this.el.nativeElement; + } + private hasHoverEffects = false; constructor(private el: ElementRef) { diff --git a/libs/components/src/icon-button/icon-button.component.ts b/libs/components/src/icon-button/icon-button.component.ts index 53e8032795..54f6dfda96 100644 --- a/libs/components/src/icon-button/icon-button.component.ts +++ b/libs/components/src/icon-button/icon-button.component.ts @@ -1,6 +1,7 @@ -import { Component, HostBinding, Input } from "@angular/core"; +import { Component, ElementRef, HostBinding, Input } from "@angular/core"; import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction"; +import { FocusableElement } from "../shared/focusable-element"; export type IconButtonType = ButtonType | "contrast" | "main" | "muted" | "light"; @@ -123,9 +124,12 @@ const sizes: Record = { @Component({ selector: "button[bitIconButton]:not(button[bitButton])", templateUrl: "icon-button.component.html", - providers: [{ provide: ButtonLikeAbstraction, useExisting: BitIconButtonComponent }], + providers: [ + { provide: ButtonLikeAbstraction, useExisting: BitIconButtonComponent }, + { provide: FocusableElement, useExisting: BitIconButtonComponent }, + ], }) -export class BitIconButtonComponent implements ButtonLikeAbstraction { +export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { @Input("bitIconButton") icon: string; @Input() buttonType: IconButtonType; @@ -162,4 +166,10 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction { setButtonType(value: "primary" | "secondary" | "danger" | "unstyled") { this.buttonType = value; } + + getFocusTarget() { + return this.elementRef.nativeElement; + } + + constructor(private elementRef: ElementRef) {} } diff --git a/libs/components/src/index.ts b/libs/components/src/index.ts index 36185911a6..1e4a3a86ff 100644 --- a/libs/components/src/index.ts +++ b/libs/components/src/index.ts @@ -16,6 +16,7 @@ export * from "./form-field"; export * from "./icon-button"; export * from "./icon"; export * from "./input"; +export * from "./item"; export * from "./layout"; export * from "./link"; export * from "./menu"; diff --git a/libs/components/src/input/autofocus.directive.ts b/libs/components/src/input/autofocus.directive.ts index f8161ee6e0..625e7fbc92 100644 --- a/libs/components/src/input/autofocus.directive.ts +++ b/libs/components/src/input/autofocus.directive.ts @@ -3,12 +3,7 @@ import { take } from "rxjs/operators"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -/** - * Interface for implementing focusable components. Used by the AutofocusDirective. - */ -export abstract class FocusableElement { - focus: () => void; -} +import { FocusableElement } from "../shared/focusable-element"; /** * Directive to focus an element. @@ -46,7 +41,7 @@ export class AutofocusDirective { private focus() { if (this.focusableElement) { - this.focusableElement.focus(); + this.focusableElement.getFocusTarget().focus(); } else { this.el.nativeElement.focus(); } diff --git a/libs/components/src/item/index.ts b/libs/components/src/item/index.ts new file mode 100644 index 0000000000..56896cdc3c --- /dev/null +++ b/libs/components/src/item/index.ts @@ -0,0 +1 @@ +export * from "./item.module"; diff --git a/libs/components/src/item/item-action.component.ts b/libs/components/src/item/item-action.component.ts new file mode 100644 index 0000000000..8cabf5c5c2 --- /dev/null +++ b/libs/components/src/item/item-action.component.ts @@ -0,0 +1,12 @@ +import { Component } from "@angular/core"; + +import { A11yCellDirective } from "../a11y/a11y-cell.directive"; + +@Component({ + selector: "bit-item-action", + standalone: true, + imports: [], + template: ``, + providers: [{ provide: A11yCellDirective, useExisting: ItemActionComponent }], +}) +export class ItemActionComponent extends A11yCellDirective {} diff --git a/libs/components/src/item/item-content.component.html b/libs/components/src/item/item-content.component.html new file mode 100644 index 0000000000..d034a4a001 --- /dev/null +++ b/libs/components/src/item/item-content.component.html @@ -0,0 +1,16 @@ +
+ + +
+
+ +
+
+ +
+
+
+ +
+ +
diff --git a/libs/components/src/item/item-content.component.ts b/libs/components/src/item/item-content.component.ts new file mode 100644 index 0000000000..58a1198512 --- /dev/null +++ b/libs/components/src/item/item-content.component.ts @@ -0,0 +1,15 @@ +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component } from "@angular/core"; + +@Component({ + selector: "bit-item-content, [bit-item-content]", + standalone: true, + imports: [CommonModule], + templateUrl: `item-content.component.html`, + host: { + class: + "fvw-target tw-outline-none tw-text-main hover:tw-text-main hover:tw-no-underline tw-text-base tw-py-2 tw-px-4 tw-bg-transparent tw-w-full tw-border-none tw-flex tw-gap-4 tw-items-center tw-justify-between", + }, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class ItemContentComponent {} diff --git a/libs/components/src/item/item-group.component.ts b/libs/components/src/item/item-group.component.ts new file mode 100644 index 0000000000..2a9a8275cc --- /dev/null +++ b/libs/components/src/item/item-group.component.ts @@ -0,0 +1,13 @@ +import { ChangeDetectionStrategy, Component } from "@angular/core"; + +@Component({ + selector: "bit-item-group", + standalone: true, + imports: [], + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, + host: { + class: "tw-block", + }, +}) +export class ItemGroupComponent {} diff --git a/libs/components/src/item/item.component.html b/libs/components/src/item/item.component.html new file mode 100644 index 0000000000..0c91c6848e --- /dev/null +++ b/libs/components/src/item/item.component.html @@ -0,0 +1,21 @@ + +
+ + + + +
+ +
+
diff --git a/libs/components/src/item/item.component.ts b/libs/components/src/item/item.component.ts new file mode 100644 index 0000000000..4b7b57fa9f --- /dev/null +++ b/libs/components/src/item/item.component.ts @@ -0,0 +1,29 @@ +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component, HostListener, signal } from "@angular/core"; + +import { A11yRowDirective } from "../a11y/a11y-row.directive"; + +import { ItemActionComponent } from "./item-action.component"; + +@Component({ + selector: "bit-item", + standalone: true, + imports: [CommonModule, ItemActionComponent], + changeDetection: ChangeDetectionStrategy.OnPush, + templateUrl: "item.component.html", + providers: [{ provide: A11yRowDirective, useExisting: ItemComponent }], +}) +export class ItemComponent extends A11yRowDirective { + /** + * We have `:focus-within` and `:focus-visible` but no `:focus-visible-within` + */ + protected focusVisibleWithin = signal(false); + @HostListener("focusin", ["$event.target"]) + onFocusIn(target: HTMLElement) { + this.focusVisibleWithin.set(target.matches(".fvw-target:focus-visible")); + } + @HostListener("focusout") + onFocusOut() { + this.focusVisibleWithin.set(false); + } +} diff --git a/libs/components/src/item/item.mdx b/libs/components/src/item/item.mdx new file mode 100644 index 0000000000..8506de72bb --- /dev/null +++ b/libs/components/src/item/item.mdx @@ -0,0 +1,141 @@ +import { Meta, Story, Primary, Controls, Canvas } from "@storybook/addon-docs"; + +import * as stories from "./item.stories"; + + + +```ts +import { ItemModule } from "@bitwarden/components"; +``` + +# Item + +`` is a horizontal card that contains one or more interactive actions. + +It is a generic container that can be used for either standalone content, an alternative to tables, +or to list nav links. + + + + + +## Primary Content + +The primary content of an item is supplied by `bit-item-content`. + +### Content Types + +The content can be a button, anchor, or static container. + +```html + + Hi, I am a link. + + + + + + + + I'm just static :( + +``` + + + + + +### Content Slots + +`bit-item-content` contains the following slots to help position the content: + +| Slot | Description | +| ------------------ | --------------------------------------------------- | +| default | primary text or arbitrary content; fan favorite | +| `slot="secondary"` | supporting text; under the default slot | +| `slot="start"` | commonly an icon or avatar; before the default slot | +| `slot="end"` | commonly an icon; after the default slot | + +- Note: There is also an `end` slot within `bit-item` itself. Place + [interactive secondary actions](#secondary-actions) there, and place non-interactive content (such + as icons) in `bit-item-content` + +```html + + + +``` + + + + + +## Secondary Actions + +Secondary interactive actions can be placed in the item through the `"end"` slot, outside of +`bit-item-content`. + +Each action must be wrapped by ``. + +Actions are commonly icon buttons or badge buttons. + +```html + + + + + + + + + + + + + + + +``` + +## Item Groups + +Groups of items can be associated by wrapping them in the ``. + + + + + + + + + +### A11y + +Keyboard nav is currently disabled due to a bug when used within a virtual scroll viewport. + +Item groups utilize arrow-based keyboard navigation +([further reading here](https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/layout-grids/#kbd_label)). + +Use `aria-label` or `aria-labelledby` to give groups an accessible name. + +```html + + ... + ... + ... + +``` + +### Virtual Scrolling + + + + diff --git a/libs/components/src/item/item.module.ts b/libs/components/src/item/item.module.ts new file mode 100644 index 0000000000..226fed11d8 --- /dev/null +++ b/libs/components/src/item/item.module.ts @@ -0,0 +1,12 @@ +import { NgModule } from "@angular/core"; + +import { ItemActionComponent } from "./item-action.component"; +import { ItemContentComponent } from "./item-content.component"; +import { ItemGroupComponent } from "./item-group.component"; +import { ItemComponent } from "./item.component"; + +@NgModule({ + imports: [ItemComponent, ItemContentComponent, ItemActionComponent, ItemGroupComponent], + exports: [ItemComponent, ItemContentComponent, ItemActionComponent, ItemGroupComponent], +}) +export class ItemModule {} diff --git a/libs/components/src/item/item.stories.ts b/libs/components/src/item/item.stories.ts new file mode 100644 index 0000000000..b9d8d6cc2e --- /dev/null +++ b/libs/components/src/item/item.stories.ts @@ -0,0 +1,326 @@ +import { ScrollingModule } from "@angular/cdk/scrolling"; +import { CommonModule } from "@angular/common"; +import { Meta, StoryObj, componentWrapperDecorator, moduleMetadata } from "@storybook/angular"; + +import { A11yGridDirective } from "../a11y/a11y-grid.directive"; +import { AvatarModule } from "../avatar"; +import { BadgeModule } from "../badge"; +import { IconButtonModule } from "../icon-button"; +import { TypographyModule } from "../typography"; + +import { ItemActionComponent } from "./item-action.component"; +import { ItemContentComponent } from "./item-content.component"; +import { ItemGroupComponent } from "./item-group.component"; +import { ItemComponent } from "./item.component"; + +export default { + title: "Component Library/Item", + component: ItemComponent, + decorators: [ + moduleMetadata({ + imports: [ + CommonModule, + ItemGroupComponent, + AvatarModule, + IconButtonModule, + BadgeModule, + TypographyModule, + ItemActionComponent, + ItemContentComponent, + A11yGridDirective, + ScrollingModule, + ], + }), + componentWrapperDecorator((story) => `
${story}
`), + ], +} as Meta; + +type Story = StoryObj; + +export const Default: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + + + + + + + + + + + + + `, + }), +}; + +export const ContentSlots: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + `, + }), +}; + +export const ContentTypes: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + Hi, I am a link. + + + + + + + + I'm just static :( + + + `, + }), +}; + +export const TextOverflow: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` +
TODO: Fix truncation
+ + + Helloooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo + + + `, + }), +}; + +export const MultipleActionList: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + `, + }), +}; + +export const SingleActionList: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + `, + }), +}; + +export const VirtualScrolling: Story = { + render: (_args) => ({ + props: { + data: Array.from(Array(100000).keys()), + }, + template: /*html*/ ` + + + + + + + + + + + + + + + + + + + + `, + }), +}; diff --git a/libs/components/src/search/search.component.ts b/libs/components/src/search/search.component.ts index a0f3eb363f..27170d5d7b 100644 --- a/libs/components/src/search/search.component.ts +++ b/libs/components/src/search/search.component.ts @@ -1,7 +1,7 @@ import { Component, ElementRef, Input, ViewChild } from "@angular/core"; import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms"; -import { FocusableElement } from "../input/autofocus.directive"; +import { FocusableElement } from "../shared/focusable-element"; let nextId = 0; @@ -32,8 +32,8 @@ export class SearchComponent implements ControlValueAccessor, FocusableElement { @Input() disabled: boolean; @Input() placeholder: string; - focus() { - this.input.nativeElement.focus(); + getFocusTarget() { + return this.input.nativeElement; } onChange(searchText: string) { diff --git a/libs/components/src/shared/focusable-element.ts b/libs/components/src/shared/focusable-element.ts new file mode 100644 index 0000000000..1ea422aa6f --- /dev/null +++ b/libs/components/src/shared/focusable-element.ts @@ -0,0 +1,8 @@ +/** + * Interface for implementing focusable components. + * + * Used by the `AutofocusDirective` and `A11yGridDirective`. + */ +export abstract class FocusableElement { + getFocusTarget: () => HTMLElement; +} diff --git a/libs/components/src/styles.scss b/libs/components/src/styles.scss index ae97838e09..7ddcb1b64b 100644 --- a/libs/components/src/styles.scss +++ b/libs/components/src/styles.scss @@ -49,6 +49,6 @@ $card-icons-base: "../../src/billing/images/cards/"; @import "multi-select/scss/bw.theme.scss"; // Workaround for https://bitwarden.atlassian.net/browse/CL-110 -#storybook-docs pre.prismjs { +.sbdocs-preview pre.prismjs { color: white; } diff --git a/package-lock.json b/package-lock.json index 4c652b7419..ba0119ca63 100644 --- a/package-lock.json +++ b/package-lock.json @@ -114,7 +114,7 @@ "@types/node-ipc": "9.2.3", "@types/papaparse": "5.3.14", "@types/proper-lockfile": "4.1.4", - "@types/react": "16.14.57", + "@types/react": "16.14.60", "@types/retry": "0.12.5", "@types/zxcvbn": "4.4.4", "@typescript-eslint/eslint-plugin": "7.7.1", @@ -163,8 +163,8 @@ "prettier": "3.2.2", "prettier-plugin-tailwindcss": "0.5.14", "process": "0.11.10", - "react": "18.2.0", - "react-dom": "18.2.0", + "react": "18.3.1", + "react-dom": "18.3.1", "regedit": "^3.0.3", "remark-gfm": "3.0.1", "rimraf": "5.0.5", @@ -10618,13 +10618,13 @@ "dev": true }, "node_modules/@types/react": { - "version": "16.14.57", - "resolved": "https://registry.npmjs.org/@types/react/-/react-16.14.57.tgz", - "integrity": "sha512-fuNq/GV1a6GgqSuVuC457vYeTbm4E1CUBQVZwSPxqYnRhIzSXCJ1gGqyv+PKhqLyfbKCga9dXHJDzv+4XE41fw==", + "version": "16.14.60", + "resolved": "https://registry.npmjs.org/@types/react/-/react-16.14.60.tgz", + "integrity": "sha512-wIFmnczGsTcgwCBeIYOuy2mdXEiKZ5znU/jNOnMZPQyCcIxauMGWlX0TNG4lZ7NxRKj7YUIZRneJQSSdB2jKgg==", "dev": true, "dependencies": { "@types/prop-types": "*", - "@types/scheduler": "*", + "@types/scheduler": "^0.16", "csstype": "^3.0.2" } }, @@ -32085,9 +32085,9 @@ } }, "node_modules/react": { - "version": "18.2.0", - "resolved": "https://registry.npmjs.org/react/-/react-18.2.0.tgz", - "integrity": "sha512-/3IjMdb2L9QbBdWiW5e3P2/npwMBaU9mHCSCUzNln0ZCYbcfTsGbTJrU/kGemdH2IWmB2ioZ+zkxtmq6g09fGQ==", + "version": "18.3.1", + "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", + "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", "dev": true, "dependencies": { "loose-envify": "^1.1.0" @@ -32107,16 +32107,16 @@ } }, "node_modules/react-dom": { - "version": "18.2.0", - "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.2.0.tgz", - "integrity": "sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==", + "version": "18.3.1", + "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.3.1.tgz", + "integrity": "sha512-5m4nQKp+rZRb09LNH59GM4BxTh9251/ylbKIbpe7TpGxfJ+9kv6BLkLBXIjjspbgbnIBNqlI23tRnTWT0snUIw==", "dev": true, "dependencies": { "loose-envify": "^1.1.0", - "scheduler": "^0.23.0" + "scheduler": "^0.23.2" }, "peerDependencies": { - "react": "^18.2.0" + "react": "^18.3.1" } }, "node_modules/react-is": { @@ -33604,9 +33604,9 @@ } }, "node_modules/scheduler": { - "version": "0.23.0", - "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.23.0.tgz", - "integrity": "sha512-CtuThmgHNg7zIZWAXi3AsyIzA3n4xx7aNyjwC2VJldO2LMVDhFK+63xGqq6CsJH4rTAt6/M+N4GhZiDYPx9eUw==", + "version": "0.23.2", + "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.23.2.tgz", + "integrity": "sha512-UOShsPwz7NrMUqhR6t0hWjFduvOzbtv7toDH1/hIrfRNIDBnnBWd0CwJTGvTpngVlmwGCdP9/Zl/tVrDqcuYzQ==", "dev": true, "dependencies": { "loose-envify": "^1.1.0" diff --git a/package.json b/package.json index 6c1fac2e54..c73ae492f5 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "@types/node-ipc": "9.2.3", "@types/papaparse": "5.3.14", "@types/proper-lockfile": "4.1.4", - "@types/react": "16.14.57", + "@types/react": "16.14.60", "@types/retry": "0.12.5", "@types/zxcvbn": "4.4.4", "@typescript-eslint/eslint-plugin": "7.7.1", @@ -124,8 +124,8 @@ "prettier": "3.2.2", "prettier-plugin-tailwindcss": "0.5.14", "process": "0.11.10", - "react": "18.2.0", - "react-dom": "18.2.0", + "react": "18.3.1", + "react-dom": "18.3.1", "regedit": "^3.0.3", "remark-gfm": "3.0.1", "rimraf": "5.0.5", @@ -213,7 +213,7 @@ "replacestream": "4.0.3" }, "resolutions": { - "@types/react": "16.14.57" + "@types/react": "16.14.60" }, "lint-staged": { "*": "prettier --cache --ignore-unknown --write",