mirror of
https://github.com/bitwarden/browser
synced 2025-12-10 13:23:34 +00:00
Expand account service (#6622)
* Define account service observable responsibilities * Establish account service observables and update methods * Update Account Service observables from state service This is a temporary stop-gap to avoid needing to reroute all account activity and status changes through the account service. That can be done as part of the breakup of state service. * Add matchers for Observable emissions * Fix null active account * Test account service * Transition account status to account info * Remove unused matchers * Remove duplicate class * Replay active account for late subscriptions * Add factories for background services * Fix state service for web * Allow for optional messaging This is a temporary hack until the flow of account status can be reversed from state -> account to account -> state. The foreground account service will still logout, it's just the background one cannot send messages * Fix add account logic * Do not throw on recoverable errors It's possible that duplicate entries exist in `activeAccounts` exist in the wild. If we throw on adding a duplicate account this will cause applications to be unusable until duplicates are removed it is not necessary to throw since this is recoverable. with some potential loss in current account status * Add documentation to abstraction * Update libs/common/spec/utils.ts Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> * Fix justin's comment :fist-shake: --------- Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { mock, MockProxy } from "jest-mock-extended";
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
|
||||
|
||||
@@ -40,3 +41,40 @@ export function makeStaticByteArray(length: number, start = 0) {
|
||||
* Use to mock a return value of a static fromJSON method.
|
||||
*/
|
||||
export const mockFromJson = (stub: any) => (stub + "_fromJSON") as any;
|
||||
|
||||
/**
|
||||
* Tracks the emissions of the given observable.
|
||||
*
|
||||
* Call this function before you expect any emissions and then use code that will cause the observable to emit values,
|
||||
* then assert after all expected emissions have occurred.
|
||||
* @param observable
|
||||
* @returns An array that will be populated with all emissions of the observable.
|
||||
*/
|
||||
export function trackEmissions<T>(observable: Observable<T>): T[] {
|
||||
const emissions: T[] = [];
|
||||
observable.subscribe((value) => {
|
||||
switch (value) {
|
||||
case undefined:
|
||||
case null:
|
||||
emissions.push(value);
|
||||
return;
|
||||
default:
|
||||
// process by type
|
||||
break;
|
||||
}
|
||||
|
||||
switch (typeof value) {
|
||||
case "string":
|
||||
case "number":
|
||||
case "boolean":
|
||||
emissions.push(value);
|
||||
break;
|
||||
case "object":
|
||||
emissions.push({ ...value });
|
||||
break;
|
||||
default:
|
||||
emissions.push(JSON.parse(JSON.stringify(value)));
|
||||
}
|
||||
});
|
||||
return emissions;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,50 @@
|
||||
export abstract class AccountService {}
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
import { UserId } from "../../types/guid";
|
||||
import { AuthenticationStatus } from "../enums/authentication-status";
|
||||
|
||||
export type AccountInfo = {
|
||||
status: AuthenticationStatus;
|
||||
email: string;
|
||||
name: string | undefined;
|
||||
};
|
||||
|
||||
export abstract class AccountService {
|
||||
accounts$: Observable<Record<UserId, AccountInfo>>;
|
||||
activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>;
|
||||
accountLock$: Observable<UserId>;
|
||||
accountLogout$: Observable<UserId>;
|
||||
/**
|
||||
* Updates the `accounts$` observable with the new account data.
|
||||
* @param userId
|
||||
* @param accountData
|
||||
*/
|
||||
abstract addAccount(userId: UserId, accountData: AccountInfo): void;
|
||||
/**
|
||||
* updates the `accounts$` observable with the new preferred name for the account.
|
||||
* @param userId
|
||||
* @param name
|
||||
*/
|
||||
abstract setAccountName(userId: UserId, name: string): void;
|
||||
/**
|
||||
* updates the `accounts$` observable with the new email for the account.
|
||||
* @param userId
|
||||
* @param email
|
||||
*/
|
||||
abstract setAccountEmail(userId: UserId, email: string): void;
|
||||
/**
|
||||
* Updates the `accounts$` observable with the new account status.
|
||||
* Also emits the `accountLock$` or `accountLogout$` observable if the status is `Locked` or `LoggedOut` respectively.
|
||||
* @param userId
|
||||
* @param status
|
||||
*/
|
||||
abstract setAccountStatus(userId: UserId, status: AuthenticationStatus): void;
|
||||
/**
|
||||
* Updates the `activeAccount$` observable with the new active account.
|
||||
* @param userId
|
||||
*/
|
||||
abstract switchAccount(userId: UserId): void;
|
||||
}
|
||||
|
||||
export abstract class InternalAccountService extends AccountService {
|
||||
abstract delete(): void;
|
||||
|
||||
181
libs/common/src/auth/services/account.service.spec.ts
Normal file
181
libs/common/src/auth/services/account.service.spec.ts
Normal file
@@ -0,0 +1,181 @@
|
||||
import { MockProxy, mock } from "jest-mock-extended";
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { trackEmissions } from "../../../spec/utils";
|
||||
import { LogService } from "../../platform/abstractions/log.service";
|
||||
import { MessagingService } from "../../platform/abstractions/messaging.service";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { AccountInfo } from "../abstractions/account.service";
|
||||
import { AuthenticationStatus } from "../enums/authentication-status";
|
||||
|
||||
import { AccountServiceImplementation } from "./account.service";
|
||||
|
||||
describe("accountService", () => {
|
||||
let messagingService: MockProxy<MessagingService>;
|
||||
let logService: MockProxy<LogService>;
|
||||
let sut: AccountServiceImplementation;
|
||||
const userId = "userId" as UserId;
|
||||
function userInfo(status: AuthenticationStatus): AccountInfo {
|
||||
return { status, email: "email", name: "name" };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
messagingService = mock<MessagingService>();
|
||||
logService = mock<LogService>();
|
||||
|
||||
sut = new AccountServiceImplementation(messagingService, logService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
describe("activeAccount$", () => {
|
||||
it("should emit undefined if no account is active", () => {
|
||||
const emissions = trackEmissions(sut.activeAccount$);
|
||||
|
||||
expect(emissions).toEqual([undefined]);
|
||||
});
|
||||
|
||||
it("should emit the active account and status", async () => {
|
||||
const emissions = trackEmissions(sut.activeAccount$);
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
sut.switchAccount(userId);
|
||||
|
||||
expect(emissions).toEqual([
|
||||
undefined, // initial value
|
||||
{ id: userId, ...userInfo(AuthenticationStatus.Unlocked) },
|
||||
]);
|
||||
});
|
||||
|
||||
it("should remember the last emitted value", async () => {
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
sut.switchAccount(userId);
|
||||
|
||||
expect(await firstValueFrom(sut.activeAccount$)).toEqual({
|
||||
id: userId,
|
||||
...userInfo(AuthenticationStatus.Unlocked),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("addAccount", () => {
|
||||
it("should emit the new account", () => {
|
||||
const emissions = trackEmissions(sut.accounts$);
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
|
||||
expect(emissions).toEqual([
|
||||
{}, // initial value
|
||||
{ [userId]: userInfo(AuthenticationStatus.Unlocked) },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setAccountName", () => {
|
||||
beforeEach(() => {
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
});
|
||||
|
||||
it("should emit the updated account", () => {
|
||||
const emissions = trackEmissions(sut.accounts$);
|
||||
sut.setAccountName(userId, "new name");
|
||||
|
||||
expect(emissions).toEqual([
|
||||
{ [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "name" } },
|
||||
{ [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "new name" } },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setAccountEmail", () => {
|
||||
beforeEach(() => {
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
});
|
||||
|
||||
it("should emit the updated account", () => {
|
||||
const emissions = trackEmissions(sut.accounts$);
|
||||
sut.setAccountEmail(userId, "new email");
|
||||
|
||||
expect(emissions).toEqual([
|
||||
{ [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "email" } },
|
||||
{ [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "new email" } },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setAccountStatus", () => {
|
||||
beforeEach(() => {
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
});
|
||||
|
||||
it("should not emit if the status is the same", async () => {
|
||||
const emissions = trackEmissions(sut.accounts$);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.Unlocked);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.Unlocked);
|
||||
|
||||
expect(emissions).toEqual([{ userId: userInfo(AuthenticationStatus.Unlocked) }]);
|
||||
});
|
||||
|
||||
it("should maintain an accounts cache", async () => {
|
||||
expect(await firstValueFrom(sut.accounts$)).toEqual({
|
||||
[userId]: userInfo(AuthenticationStatus.Unlocked),
|
||||
});
|
||||
});
|
||||
|
||||
it("should emit if the status is different", () => {
|
||||
const emissions = trackEmissions(sut.accounts$);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.Locked);
|
||||
|
||||
expect(emissions).toEqual([
|
||||
{ userId: userInfo(AuthenticationStatus.Unlocked) }, // initial value from beforeEach
|
||||
{ userId: userInfo(AuthenticationStatus.Locked) },
|
||||
]);
|
||||
});
|
||||
|
||||
it("should emit logout if the status is logged out", () => {
|
||||
const emissions = trackEmissions(sut.accountLogout$);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.LoggedOut);
|
||||
|
||||
expect(emissions).toEqual([userId]);
|
||||
});
|
||||
|
||||
it("should emit lock if the status is locked", () => {
|
||||
const emissions = trackEmissions(sut.accountLock$);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.Locked);
|
||||
|
||||
expect(emissions).toEqual([userId]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("switchAccount", () => {
|
||||
let emissions: { id: string; status: AuthenticationStatus }[];
|
||||
|
||||
beforeEach(() => {
|
||||
emissions = [];
|
||||
sut.activeAccount$.subscribe((value) => emissions.push(value));
|
||||
});
|
||||
|
||||
it("should emit undefined if no account is provided", () => {
|
||||
sut.switchAccount(undefined);
|
||||
|
||||
expect(emissions).toEqual([undefined]);
|
||||
});
|
||||
|
||||
it("should emit the active account and status", () => {
|
||||
sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked));
|
||||
sut.switchAccount(userId);
|
||||
sut.setAccountStatus(userId, AuthenticationStatus.Locked);
|
||||
sut.switchAccount(undefined);
|
||||
sut.switchAccount(undefined);
|
||||
expect(emissions).toEqual([
|
||||
undefined, // initial value
|
||||
{ id: userId, ...userInfo(AuthenticationStatus.Unlocked) },
|
||||
{ id: userId, ...userInfo(AuthenticationStatus.Locked) },
|
||||
]);
|
||||
});
|
||||
|
||||
it("should throw if switched to an unknown account", () => {
|
||||
expect(() => sut.switchAccount(userId)).toThrowError("Account does not exist");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,16 +1,93 @@
|
||||
import { InternalAccountService } from "../../auth/abstractions/account.service";
|
||||
import {
|
||||
BehaviorSubject,
|
||||
Subject,
|
||||
combineLatestWith,
|
||||
map,
|
||||
distinctUntilChanged,
|
||||
shareReplay,
|
||||
} from "rxjs";
|
||||
|
||||
import { AccountInfo, InternalAccountService } from "../../auth/abstractions/account.service";
|
||||
import { LogService } from "../../platform/abstractions/log.service";
|
||||
import { MessagingService } from "../../platform/abstractions/messaging.service";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { AuthenticationStatus } from "../enums/authentication-status";
|
||||
|
||||
export class AccountServiceImplementation implements InternalAccountService {
|
||||
private accounts = new BehaviorSubject<Record<UserId, AccountInfo>>({});
|
||||
private activeAccountId = new BehaviorSubject<UserId | undefined>(undefined);
|
||||
private lock = new Subject<UserId>();
|
||||
private logout = new Subject<UserId>();
|
||||
|
||||
accounts$ = this.accounts.asObservable();
|
||||
activeAccount$ = this.activeAccountId.pipe(
|
||||
combineLatestWith(this.accounts$),
|
||||
map(([id, accounts]) => (id ? { id, ...accounts[id] } : undefined)),
|
||||
distinctUntilChanged(),
|
||||
shareReplay({ bufferSize: 1, refCount: false })
|
||||
);
|
||||
accountLock$ = this.lock.asObservable();
|
||||
accountLogout$ = this.logout.asObservable();
|
||||
constructor(private messagingService: MessagingService, private logService: LogService) {}
|
||||
|
||||
addAccount(userId: UserId, accountData: AccountInfo): void {
|
||||
this.accounts.value[userId] = accountData;
|
||||
this.accounts.next(this.accounts.value);
|
||||
}
|
||||
|
||||
setAccountName(userId: UserId, name: string): void {
|
||||
this.setAccountInfo(userId, { ...this.accounts.value[userId], name });
|
||||
}
|
||||
|
||||
setAccountEmail(userId: UserId, email: string): void {
|
||||
this.setAccountInfo(userId, { ...this.accounts.value[userId], email });
|
||||
}
|
||||
|
||||
setAccountStatus(userId: UserId, status: AuthenticationStatus): void {
|
||||
this.setAccountInfo(userId, { ...this.accounts.value[userId], status });
|
||||
|
||||
if (status === AuthenticationStatus.LoggedOut) {
|
||||
this.logout.next(userId);
|
||||
} else if (status === AuthenticationStatus.Locked) {
|
||||
this.lock.next(userId);
|
||||
}
|
||||
}
|
||||
|
||||
switchAccount(userId: UserId) {
|
||||
if (userId == null) {
|
||||
// indicates no account is active
|
||||
this.activeAccountId.next(undefined);
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.accounts.value[userId] == null) {
|
||||
throw new Error("Account does not exist");
|
||||
}
|
||||
this.activeAccountId.next(userId);
|
||||
}
|
||||
|
||||
// TODO: update to use our own account status settings. Requires inverting direction of state service accounts flow
|
||||
async delete(): Promise<void> {
|
||||
try {
|
||||
this.messagingService.send("logout");
|
||||
this.messagingService?.send("logout");
|
||||
} catch (e) {
|
||||
this.logService.error(e);
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
private setAccountInfo(userId: UserId, accountInfo: AccountInfo) {
|
||||
if (this.accounts.value[userId] == null) {
|
||||
throw new Error("Account does not exist");
|
||||
}
|
||||
|
||||
// Avoid unnecessary updates
|
||||
// TODO: Faster comparison, maybe include a hash on the objects?
|
||||
if (JSON.stringify(this.accounts.value[userId]) === JSON.stringify(accountInfo)) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.accounts.value[userId] = accountInfo;
|
||||
this.accounts.next(this.accounts.value);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,8 @@ import { OrganizationData } from "../../admin-console/models/data/organization.d
|
||||
import { PolicyData } from "../../admin-console/models/data/policy.data";
|
||||
import { ProviderData } from "../../admin-console/models/data/provider.data";
|
||||
import { Policy } from "../../admin-console/models/domain/policy";
|
||||
import { AccountService } from "../../auth/abstractions/account.service";
|
||||
import { AuthenticationStatus } from "../../auth/enums/authentication-status";
|
||||
import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable";
|
||||
import { EnvironmentUrls } from "../../auth/models/domain/environment-urls";
|
||||
import { ForceResetPasswordReason } from "../../auth/models/domain/force-reset-password-reason";
|
||||
@@ -27,6 +29,7 @@ import { GeneratedPasswordHistory, PasswordGeneratorOptions } from "../../tools/
|
||||
import { UsernameGeneratorOptions } from "../../tools/generator/username";
|
||||
import { SendData } from "../../tools/send/models/data/send.data";
|
||||
import { SendView } from "../../tools/send/models/view/send.view";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { CipherData } from "../../vault/models/data/cipher.data";
|
||||
import { CollectionData } from "../../vault/models/data/collection.data";
|
||||
import { FolderData } from "../../vault/models/data/folder.data";
|
||||
@@ -110,6 +113,7 @@ export class StateService<
|
||||
protected memoryStorageService: AbstractMemoryStorageService,
|
||||
protected logService: LogService,
|
||||
protected stateFactory: StateFactory<TGlobalState, TAccount>,
|
||||
protected accountService: AccountService,
|
||||
protected useAccountCache: boolean = true
|
||||
) {
|
||||
// If the account gets changed, verify the new account is unlocked
|
||||
@@ -168,6 +172,8 @@ export class StateService<
|
||||
}
|
||||
await this.pushAccounts();
|
||||
this.activeAccountSubject.next(state.activeUserId);
|
||||
// TODO: Temporary update to avoid routing all account status changes through account service for now.
|
||||
this.accountService.switchAccount(state.activeUserId as UserId);
|
||||
|
||||
return state;
|
||||
});
|
||||
@@ -184,6 +190,12 @@ export class StateService<
|
||||
state.accounts[userId] = this.createAccount();
|
||||
const diskAccount = await this.getAccountFromDisk({ userId: userId });
|
||||
state.accounts[userId].profile = diskAccount.profile;
|
||||
// TODO: Temporary update to avoid routing all account status changes through account service for now.
|
||||
this.accountService.addAccount(userId as UserId, {
|
||||
status: AuthenticationStatus.Locked,
|
||||
name: diskAccount.profile.name,
|
||||
email: diskAccount.profile.email,
|
||||
});
|
||||
return state;
|
||||
});
|
||||
}
|
||||
@@ -198,6 +210,12 @@ export class StateService<
|
||||
});
|
||||
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.
|
||||
this.accountService.addAccount(account.profile.userId as UserId, {
|
||||
status: AuthenticationStatus.Locked,
|
||||
name: account.profile.name,
|
||||
email: account.profile.email,
|
||||
});
|
||||
await this.setActiveUser(account.profile.userId);
|
||||
this.activeAccountSubject.next(account.profile.userId);
|
||||
}
|
||||
@@ -208,6 +226,9 @@ export class StateService<
|
||||
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.
|
||||
this.accountService.switchAccount(userId as UserId);
|
||||
|
||||
return state;
|
||||
});
|
||||
|
||||
@@ -548,6 +569,9 @@ export class StateService<
|
||||
this.reconcileOptions(options, await this.defaultInMemoryOptions())
|
||||
);
|
||||
|
||||
const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked;
|
||||
this.accountService.setAccountStatus(options.userId as UserId, nextStatus);
|
||||
|
||||
if (options.userId == this.activeAccountSubject.getValue()) {
|
||||
const nextValue = value != null;
|
||||
|
||||
@@ -581,6 +605,9 @@ export class StateService<
|
||||
this.reconcileOptions(options, await this.defaultInMemoryOptions())
|
||||
);
|
||||
|
||||
const nextStatus = value != null ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked;
|
||||
this.accountService.setAccountStatus(options.userId as UserId, nextStatus);
|
||||
|
||||
if (options?.userId == this.activeAccountSubject.getValue()) {
|
||||
const nextValue = value != null;
|
||||
|
||||
@@ -3062,7 +3089,6 @@ export class StateService<
|
||||
this.reconcileOptions({ userId: account.profile.userId }, await this.defaultOnDiskOptions())
|
||||
);
|
||||
}
|
||||
//
|
||||
|
||||
protected async pushAccounts(): Promise<void> {
|
||||
await this.pruneInMemoryAccounts();
|
||||
@@ -3180,6 +3206,8 @@ export class StateService<
|
||||
|
||||
return state;
|
||||
});
|
||||
// TODO: Invert this logic, we should remove accounts based on logged out emit
|
||||
this.accountService.setAccountStatus(userId as UserId, AuthenticationStatus.LoggedOut);
|
||||
}
|
||||
|
||||
protected async pruneInMemoryAccounts() {
|
||||
|
||||
5
libs/common/src/types/guid.d.ts
vendored
Normal file
5
libs/common/src/types/guid.d.ts
vendored
Normal file
@@ -0,0 +1,5 @@
|
||||
import { Opaque } from "type-fest";
|
||||
|
||||
type Guid = Opaque<string, "Guid">;
|
||||
|
||||
type UserId = Opaque<string, "UserId">;
|
||||
Reference in New Issue
Block a user