Remove state service active account Observables.

This commit is contained in:
Matt Gibson 2024-03-26 15:19:40 -04:00
parent 08399ea86e
commit 7cfa7d8d55
No known key found for this signature in database
GPG Key ID: 963EE038B0581878
14 changed files with 59 additions and 66 deletions

View File

@ -1,7 +1,8 @@
import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core";
import { NavigationEnd, Router, RouterOutlet } from "@angular/router";
import { filter, concatMap, Subject, takeUntil, firstValueFrom, tap, map } from "rxjs";
import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@ -46,6 +47,7 @@ export class AppComponent implements OnInit, OnDestroy {
private dialogService: DialogService,
private messageListener: MessageListener,
private toastService: ToastService,
private accountService: AccountService,
) {}
async ngOnInit() {
@ -53,14 +55,13 @@ export class AppComponent implements OnInit, OnDestroy {
// Clear them aggressively to make sure this doesn't occur
await this.clearComponentStates();
this.stateService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((userId) => {
this.activeUserId = userId;
this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => {
this.activeUserId = account?.id;
});
this.authService.activeAccountStatus$
.pipe(
map((status) => status === AuthenticationStatus.Unlocked),
filter((unlocked) => unlocked),
filter((status) => status === AuthenticationStatus.Unlocked),
concatMap(async () => {
await this.recordActivity();
}),

View File

@ -154,8 +154,8 @@ export class AppComponent implements OnInit, OnDestroy {
) {}
ngOnInit() {
this.stateService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((userId) => {
this.activeUserId = userId;
this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => {
this.activeUserId = account?.id;
});
this.ngZone.runOutsideAngular(() => {

View File

@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core";
import { UntypedFormControl } from "@angular/forms";
import { Subscription } from "rxjs";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { SearchBarService, SearchBarState } from "./search-bar.service";
@ -18,7 +18,7 @@ export class SearchComponent implements OnInit, OnDestroy {
constructor(
private searchBarService: SearchBarService,
private stateService: StateService,
private accountService: AccountService,
) {
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
this.searchBarService.state$.subscribe((state) => {
@ -33,7 +33,7 @@ export class SearchComponent implements OnInit, OnDestroy {
ngOnInit() {
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
this.activeAccountSubscription = this.stateService.activeAccount$.subscribe((value) => {
this.activeAccountSubscription = this.accountService.activeAccount$.subscribe((_) => {
this.searchBarService.setSearchText("");
this.searchText.patchValue("");
});

View File

@ -49,7 +49,7 @@ describe("LockComponent", () => {
let component: LockComponent;
let fixture: ComponentFixture<LockComponent>;
let stateServiceMock: MockProxy<StateService>;
const biometricStateService = mock<BiometricStateService>();
let biometricStateService: MockProxy<BiometricStateService>;
let messagingServiceMock: MockProxy<MessagingService>;
let broadcasterServiceMock: MockProxy<BroadcasterService>;
let platformUtilsServiceMock: MockProxy<PlatformUtilsService>;
@ -61,7 +61,6 @@ describe("LockComponent", () => {
beforeEach(async () => {
stateServiceMock = mock<StateService>();
stateServiceMock.activeAccount$ = of(null);
messagingServiceMock = mock<MessagingService>();
broadcasterServiceMock = mock<BroadcasterService>();
@ -72,6 +71,7 @@ describe("LockComponent", () => {
mockMasterPasswordService = new FakeMasterPasswordService();
biometricStateService = mock();
biometricStateService.dismissedRequirePasswordOnStartCallout$ = of(false);
biometricStateService.promptAutomatically$ = of(false);
biometricStateService.promptCancelled$ = of(false);

View File

@ -58,7 +58,7 @@
[bitMenuTriggerFor]="accountMenu"
class="tw-border-0 tw-bg-transparent tw-p-0"
>
<dynamic-avatar [id]="account.userId" [text]="account | userName"></dynamic-avatar>
<dynamic-avatar [id]="account.id" [text]="account | userName"></dynamic-avatar>
</button>
<bit-menu #accountMenu>
@ -67,7 +67,7 @@
class="tw-flex tw-items-center tw-px-4 tw-py-1 tw-leading-tight tw-text-info"
appStopProp
>
<dynamic-avatar [id]="account.userId" [text]="account | userName"></dynamic-avatar>
<dynamic-avatar [id]="account.id" [text]="account | userName"></dynamic-avatar>
<div class="tw-ml-2 tw-block tw-overflow-hidden tw-whitespace-nowrap">
<span>{{ "loggedInAs" | i18n }}</span>
<small class="tw-block tw-overflow-hidden tw-whitespace-nowrap tw-text-muted">

View File

@ -1,16 +1,17 @@
import { Component, Input } from "@angular/core";
import { ActivatedRoute } from "@angular/router";
import { combineLatest, map, Observable } from "rxjs";
import { map, Observable } from "rxjs";
import { User } from "@bitwarden/angular/pipes/user-name.pipe";
import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service";
import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { AccountProfile } from "@bitwarden/common/platform/models/domain/account";
import { UserId } from "@bitwarden/common/types/guid";
@Component({
selector: "app-header",
@ -28,7 +29,7 @@ export class WebHeaderComponent {
@Input() icon: string;
protected routeData$: Observable<{ titleId: string }>;
protected account$: Observable<AccountProfile>;
protected account$: Observable<User & { id: UserId }>;
protected canLock$: Observable<boolean>;
protected selfHosted: boolean;
protected hostname = location.hostname;
@ -38,12 +39,12 @@ export class WebHeaderComponent {
constructor(
private route: ActivatedRoute,
private stateService: StateService,
private platformUtilsService: PlatformUtilsService,
private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
private messagingService: MessagingService,
protected unassignedItemsBannerService: UnassignedItemsBannerService,
private configService: ConfigService,
private accountService: AccountService,
) {
this.routeData$ = this.route.data.pipe(
map((params) => {
@ -55,14 +56,7 @@ export class WebHeaderComponent {
this.selfHosted = this.platformUtilsService.isSelfHost();
this.account$ = combineLatest([
this.stateService.activeAccount$,
this.stateService.accounts$,
]).pipe(
map(([activeAccount, accounts]) => {
return accounts[activeAccount]?.profile;
}),
);
this.account$ = this.accountService.activeAccount$;
this.canLock$ = this.vaultTimeoutSettingsService
.availableVaultTimeoutActions$()
.pipe(map((actions) => actions.includes(VaultTimeoutAction.Lock)));

View File

@ -55,7 +55,6 @@ export default {
{
provide: StateService,
useValue: {
activeAccount$: new BehaviorSubject("1").asObservable(),
accounts$: new BehaviorSubject({ "1": { profile: { name: "Foo" } } }).asObservable(),
async getShowFavicon() {
return true;

View File

@ -82,7 +82,7 @@ export class LockComponent implements OnInit, OnDestroy {
) {}
async ngOnInit() {
this.stateService.activeAccount$
this.accountService.activeAccount$
.pipe(
concatMap(async () => {
await this.load();

View File

@ -1,6 +1,6 @@
import { Pipe, PipeTransform } from "@angular/core";
interface User {
export interface User {
name?: string;
email?: string;
}

View File

@ -27,7 +27,6 @@ export type InitOptions = {
export abstract class StateService<T extends Account = Account> {
accounts$: Observable<{ [userId: string]: T }>;
activeAccount$: Observable<string>;
addAccount: (account: T) => Promise<void>;
setActiveUser: (userId: string) => Promise<void>;

View File

@ -59,9 +59,6 @@ export class StateService<
protected accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({});
accounts$ = this.accountsSubject.asObservable();
protected activeAccountSubject = new BehaviorSubject<string | null>(null);
activeAccount$ = this.activeAccountSubject.asObservable();
private hasBeenInited = false;
protected isRecoveredSession = false;
@ -133,7 +130,6 @@ export class StateService<
state.activeUserId = storedActiveUser;
}
await this.pushAccounts();
this.activeAccountSubject.next(state.activeUserId);
return state;
});
@ -174,7 +170,6 @@ export class StateService<
await this.updateState(async (state) => {
state.activeUserId = userId;
await this.storageService.save(keys.activeUserId, userId);
this.activeAccountSubject.next(state.activeUserId);
return state;
});

View File

@ -1,4 +1,4 @@
import { firstValueFrom, timeout } from "rxjs";
import { firstValueFrom, map, timeout } from "rxjs";
import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service";
import { AccountService } from "../../auth/abstractions/account.service";
@ -65,7 +65,12 @@ export class SystemService implements SystemServiceAbstraction {
clearInterval(this.reloadInterval);
this.reloadInterval = null;
const currentUser = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500)));
const currentUser = await firstValueFrom(
this.accountService.activeAccount$.pipe(
map((a) => a?.id),
timeout(500),
),
);
// Replace current active user if they will be logged out on reload
if (currentUser != null) {
const timeoutAction = await firstValueFrom(

View File

@ -1,9 +1,10 @@
import { MockProxy, any, mock } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";
import { BehaviorSubject, of } from "rxjs";
import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service";
import { SearchService } from "../../abstractions/search.service";
import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service";
import { AccountInfo } from "../../auth/abstractions/account.service";
import { AuthService } from "../../auth/abstractions/auth.service";
import { AuthenticationStatus } from "../../auth/enums/authentication-status";
import { FakeMasterPasswordService } from "../../auth/services/master-password/fake-master-password.service";
@ -13,7 +14,6 @@ import { MessagingService } from "../../platform/abstractions/messaging.service"
import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service";
import { StateService } from "../../platform/abstractions/state.service";
import { Utils } from "../../platform/misc/utils";
import { Account } from "../../platform/models/domain/account";
import { StateEventRunnerService } from "../../platform/state";
import { UserId } from "../../types/guid";
import { CipherService } from "../../vault/abstractions/cipher.service";
@ -39,7 +39,6 @@ describe("VaultTimeoutService", () => {
let lockedCallback: jest.Mock<Promise<void>, [userId: string]>;
let loggedOutCallback: jest.Mock<Promise<void>, [expired: boolean, userId?: string]>;
let accountsSubject: BehaviorSubject<Record<string, Account>>;
let vaultTimeoutActionSubject: BehaviorSubject<VaultTimeoutAction>;
let availableVaultTimeoutActionsSubject: BehaviorSubject<VaultTimeoutAction[]>;
@ -65,10 +64,6 @@ describe("VaultTimeoutService", () => {
lockedCallback = jest.fn();
loggedOutCallback = jest.fn();
accountsSubject = new BehaviorSubject(null);
stateService.accounts$ = accountsSubject;
vaultTimeoutActionSubject = new BehaviorSubject(VaultTimeoutAction.Lock);
vaultTimeoutSettingsService.vaultTimeoutAction$.mockReturnValue(vaultTimeoutActionSubject);
@ -133,8 +128,7 @@ describe("VaultTimeoutService", () => {
stateService.getUserId.mockResolvedValue(globalSetups?.userId);
stateService.activeAccount$ = new BehaviorSubject<string>(globalSetups?.userId);
// Set desired user active and known users on accounts service : note the only thing that matters here is that the ID are set
if (globalSetups?.userId) {
accountService.activeAccountSubject.next({
id: globalSetups.userId as UserId,
@ -142,6 +136,18 @@ describe("VaultTimeoutService", () => {
name: null,
});
}
accountService.accounts$ = of(
Object.entries(accounts).reduce(
(agg, [id]) => {
agg[id] = {
email: "",
name: "",
};
return agg;
},
{} as Record<string, AccountInfo>,
),
);
platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false);
@ -158,16 +164,6 @@ describe("VaultTimeoutService", () => {
],
);
});
const accountsSubjectValue: Record<string, Account> = Object.keys(accounts).reduce(
(agg, key) => {
const newPartial: Record<string, unknown> = {};
newPartial[key] = null; // No values actually matter on this other than the key
return Object.assign(agg, newPartial);
},
{} as Record<string, Account>,
);
accountsSubject.next(accountsSubjectValue);
};
const expectUserToHaveLocked = (userId: string) => {

View File

@ -1,4 +1,4 @@
import { firstValueFrom, timeout } from "rxjs";
import { combineLatest, firstValueFrom, switchMap } from "rxjs";
import { SearchService } from "../../abstractions/search.service";
import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service";
@ -64,14 +64,18 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction {
// Get whether or not the view is open a single time so it can be compared for each user
const isViewOpen = await this.platformUtilsService.isViewOpen();
const activeUserId = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500)));
const accounts = await firstValueFrom(this.stateService.accounts$);
for (const userId in accounts) {
if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) {
await this.executeTimeoutAction(userId);
}
}
await firstValueFrom(
combineLatest([this.accountService.activeAccount$, this.accountService.accounts$]).pipe(
switchMap(async ([activeAccount, accounts]) => {
const activeUserId = activeAccount?.id;
for (const userId in accounts) {
if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) {
await this.executeTimeoutAction(userId);
}
}
}),
),
);
}
async lock(userId?: string): Promise<void> {