Browse Source

Merge 8bba053bee into 930cb9ab96

pull/17928/merge
Todd Martin 7 hours ago committed by GitHub
parent
commit
927e73b5d7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 2
      apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts
  2. 2
      apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts
  3. 4
      libs/common/spec/fake-account-service.ts
  4. 34
      libs/common/src/auth/abstractions/account.service.ts
  5. 100
      libs/common/src/auth/services/account.service.spec.ts
  6. 29
      libs/common/src/auth/services/account.service.ts
  7. 2
      libs/common/src/platform/sync/default-sync.service.ts

2
apps/web/src/app/layouts/product-switcher/navigation-switcher/navigation-switcher.stories.ts

@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> { @@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> {
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"),
});
}

2
apps/web/src/app/layouts/product-switcher/product-switcher.stories.ts

@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> { @@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> {
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"),
});
}

4
libs/common/spec/fake-account-service.ts

@ -15,7 +15,7 @@ export function mockAccountInfoWith(info: Partial<AccountInfo> = {}): AccountInf @@ -15,7 +15,7 @@ export function mockAccountInfoWith(info: Partial<AccountInfo> = {}): 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 { @@ -111,7 +111,7 @@ export class FakeAccountService implements AccountService {
await this.mock.setAccountEmailVerified(userId, emailVerified);
}
async setAccountCreationDate(userId: UserId, creationDate: string): Promise<void> {
async setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void> {
await this.mock.setAccountCreationDate(userId, creationDate);
}

34
libs/common/src/auth/abstractions/account.service.ts

@ -2,33 +2,25 @@ import { Observable } from "rxjs"; @@ -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<keyof AccountInfo>;
for (const key of keys) {
if (a[key] !== b[key]) {
return false;
}
}
return true;
}
export abstract class AccountService {
abstract accounts$: Observable<Record<UserId, AccountInfo>>;
@ -77,7 +69,7 @@ export abstract class AccountService { @@ -77,7 +69,7 @@ export abstract class AccountService {
* @param userId
* @param creationDate
*/
abstract setAccountCreationDate(userId: UserId, creationDate: string): Promise<void>;
abstract setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void>;
/**
* updates the `accounts$` observable with the new VerifyNewDeviceLogin property for the account.
* @param userId

100
libs/common/src/auth/services/account.service.spec.ts

@ -17,7 +17,7 @@ import { LogService } from "../../platform/abstractions/log.service"; @@ -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 { @@ -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<MessagingService>;
let logService: MockProxy<LogService>;
@ -281,7 +224,7 @@ describe("accountService", () => { @@ -281,7 +224,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 +240,24 @@ describe("accountService", () => { @@ -297,6 +240,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 +265,7 @@ describe("accountService", () => { @@ -304,7 +265,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 +274,19 @@ describe("accountService", () => { @@ -313,14 +274,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();
});
});

29
libs/common/src/auth/services/account.service.ts

@ -18,7 +18,6 @@ import { @@ -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<AccountInfo, UserId>( @@ -37,7 +36,10 @@ export const ACCOUNT_ACCOUNTS = KeyDefinition.record<AccountInfo, UserId>(
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 { @@ -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 { @@ -168,7 +170,7 @@ export class AccountServiceImplementation implements InternalAccountService {
await this.setAccountInfo(userId, { emailVerified });
}
async setAccountCreationDate(userId: UserId, creationDate: string): Promise<void> {
async setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void> {
await this.setAccountInfo(userId, { creationDate });
}
@ -274,6 +276,23 @@ export class AccountServiceImplementation implements InternalAccountService { @@ -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<AccountInfo>): Promise<void> {
function newAccountInfo(oldAccountInfo: AccountInfo): AccountInfo {
return { ...oldAccountInfo, ...update };
@ -291,7 +310,7 @@ export class AccountServiceImplementation implements InternalAccountService { @@ -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]));
},
},
);

2
libs/common/src/platform/sync/default-sync.service.ts

@ -271,8 +271,8 @@ export class DefaultSyncService extends CoreSyncService { @@ -271,8 +271,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,

Loading…
Cancel
Save