diff --git a/apps/browser/src/auth/background/service-factories/avatar-service.factory.ts b/apps/browser/src/auth/background/service-factories/avatar-service.factory.ts new file mode 100644 index 00000000000..456edfa93da --- /dev/null +++ b/apps/browser/src/auth/background/service-factories/avatar-service.factory.ts @@ -0,0 +1,38 @@ +import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; +import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; + +import { + ApiServiceInitOptions, + apiServiceFactory, +} from "../../../platform/background/service-factories/api-service.factory"; +import { + CachedServices, + factory, + FactoryOptions, +} from "../../../platform/background/service-factories/factory-options"; +import { + stateProviderFactory, + StateProviderInitOptions, +} from "../../../platform/background/service-factories/state-provider.factory"; + +type AvatarServiceFactoryOptions = FactoryOptions; + +export type AvatarServiceInitOptions = AvatarServiceFactoryOptions & + ApiServiceInitOptions & + StateProviderInitOptions; + +export function avatarServiceFactory( + cache: { avatarService?: AvatarServiceAbstraction } & CachedServices, + opts: AvatarServiceInitOptions, +): Promise { + return factory( + cache, + "avatarService", + opts, + async () => + new AvatarService( + await apiServiceFactory(cache, opts), + await stateProviderFactory(cache, opts), + ), + ); +} diff --git a/apps/browser/src/auth/popup/account-switching/current-account.component.ts b/apps/browser/src/auth/popup/account-switching/current-account.component.ts index 06612b8d338..1c7f93bf304 100644 --- a/apps/browser/src/auth/popup/account-switching/current-account.component.ts +++ b/apps/browser/src/auth/popup/account-switching/current-account.component.ts @@ -1,32 +1,61 @@ import { Location } from "@angular/common"; import { Component } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; +import { Observable, combineLatest, switchMap } from "rxjs"; -import { CurrentAccountService } from "./services/current-account.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { UserId } from "@bitwarden/common/types/guid"; + +export type CurrentAccount = { + id: UserId; + name: string | undefined; + email: string; + status: AuthenticationStatus; + avatarColor: string; +}; @Component({ selector: "app-current-account", templateUrl: "current-account.component.html", }) export class CurrentAccountComponent { + currentAccount$: Observable; + constructor( - private currentAccountService: CurrentAccountService, + private accountService: AccountService, + private avatarService: AvatarService, private router: Router, private location: Location, private route: ActivatedRoute, - ) {} + ) { + this.currentAccount$ = combineLatest([ + this.accountService.activeAccount$, + this.avatarService.avatarColor$, + ]).pipe( + switchMap(async ([account, avatarColor]) => { + if (account == null) { + return null; + } + const currentAccount: CurrentAccount = { + id: account.id, + name: account.name || account.email, + email: account.email, + status: account.status, + avatarColor, + }; - get currentAccount$() { - return this.currentAccountService.currentAccount$; + return currentAccount; + }), + ); } async currentAccountClicked() { if (this.route.snapshot.data.state.includes("account-switcher")) { this.location.back(); } else { - // 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(["/account-switcher"]); + await this.router.navigate(["/account-switcher"]); } } } diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index 9845fac1dad..f02a8ee2016 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -1,12 +1,12 @@ import { matches, mock } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom, timeout } from "rxjs"; +import { BehaviorSubject, firstValueFrom, of, timeout } from "rxjs"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { UserId } from "@bitwarden/common/types/guid"; import { AccountSwitcherService } from "./account-switcher.service"; @@ -16,7 +16,7 @@ describe("AccountSwitcherService", () => { const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>(null); const accountService = mock(); - const stateService = mock(); + const avatarService = mock(); const messagingService = mock(); const environmentService = mock(); const logService = mock(); @@ -25,11 +25,13 @@ describe("AccountSwitcherService", () => { beforeEach(() => { jest.resetAllMocks(); + accountService.accounts$ = accountsSubject; accountService.activeAccount$ = activeAccountSubject; + accountSwitcherService = new AccountSwitcherService( accountService, - stateService, + avatarService, messagingService, environmentService, logService, @@ -44,6 +46,7 @@ describe("AccountSwitcherService", () => { status: AuthenticationStatus.Unlocked, }; + avatarService.getUserAvatarColor$.mockReturnValue(of("#cccccc")); accountsSubject.next({ "1": user1AccountInfo, } as Record); @@ -72,6 +75,7 @@ describe("AccountSwitcherService", () => { status: AuthenticationStatus.Unlocked, }; } + avatarService.getUserAvatarColor$.mockReturnValue(of("#cccccc")); accountsSubject.next(seedAccounts); activeAccountSubject.next( Object.assign(seedAccounts["1" as UserId], { id: "1" as UserId }), diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts index 0b1015544c1..cf78f2ff913 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -11,11 +11,11 @@ import { } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { UserId } from "@bitwarden/common/types/guid"; import { fromChromeEvent } from "../../../../platform/browser/from-chrome-event"; @@ -44,7 +44,7 @@ export class AccountSwitcherService { constructor( private accountService: AccountService, - private stateService: StateService, + private avatarService: AvatarService, private messagingService: MessagingService, private environmentService: EnvironmentService, private logService: LogService, @@ -68,7 +68,9 @@ export class AccountSwitcherService { server: await this.environmentService.getHost(id), status: account.status, isActive: id === activeAccount?.id, - avatarColor: await this.stateService.getAvatarColor({ userId: id }), + avatarColor: await firstValueFrom( + this.avatarService.getUserAvatarColor$(id as UserId), + ), }; }), ); diff --git a/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts b/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts deleted file mode 100644 index 21fc3bdac43..00000000000 --- a/apps/browser/src/auth/popup/account-switching/services/current-account.service.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { Injectable } from "@angular/core"; -import { Observable, switchMap } from "rxjs"; - -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { UserId } from "@bitwarden/common/types/guid"; - -export type CurrentAccount = { - id: UserId; - name: string | undefined; - email: string; - status: AuthenticationStatus; - avatarColor: string; -}; - -@Injectable({ - providedIn: "root", -}) -export class CurrentAccountService { - currentAccount$: Observable; - - constructor( - private accountService: AccountService, - private stateService: StateService, - ) { - this.currentAccount$ = this.accountService.activeAccount$.pipe( - switchMap(async (account) => { - if (account == null) { - return null; - } - const currentAccount: CurrentAccount = { - id: account.id, - name: account.name || account.email, - email: account.email, - status: account.status, - avatarColor: await this.stateService.getAvatarColor({ userId: account.id }), - }; - - return currentAccount; - }), - ); - } -} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 23160f8a144..fd2208fe3c8 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -8,7 +8,6 @@ import { AuthRequestServiceAbstraction, AuthRequestService, } from "@bitwarden/auth/common"; -import { AvatarUpdateService as AvatarUpdateServiceAbstraction } from "@bitwarden/common/abstractions/account/avatar-update.service"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -39,6 +38,7 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authenticatio import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; +import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { DeviceTrustCryptoService } from "@bitwarden/common/auth/services/device-trust-crypto.service.implementation"; import { DevicesServiceImplementation } from "@bitwarden/common/auth/services/devices/devices.service.implementation"; import { DevicesApiServiceImplementation } from "@bitwarden/common/auth/services/devices-api.service.implementation"; @@ -117,7 +117,6 @@ import { DefaultStateProvider } from "@bitwarden/common/platform/state/implement import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service"; /* eslint-enable import/no-restricted-paths */ import { DefaultThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; -import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service"; import { ApiService } from "@bitwarden/common/services/api.service"; import { AuditService } from "@bitwarden/common/services/audit.service"; import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service"; @@ -125,6 +124,7 @@ import { EventUploadService } from "@bitwarden/common/services/event/event-uploa import { NotificationsService } from "@bitwarden/common/services/notifications.service"; import { SearchService } from "@bitwarden/common/services/search.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/services/vault-timeout/vault-timeout-settings.service"; +import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/src/auth/abstractions/avatar.service"; import { PasswordGenerationService, PasswordGenerationServiceAbstraction, @@ -288,7 +288,7 @@ export default class MainBackground { fido2UserInterfaceService: Fido2UserInterfaceServiceAbstraction; fido2AuthenticatorService: Fido2AuthenticatorServiceAbstraction; fido2ClientService: Fido2ClientServiceAbstraction; - avatarUpdateService: AvatarUpdateServiceAbstraction; + avatarService: AvatarServiceAbstraction; mainContextMenuHandler: MainContextMenuHandler; cipherContextMenuHandler: CipherContextMenuHandler; configService: BrowserConfigService; @@ -685,7 +685,11 @@ export default class MainBackground { this.fileUploadService, this.sendService, ); + + this.avatarService = new AvatarService(this.apiService, this.stateProvider); + this.providerService = new ProviderService(this.stateProvider); + this.syncService = new SyncService( this.apiService, this.domainSettingsService, @@ -703,6 +707,7 @@ export default class MainBackground { this.folderApiService, this.organizationService, this.sendApiService, + this.avatarService, logoutCallback, ); this.eventUploadService = new EventUploadService( @@ -943,8 +948,6 @@ export default class MainBackground { this.apiService, ); - this.avatarUpdateService = new AvatarUpdateService(this.apiService, this.stateService); - if (!this.popupOnlyContext) { this.mainContextMenuHandler = new MainContextMenuHandler( this.stateService, diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 6c0c0f169aa..55be9ba64d1 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -130,9 +130,6 @@ export default class RuntimeBackground { await this.main.refreshBadge(); await this.main.refreshMenu(); }, 2000); - // 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.main.avatarUpdateService.loadColorFromState(); this.configService.triggerServerConfigFetch(); } break; diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 031be3b48c5..1e624de1b15 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -23,10 +23,12 @@ import { PolicyApiService } from "@bitwarden/common/admin-console/services/polic import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; +import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { DeviceTrustCryptoService } from "@bitwarden/common/auth/services/device-trust-crypto.service.implementation"; import { DevicesApiServiceImplementation } from "@bitwarden/common/auth/services/devices-api.service.implementation"; import { KeyConnectorService } from "@bitwarden/common/auth/services/key-connector.service"; @@ -216,6 +218,7 @@ export class Main { derivedStateProvider: DerivedStateProvider; stateProvider: StateProvider; loginStrategyService: LoginStrategyServiceAbstraction; + avatarService: AvatarServiceAbstraction; stateEventRunnerService: StateEventRunnerService; biometricStateService: BiometricStateService; @@ -555,6 +558,8 @@ export class Main { null, ); + this.avatarService = new AvatarService(this.apiService, this.stateProvider); + this.syncService = new SyncService( this.apiService, this.domainSettingsService, @@ -572,6 +577,7 @@ export class Main { this.folderApiService, this.organizationService, this.sendApiService, + this.avatarService, async (expired: boolean) => await this.logout(), ); diff --git a/apps/desktop/src/app/layout/account-switcher.component.ts b/apps/desktop/src/app/layout/account-switcher.component.ts index 7e6256e9ba6..795870b3054 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.ts +++ b/apps/desktop/src/app/layout/account-switcher.component.ts @@ -2,9 +2,10 @@ import { animate, state, style, transition, trigger } from "@angular/animations" import { ConnectedPosition } from "@angular/cdk/overlay"; import { Component, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; -import { concatMap, Subject, takeUntil } from "rxjs"; +import { concatMap, firstValueFrom, Subject, takeUntil } from "rxjs"; 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"; @@ -12,6 +13,7 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag 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 = { id: string; @@ -84,6 +86,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { constructor( private stateService: StateService, private authService: AuthService, + private avatarService: AvatarService, private messagingService: MessagingService, private router: Router, private tokenService: TokenService, @@ -101,7 +104,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { id: await this.tokenService.getUserId(), name: (await this.tokenService.getName()) ?? (await this.tokenService.getEmail()), email: await this.tokenService.getEmail(), - avatarColor: await this.stateService.getAvatarColor(), + avatarColor: await firstValueFrom(this.avatarService.avatarColor$), server: await this.environmentService.getHost(), }; } catch { @@ -154,7 +157,7 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { name: baseAccounts[userId].profile.name, email: baseAccounts[userId].profile.email, authenticationStatus: await this.authService.getAuthStatus(userId), - avatarColor: await this.stateService.getAvatarColor({ userId: userId }), + avatarColor: await firstValueFrom(this.avatarService.getUserAvatarColor$(userId as UserId)), server: await this.environmentService.getHost(userId), }; } diff --git a/apps/web/src/app/auth/settings/account/change-avatar.component.ts b/apps/web/src/app/auth/settings/account/change-avatar.component.ts index 2efe554bbe4..bbcbaf6820f 100644 --- a/apps/web/src/app/auth/settings/account/change-avatar.component.ts +++ b/apps/web/src/app/auth/settings/account/change-avatar.component.ts @@ -9,9 +9,9 @@ import { ViewChild, ViewEncapsulation, } from "@angular/core"; -import { BehaviorSubject, debounceTime, Subject, takeUntil } from "rxjs"; +import { BehaviorSubject, debounceTime, firstValueFrom, Subject, takeUntil } from "rxjs"; -import { AvatarUpdateService } from "@bitwarden/common/abstractions/account/avatar-update.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { ProfileResponse } from "@bitwarden/common/models/response/profile.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -55,7 +55,7 @@ export class ChangeAvatarComponent implements OnInit, OnDestroy { private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, private logService: LogService, - private accountUpdateService: AvatarUpdateService, + private avatarService: AvatarService, ) {} async ngOnInit() { @@ -73,9 +73,7 @@ export class ChangeAvatarComponent implements OnInit, OnDestroy { this.currentSelection = color; }); - // 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.setSelection(await this.accountUpdateService.loadColorFromState()); + await this.setSelection(await firstValueFrom(this.avatarService.avatarColor$)); } async showCustomPicker() { @@ -93,7 +91,7 @@ export class ChangeAvatarComponent implements OnInit, OnDestroy { async submit() { try { if (Utils.validateHexColor(this.currentSelection) || this.currentSelection == null) { - await this.accountUpdateService.pushUpdate(this.currentSelection); + await this.avatarService.setAvatarColor(this.currentSelection); this.changeColor.emit(this.currentSelection); this.platformUtilsService.showToast("success", null, this.i18nService.t("avatarUpdated")); } else { diff --git a/apps/web/src/app/components/dynamic-avatar.component.ts b/apps/web/src/app/components/dynamic-avatar.component.ts index e9eea873a57..4cdfda4eba8 100644 --- a/apps/web/src/app/components/dynamic-avatar.component.ts +++ b/apps/web/src/app/components/dynamic-avatar.component.ts @@ -1,7 +1,7 @@ import { Component, Input, OnDestroy } from "@angular/core"; -import { Observable, Subject } from "rxjs"; +import { Subject } from "rxjs"; -import { AvatarUpdateService } from "@bitwarden/common/abstractions/account/avatar-update.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { SharedModule } from "../shared"; @@ -29,14 +29,14 @@ export class DynamicAvatarComponent implements OnDestroy { @Input() text: string; @Input() title: string; @Input() size: SizeTypes = "default"; - color$: Observable; private destroy$ = new Subject(); - constructor(private accountUpdateService: AvatarUpdateService) { + color$ = this.avatarService.avatarColor$; + + constructor(private avatarService: AvatarService) { if (this.text) { this.text = this.text.toUpperCase(); } - this.color$ = this.accountUpdateService.avatarUpdate$; } async ngOnDestroy() { 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 33486fcbb81..8f33c501646 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 @@ -1,12 +1,12 @@ import { importProvidersFrom } from "@angular/core"; import { RouterModule } from "@angular/router"; import { applicationConfig, Meta, moduleMetadata, Story } from "@storybook/angular"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; -import { AvatarUpdateService } from "@bitwarden/common/abstractions/account/avatar-update.service"; import { SettingsService } from "@bitwarden/common/abstractions/settings.service"; import { OrganizationUserType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; @@ -72,12 +72,10 @@ export default { } as Partial, }, { - provide: AvatarUpdateService, + provide: AvatarService, useValue: { - async loadColorFromState() { - return "#FF0000"; - }, - } as Partial, + avatarColor$: of("#FF0000"), + } as Partial, }, { provide: TokenService, diff --git a/apps/web/src/app/vault/individual-vault/organization-badge/organization-name-badge.component.ts b/apps/web/src/app/vault/individual-vault/organization-badge/organization-name-badge.component.ts index 954e558a1d9..6d53b8ad720 100644 --- a/apps/web/src/app/vault/individual-vault/organization-badge/organization-name-badge.component.ts +++ b/apps/web/src/app/vault/individual-vault/organization-badge/organization-name-badge.component.ts @@ -1,6 +1,7 @@ import { Component, Input, OnChanges } from "@angular/core"; +import { firstValueFrom } from "rxjs"; -import { AvatarUpdateService } from "@bitwarden/common/abstractions/account/avatar-update.service"; +import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -24,7 +25,7 @@ export class OrganizationNameBadgeComponent implements OnChanges { constructor( private i18nService: I18nService, - private avatarService: AvatarUpdateService, + private avatarService: AvatarService, private tokenService: TokenService, ) {} @@ -35,7 +36,7 @@ export class OrganizationNameBadgeComponent implements OnChanges { if (this.isMe) { this.name = this.i18nService.t("me"); - this.color = await this.avatarService.loadColorFromState(); + this.color = await firstValueFrom(this.avatarService.avatarColor$); if (this.color == null) { const userId = await this.tokenService.getUserId(); if (userId != null) { diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 743aead0ca9..7aecdccbea3 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -9,7 +9,6 @@ import { LoginStrategyServiceAbstraction, LoginStrategyService, } from "@bitwarden/auth/common"; -import { AvatarUpdateService as AccountUpdateServiceAbstraction } from "@bitwarden/common/abstractions/account/avatar-update.service"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -51,6 +50,7 @@ import { } from "@bitwarden/common/auth/abstractions/account.service"; import { AnonymousHubService as AnonymousHubServiceAbstraction } from "@bitwarden/common/auth/abstractions/anonymous-hub.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust-crypto.service.abstraction"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices-api.service.abstraction"; @@ -69,6 +69,7 @@ import { AccountApiServiceImplementation } from "@bitwarden/common/auth/services import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AnonymousHubService } from "@bitwarden/common/auth/services/anonymous-hub.service"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; +import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { DeviceTrustCryptoService } from "@bitwarden/common/auth/services/device-trust-crypto.service.implementation"; import { DevicesServiceImplementation } from "@bitwarden/common/auth/services/devices/devices.service.implementation"; import { DevicesApiServiceImplementation } from "@bitwarden/common/auth/services/devices-api.service.implementation"; @@ -163,7 +164,6 @@ import { DefaultThemeStateService, ThemeStateService, } from "@bitwarden/common/platform/theming/theme-state.service"; -import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service"; import { ApiService } from "@bitwarden/common/services/api.service"; import { AuditService } from "@bitwarden/common/services/audit.service"; import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service"; @@ -452,9 +452,9 @@ const typesafeProviders: Array = [ useExisting: InternalAccountService, }), safeProvider({ - provide: AccountUpdateServiceAbstraction, - useClass: AvatarUpdateService, - deps: [ApiServiceAbstraction, StateServiceAbstraction], + provide: AvatarServiceAbstraction, + useClass: AvatarService, + deps: [ApiServiceAbstraction, StateProvider], }), safeProvider({ provide: LogService, useFactory: () => new ConsoleLogService(false), deps: [] }), safeProvider({ @@ -561,6 +561,7 @@ const typesafeProviders: Array = [ FolderApiServiceAbstraction, InternalOrganizationServiceAbstraction, SendApiServiceAbstraction, + AvatarServiceAbstraction, LOGOUT_CALLBACK, ], }), diff --git a/libs/common/src/abstractions/account/avatar-update.service.ts b/libs/common/src/abstractions/account/avatar-update.service.ts deleted file mode 100644 index 980942dfe39..00000000000 --- a/libs/common/src/abstractions/account/avatar-update.service.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { Observable } from "rxjs"; - -import { ProfileResponse } from "../../models/response/profile.response"; -export abstract class AvatarUpdateService { - avatarUpdate$ = new Observable(); - abstract pushUpdate(color: string): Promise; - abstract loadColorFromState(): Promise; -} diff --git a/libs/common/src/auth/abstractions/avatar.service.ts b/libs/common/src/auth/abstractions/avatar.service.ts new file mode 100644 index 00000000000..1192ef745df --- /dev/null +++ b/libs/common/src/auth/abstractions/avatar.service.ts @@ -0,0 +1,29 @@ +import { Observable } from "rxjs"; + +import { UserId } from "../../types/guid"; + +export abstract class AvatarService { + /** + * An observable monitoring the active user's avatar color. + * The observable updates when the avatar color changes. + */ + avatarColor$: Observable; + /** + * Sets the avatar color of the active user + * + * @param color the color to set the avatar color to + * @returns a promise that resolves when the avatar color is set + */ + abstract setAvatarColor(color: string): Promise; + /** + * Gets the avatar color of the specified user. + * + * @remarks This is most useful for account switching where we show an + * avatar for each account. If you only need the active user's + * avatar color, use the avatarColor$ observable above instead. + * + * @param userId the userId of the user whose avatar color should be retreived + * @return an Observable that emits a string of the avatar color of the specified user + */ + abstract getUserAvatarColor$(userId: UserId): Observable; +} diff --git a/libs/common/src/auth/services/avatar.service.ts b/libs/common/src/auth/services/avatar.service.ts new file mode 100644 index 00000000000..b770dc39b98 --- /dev/null +++ b/libs/common/src/auth/services/avatar.service.ts @@ -0,0 +1,33 @@ +import { Observable } from "rxjs"; + +import { ApiService } from "../../abstractions/api.service"; +import { UpdateAvatarRequest } from "../../models/request/update-avatar.request"; +import { AVATAR_DISK, StateProvider, UserKeyDefinition } from "../../platform/state"; +import { UserId } from "../../types/guid"; +import { AvatarService as AvatarServiceAbstraction } from "../abstractions/avatar.service"; + +const AVATAR_COLOR = new UserKeyDefinition(AVATAR_DISK, "avatarColor", { + deserializer: (value) => value, + clearOn: [], +}); + +export class AvatarService implements AvatarServiceAbstraction { + avatarColor$: Observable; + + constructor( + private apiService: ApiService, + private stateProvider: StateProvider, + ) { + this.avatarColor$ = this.stateProvider.getActive(AVATAR_COLOR).state$; + } + + async setAvatarColor(color: string): Promise { + const { avatarColor } = await this.apiService.putAvatar(new UpdateAvatarRequest(color)); + + await this.stateProvider.setUserState(AVATAR_COLOR, avatarColor); + } + + getUserAvatarColor$(userId: UserId): Observable { + return this.stateProvider.getUser(userId, AVATAR_COLOR).state$; + } +} diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 1ec764974f7..3fc65e4acf1 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -359,9 +359,6 @@ export abstract class StateService { * @deprecated Do not call this directly, use ConfigService */ setServerConfig: (value: ServerConfigData, options?: StorageOptions) => Promise; - - getAvatarColor: (options?: StorageOptions) => Promise; - setAvatarColor: (value: string, options?: StorageOptions) => Promise; /** * fetches string value of URL user tried to navigate to while unauthenticated. * @param options Defines the storage options for the URL; Defaults to session Storage. diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 6722cb93477..08c5350d06d 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1854,23 +1854,6 @@ export class StateService< )?.settings?.serverConfig; } - async getAvatarColor(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskLocalOptions())) - )?.settings?.avatarColor; - } - - async setAvatarColor(value: string, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()), - ); - account.settings.avatarColor = value; - return await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskLocalOptions()), - ); - } - async getDeepLinkRedirectUrl(options?: StorageOptions): Promise { return ( await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())) diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index ef5d9d7721e..86b8dd051cb 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -26,6 +26,7 @@ export const PROVIDERS_DISK = new StateDefinition("providers", "disk"); // Auth export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); +export const AVATAR_DISK = new StateDefinition("avatar", "disk", { web: "disk-local" }); export const SSO_DISK = new StateDefinition("ssoLogin", "disk"); export const LOGIN_STRATEGY_MEMORY = new StateDefinition("loginStrategy", "memory"); diff --git a/libs/common/src/services/account/avatar-update.service.ts b/libs/common/src/services/account/avatar-update.service.ts deleted file mode 100644 index d7dee793bdb..00000000000 --- a/libs/common/src/services/account/avatar-update.service.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { BehaviorSubject, Observable } from "rxjs"; - -import { AvatarUpdateService as AvatarUpdateServiceAbstraction } from "../../abstractions/account/avatar-update.service"; -import { ApiService } from "../../abstractions/api.service"; -import { UpdateAvatarRequest } from "../../models/request/update-avatar.request"; -import { ProfileResponse } from "../../models/response/profile.response"; -import { StateService } from "../../platform/abstractions/state.service"; - -export class AvatarUpdateService implements AvatarUpdateServiceAbstraction { - private _avatarUpdate$ = new BehaviorSubject(null); - avatarUpdate$: Observable = this._avatarUpdate$.asObservable(); - - constructor( - private apiService: ApiService, - private stateService: StateService, - ) { - // 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.loadColorFromState(); - } - - loadColorFromState(): Promise { - return this.stateService.getAvatarColor().then((color) => { - this._avatarUpdate$.next(color); - return color; - }); - } - - pushUpdate(color: string | null): Promise { - return this.apiService.putAvatar(new UpdateAvatarRequest(color)).then((response) => { - // 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.setAvatarColor(response.avatarColor); - this._avatarUpdate$.next(response.avatarColor); - }); - } -} diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 1051ee952bc..77a35ccb871 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -32,6 +32,7 @@ import { AppIdMigrator } from "./migrations/33-move-app-id-to-state-providers"; import { DomainSettingsMigrator } from "./migrations/34-move-domain-settings-to-state-providers"; import { MoveThemeToStateProviderMigrator } from "./migrations/35-move-theme-to-state-providers"; import { VaultSettingsKeyMigrator } from "./migrations/36-move-show-card-and-identity-to-state-provider"; +import { AvatarColorMigrator } from "./migrations/37-move-avatar-color-to-state-providers"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -41,7 +42,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 36; +export const CURRENT_VERSION = 37; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -80,7 +81,8 @@ export function createMigrationBuilder() { .with(AppIdMigrator, 32, 33) .with(DomainSettingsMigrator, 33, 34) .with(MoveThemeToStateProviderMigrator, 34, 35) - .with(VaultSettingsKeyMigrator, 35, CURRENT_VERSION); + .with(VaultSettingsKeyMigrator, 35, 36) + .with(AvatarColorMigrator, 36, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.spec.ts b/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.spec.ts new file mode 100644 index 00000000000..c87c9f42f0d --- /dev/null +++ b/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.spec.ts @@ -0,0 +1,143 @@ +import { MockProxy } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper, runMigrator } from "../migration-helper.spec"; + +import { AvatarColorMigrator } from "./37-move-avatar-color-to-state-providers"; + +function rollbackJSON() { + return { + authenticatedAccounts: ["user-1", "user-2"], + "user_user-1_avatar_avatarColor": "#ff0000", + "user_user-2_avatar_avatarColor": "#cccccc", + "user-1": { + settings: { + extra: "data", + }, + extra: "data", + }, + "user-2": { + settings: { + extra: "data", + }, + extra: "data", + }, + }; +} + +describe("AvatarColorMigrator", () => { + const migrator = new AvatarColorMigrator(36, 37); + + it("should migrate the avatarColor property from the account settings object to a user StorageKey", async () => { + const output = await runMigrator(migrator, { + authenticatedAccounts: ["user-1", "user-2"] as const, + "user-1": { + settings: { + avatarColor: "#ff0000", + extra: "data", + }, + extra: "data", + }, + "user-2": { + settings: { + avatarColor: "#cccccc", + extra: "data", + }, + extra: "data", + }, + }); + + expect(output).toEqual({ + authenticatedAccounts: ["user-1", "user-2"], + "user_user-1_avatar_avatarColor": "#ff0000", + "user_user-2_avatar_avatarColor": "#cccccc", + "user-1": { + settings: { + extra: "data", + }, + extra: "data", + }, + "user-2": { + settings: { + extra: "data", + }, + extra: "data", + }, + }); + }); + + it("should handle missing parts", async () => { + const output = await runMigrator(migrator, { + authenticatedAccounts: ["user-1", "user-2"], + global: { + extra: "data", + }, + "user-1": { + extra: "data", + settings: { + extra: "data", + }, + }, + "user-2": null, + }); + + expect(output).toEqual({ + authenticatedAccounts: ["user-1", "user-2"], + global: { + extra: "data", + }, + "user-1": { + extra: "data", + settings: { + extra: "data", + }, + }, + "user-2": null, + }); + }); + + describe("rollback", () => { + let helper: MockProxy; + let sut: AvatarColorMigrator; + + const keyDefinitionLike = { + key: "avatarColor", + stateDefinition: { + name: "avatar", + }, + }; + + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 37); + sut = new AvatarColorMigrator(36, 37); + }); + + it("should null out the avatarColor user StorageKey for each account", async () => { + await sut.rollback(helper); + + expect(helper.setToUser).toHaveBeenCalledTimes(2); + expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinitionLike, null); + expect(helper.setToUser).toHaveBeenCalledWith("user-2", keyDefinitionLike, null); + }); + + it("should add the avatarColor property back to the account settings object", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(2); + expect(helper.set).toHaveBeenCalledWith("user-1", { + settings: { + avatarColor: "#ff0000", + extra: "data", + }, + extra: "data", + }); + expect(helper.set).toHaveBeenCalledWith("user-2", { + settings: { + avatarColor: "#cccccc", + extra: "data", + }, + extra: "data", + }); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.ts b/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.ts new file mode 100644 index 00000000000..36173cc909c --- /dev/null +++ b/libs/common/src/state-migrations/migrations/37-move-avatar-color-to-state-providers.ts @@ -0,0 +1,57 @@ +import { KeyDefinitionLike, MigrationHelper, StateDefinitionLike } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedAccountState = { + settings?: { avatarColor?: string }; +}; + +const AVATAR_COLOR_STATE: StateDefinitionLike = { name: "avatar" }; + +const AVATAR_COLOR_KEY: KeyDefinitionLike = { + key: "avatarColor", + stateDefinition: AVATAR_COLOR_STATE, +}; + +export class AvatarColorMigrator extends Migrator<36, 37> { + async migrate(helper: MigrationHelper): Promise { + const legacyAccounts = await helper.getAccounts(); + + await Promise.all( + legacyAccounts.map(async ({ userId, account }) => { + // Move account avatarColor + if (account?.settings?.avatarColor != null) { + await helper.setToUser(userId, AVATAR_COLOR_KEY, account.settings.avatarColor); + + // Delete old account avatarColor property + delete account?.settings?.avatarColor; + await helper.set(userId, account); + } + }), + ); + } + + async rollback(helper: MigrationHelper): Promise { + async function rollbackUser(userId: string, account: ExpectedAccountState) { + let updatedAccount = false; + const userAvatarColor = await helper.getFromUser(userId, AVATAR_COLOR_KEY); + + if (userAvatarColor) { + if (!account) { + account = {}; + } + + updatedAccount = true; + account.settings.avatarColor = userAvatarColor; + await helper.setToUser(userId, AVATAR_COLOR_KEY, null); + } + + if (updatedAccount) { + await helper.set(userId, account); + } + } + + const accounts = await helper.getAccounts(); + + await Promise.all(accounts.map(({ userId, account }) => rollbackUser(userId, account))); + } +} diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 200acf97f1b..1b46bf43294 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -7,6 +7,7 @@ import { OrganizationData } from "../../../admin-console/models/data/organizatio import { PolicyData } from "../../../admin-console/models/data/policy.data"; import { ProviderData } from "../../../admin-console/models/data/provider.data"; import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; +import { AvatarService } from "../../../auth/abstractions/avatar.service"; import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { DomainSettingsService } from "../../../autofill/services/domain-settings.service"; @@ -59,6 +60,7 @@ export class SyncService implements SyncServiceAbstraction { private folderApiService: FolderApiServiceAbstraction, private organizationService: InternalOrganizationServiceAbstraction, private sendApiService: SendApiService, + private avatarService: AvatarService, private logoutCallback: (expired: boolean) => Promise, ) {} @@ -309,7 +311,7 @@ export class SyncService implements SyncServiceAbstraction { await this.cryptoService.setPrivateKey(response.privateKey); await this.cryptoService.setProviderKeys(response.providers); await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations); - await this.stateService.setAvatarColor(response.avatarColor); + await this.avatarService.setAvatarColor(response.avatarColor); await this.stateService.setSecurityStamp(response.securityStamp); await this.stateService.setEmailVerified(response.emailVerified); await this.stateService.setHasPremiumPersonally(response.premiumPersonally);