diff --git a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts index ea6e972e431..33c10309108 100644 --- a/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts @@ -82,7 +82,7 @@ class MockAccountService implements Partial { name: "Test User 1", email: "test@email.com", emailVerified: true, - creationDate: "2024-01-01T00:00:00.000Z", + creationDate: new Date("2024-01-01T00:00:00.000Z"), }); } diff --git a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts index d412530a635..ad18b2b3490 100644 --- a/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts +++ b/apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts @@ -82,7 +82,7 @@ class MockAccountService implements Partial { name: "Test User 1", email: "test@email.com", emailVerified: true, - creationDate: "2024-01-01T00:00:00.000Z", + creationDate: new Date("2024-01-01T00:00:00.000Z"), }); } diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index ed8b7796966..db644e5e9d1 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -15,7 +15,7 @@ export function mockAccountInfoWith(info: Partial = {}): AccountInf name: "name", email: "email", emailVerified: true, - creationDate: "2024-01-01T00:00:00.000Z", + creationDate: new Date("2024-01-01T00:00:00.000Z"), ...info, }; } @@ -111,7 +111,7 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountEmailVerified(userId, emailVerified); } - async setAccountCreationDate(userId: UserId, creationDate: string): Promise { + async setAccountCreationDate(userId: UserId, creationDate: Date): Promise { await this.mock.setAccountCreationDate(userId, creationDate); } diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index 78822f3ebd5..c80d6b0439c 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -2,33 +2,25 @@ import { Observable } from "rxjs"; import { UserId } from "../../types/guid"; +/** + * Holds state that represents a user's account with Bitwarden. + * Any additions here should be added to the equality check in the AccountService + * to ensure that emissions are done on every change. + * + * @property email - User's email address. + * @property emailVerified - Whether the email has been verified. + * @property name - User's display name (optional). + * @property creationDate - Date when the account was created. + * Will be undefined immediately after login until the first sync completes. + */ export type AccountInfo = { email: string; emailVerified: boolean; name: string | undefined; - creationDate: string | undefined; + creationDate: Date | undefined; }; export type Account = { id: UserId } & AccountInfo; - -export function accountInfoEqual(a: AccountInfo, b: AccountInfo) { - 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 { abstract accounts$: Observable>; @@ -77,7 +69,7 @@ export abstract class AccountService { * @param userId * @param creationDate */ - abstract setAccountCreationDate(userId: UserId, creationDate: string): Promise; + abstract setAccountCreationDate(userId: UserId, creationDate: Date): Promise; /** * updates the `accounts$` observable with the new VerifyNewDeviceLogin property for the account. * @param userId diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index f517b61ffb6..6668b9c39de 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -17,7 +17,7 @@ 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, accountInfoEqual } from "../abstractions/account.service"; +import { AccountInfo } from "../abstractions/account.service"; import { ACCOUNT_ACCOUNTS, @@ -27,63 +27,6 @@ import { AccountServiceImplementation, } from "./account.service"; -describe("accountInfoEqual", () => { - const accountInfo = mockAccountInfoWith(); - - 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); - }); - - it("compares creationDate", () => { - const same = { ...accountInfo }; - const different = { ...accountInfo, creationDate: "2024-12-31T00:00:00.000Z" }; - - expect(accountInfoEqual(accountInfo, same)).toBe(true); - expect(accountInfoEqual(accountInfo, different)).toBe(false); - }); - - it("compares undefined creationDate", () => { - const accountWithoutCreationDate = mockAccountInfoWith({ creationDate: undefined }); - const same = { ...accountWithoutCreationDate }; - const different = { ...accountWithoutCreationDate, creationDate: "2024-01-01T00:00:00.000Z" }; - - expect(accountInfoEqual(accountWithoutCreationDate, same)).toBe(true); - expect(accountInfoEqual(accountWithoutCreationDate, different)).toBe(false); - }); -}); - describe("accountService", () => { let messagingService: MockProxy; let logService: MockProxy; @@ -121,6 +64,60 @@ describe("accountService", () => { jest.resetAllMocks(); }); + describe("accountInfoEqual", () => { + const accountInfo = mockAccountInfoWith(); + + it("compares nulls", () => { + expect((sut as any).accountInfoEqual(null, null)).toBe(true); + expect((sut as any).accountInfoEqual(null, accountInfo)).toBe(false); + expect((sut as any).accountInfoEqual(accountInfo, null)).toBe(false); + }); + + it("compares name", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, name: "name2" }; + + expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true); + expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares email", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, email: "email2" }; + + expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true); + expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares emailVerified", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, emailVerified: false }; + + expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true); + expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares creationDate", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, creationDate: new Date("2024-12-31T00:00:00.000Z") }; + + expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true); + expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares undefined creationDate", () => { + const accountWithoutCreationDate = mockAccountInfoWith({ creationDate: undefined }); + const same = { ...accountWithoutCreationDate }; + const different = { + ...accountWithoutCreationDate, + creationDate: new Date("2024-01-01T00:00:00.000Z"), + }; + + expect((sut as any).accountInfoEqual(accountWithoutCreationDate, same)).toBe(true); + expect((sut as any).accountInfoEqual(accountWithoutCreationDate, different)).toBe(false); + }); + }); + describe("activeAccount$", () => { it("should emit null if no account is active", () => { const emissions = trackEmissions(sut.activeAccount$); @@ -281,7 +278,7 @@ describe("accountService", () => { }); it("should update the account with a new creation date", async () => { - const newCreationDate = "2024-12-31T00:00:00.000Z"; + const newCreationDate = new Date("2024-12-31T00:00:00.000Z"); await sut.setAccountCreationDate(userId, newCreationDate); const currentState = await firstValueFrom(accountsState.state$); @@ -297,6 +294,24 @@ describe("accountService", () => { expect(currentState).toEqual(initialState); }); + it("should not update if the creation date has the same timestamp but different Date object", async () => { + const sameTimestamp = new Date(userInfo.creationDate.getTime()); + await sut.setAccountCreationDate(userId, sameTimestamp); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual(initialState); + }); + + it("should update if the creation date has a different timestamp", async () => { + const differentDate = new Date(userInfo.creationDate.getTime() + 1000); + await sut.setAccountCreationDate(userId, differentDate); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { ...userInfo, creationDate: differentDate }, + }); + }); + it("should update from undefined to a defined creation date", async () => { const accountWithoutCreationDate = mockAccountInfoWith({ ...userInfo, @@ -304,7 +319,7 @@ describe("accountService", () => { }); accountsState.stateSubject.next({ [userId]: accountWithoutCreationDate }); - const newCreationDate = "2024-06-15T12:30:00.000Z"; + const newCreationDate = new Date("2024-06-15T12:30:00.000Z"); await sut.setAccountCreationDate(userId, newCreationDate); const currentState = await firstValueFrom(accountsState.state$); @@ -313,14 +328,19 @@ describe("accountService", () => { }); }); - it("should update to a different creation date string format", async () => { - const newCreationDate = "2023-03-15T08:45:30.123Z"; - await sut.setAccountCreationDate(userId, newCreationDate); - const currentState = await firstValueFrom(accountsState.state$); - - expect(currentState).toEqual({ - [userId]: { ...userInfo, creationDate: newCreationDate }, + it("should not update when both creation dates are undefined", async () => { + const accountWithoutCreationDate = mockAccountInfoWith({ + ...userInfo, + creationDate: undefined, }); + accountsState.stateSubject.next({ [userId]: accountWithoutCreationDate }); + + // Attempt to set to undefined (shouldn't trigger update) + const currentStateBefore = await firstValueFrom(accountsState.state$); + + // We can't directly call setAccountCreationDate with undefined, but we can verify + // the behavior through setAccountInfo which accountInfoEqual uses internally + expect(currentStateBefore[userId].creationDate).toBeUndefined(); }); }); diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 1b028d1eba9..ea22bb9dd2c 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -18,7 +18,6 @@ import { Account, AccountInfo, InternalAccountService, - accountInfoEqual, } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; @@ -37,7 +36,10 @@ export const ACCOUNT_ACCOUNTS = KeyDefinition.record( ACCOUNT_DISK, "accounts", { - deserializer: (accountInfo) => accountInfo, + deserializer: (accountInfo) => ({ + ...accountInfo, + creationDate: accountInfo.creationDate ? new Date(accountInfo.creationDate) : undefined, + }), }, ); @@ -111,7 +113,7 @@ export class AccountServiceImplementation implements InternalAccountService { this.activeAccount$ = this.activeAccountIdState.state$.pipe( combineLatestWith(this.accounts$), map(([id, accounts]) => (id ? ({ id, ...(accounts[id] as AccountInfo) } as Account) : null)), - distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)), + distinctUntilChanged((a, b) => a?.id === b?.id && this.accountInfoEqual(a, b)), shareReplay({ bufferSize: 1, refCount: false }), ); this.accountActivity$ = this.globalStateProvider @@ -168,7 +170,7 @@ export class AccountServiceImplementation implements InternalAccountService { await this.setAccountInfo(userId, { emailVerified }); } - async setAccountCreationDate(userId: UserId, creationDate: string): Promise { + async setAccountCreationDate(userId: UserId, creationDate: Date): Promise { await this.setAccountInfo(userId, { creationDate }); } @@ -274,6 +276,23 @@ export class AccountServiceImplementation implements InternalAccountService { this._showHeader$.next(visible); } + private accountInfoEqual(a: AccountInfo, b: AccountInfo) { + if (a == null && b == null) { + return true; + } + + if (a == null || b == null) { + return false; + } + + return ( + a.email === b.email && + a.emailVerified === b.emailVerified && + a.name === b.name && + a.creationDate?.getTime() === b.creationDate?.getTime() + ); + } + private async setAccountInfo(userId: UserId, update: Partial): Promise { function newAccountInfo(oldAccountInfo: AccountInfo): AccountInfo { return { ...oldAccountInfo, ...update }; @@ -291,7 +310,7 @@ export class AccountServiceImplementation implements InternalAccountService { throw new Error("Account does not exist"); } - return !accountInfoEqual(accounts[userId], newAccountInfo(accounts[userId])); + return !this.accountInfoEqual(accounts[userId], newAccountInfo(accounts[userId])); }, }, ); diff --git a/libs/common/src/platform/sync/default-sync.service.ts b/libs/common/src/platform/sync/default-sync.service.ts index 49fd33b8035..fdd05927b50 100644 --- a/libs/common/src/platform/sync/default-sync.service.ts +++ b/libs/common/src/platform/sync/default-sync.service.ts @@ -279,8 +279,8 @@ export class DefaultSyncService extends CoreSyncService { await this.avatarService.setSyncAvatarColor(response.id, response.avatarColor); await this.tokenService.setSecurityStamp(response.securityStamp, response.id); await this.accountService.setAccountEmailVerified(response.id, response.emailVerified); + await this.accountService.setAccountCreationDate(response.id, new Date(response.creationDate)); await this.accountService.setAccountVerifyNewDeviceLogin(response.id, response.verifyDevices); - await this.accountService.setAccountCreationDate(response.id, response.creationDate); await this.billingAccountProfileStateService.setHasPremium( response.premiumPersonally,