[EC-381] Transition settings service into providing observables (#3253)

* [EC-381] Deleted unused method clearCache from Settings Service

* [EC-381] Marked settings methods as obsolete on State service

* [EC-381] Using observables on settings service

* [EC-381] Added unit tests for Settings service

* [EC-381] Checking userId on clear

* [EC-381] Updated references to StateService activeAccountUnlocked$

* [EC-381] Updated getEquivalentDomains to return observable

* [EC-381] Updated settings service to user concatMap on activeAccountUnlocked$

* [EC-381] Renamed getEquivalentDomains to equivalentDomains

* [EC-381] Completing Behaviors on settings.service tests

* [EC-381] Removed unused settingsPrefix from settings service

* [EC-381] Removed equivalentDomains from settings service and added type AccountSettingsSettings

* [EC-381] Updated settings service settings$ to not be nullable

* [EC-381] Settings default to {}
This commit is contained in:
Rui Tomé 2022-08-30 15:19:09 +01:00 committed by GitHub
parent 88a6541bd8
commit 595412c5fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 148 additions and 63 deletions

View File

@ -0,0 +1,64 @@
import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
import { BehaviorSubject, firstValueFrom } from "rxjs";
import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
import { ContainerService } from "@bitwarden/common/services/container.service";
import { SettingsService } from "@bitwarden/common/services/settings.service";
import { StateService } from "@bitwarden/common/services/state.service";
describe("SettingsService", () => {
let settingsService: SettingsService;
let cryptoService: SubstituteOf<CryptoService>;
let stateService: SubstituteOf<StateService>;
let activeAccount: BehaviorSubject<string>;
let activeAccountUnlocked: BehaviorSubject<boolean>;
beforeEach(() => {
cryptoService = Substitute.for();
stateService = Substitute.for();
activeAccount = new BehaviorSubject("123");
activeAccountUnlocked = new BehaviorSubject(true);
stateService.getSettings().resolves({ equivalentDomains: [["test"], ["domains"]] });
stateService.activeAccount$.returns(activeAccount);
stateService.activeAccountUnlocked$.returns(activeAccountUnlocked);
(window as any).bitwardenContainerService = new ContainerService(cryptoService);
settingsService = new SettingsService(stateService);
});
afterEach(() => {
activeAccount.complete();
activeAccountUnlocked.complete();
});
describe("getEquivalentDomains", () => {
it("returns value", async () => {
const result = await firstValueFrom(settingsService.settings$);
expect(result).toEqual({
equivalentDomains: [["test"], ["domains"]],
});
});
});
it("setEquivalentDomains", async () => {
await settingsService.setEquivalentDomains([["test2"], ["domains2"]]);
stateService.received(1).setSettings(Arg.any());
expect((await firstValueFrom(settingsService.settings$)).equivalentDomains).toEqual([
["test2"],
["domains2"],
]);
});
it("clear", async () => {
await settingsService.clear();
stateService.received(1).setSettings(Arg.any(), Arg.any());
expect(await firstValueFrom(settingsService.settings$)).toEqual({});
});
});

View File

@ -1,6 +1,10 @@
import { Observable } from "rxjs";
import { AccountSettingsSettings } from "../models/domain/account";
export abstract class SettingsService {
clearCache: () => Promise<void>;
getEquivalentDomains: () => Promise<any>;
settings$: Observable<AccountSettingsSettings>;
setEquivalentDomains: (equivalentDomains: string[][]) => Promise<any>;
clear: (userId?: string) => Promise<void>;
}

View File

@ -12,7 +12,7 @@ import { OrganizationData } from "../models/data/organizationData";
import { PolicyData } from "../models/data/policyData";
import { ProviderData } from "../models/data/providerData";
import { SendData } from "../models/data/sendData";
import { Account } from "../models/domain/account";
import { Account, AccountSettingsSettings } from "../models/domain/account";
import { EncString } from "../models/domain/encString";
import { EnvironmentUrls } from "../models/domain/environmentUrls";
import { GeneratedPasswordHistory } from "../models/domain/generatedPasswordHistory";
@ -286,8 +286,14 @@ export abstract class StateService<T extends Account = Account> {
setRememberedEmail: (value: string, options?: StorageOptions) => Promise<void>;
getSecurityStamp: (options?: StorageOptions) => Promise<string>;
setSecurityStamp: (value: string, options?: StorageOptions) => Promise<void>;
getSettings: (options?: StorageOptions) => Promise<any>;
setSettings: (value: string, options?: StorageOptions) => Promise<void>;
/**
* @deprecated Do not call this directly, use SettingsService
*/
getSettings: (options?: StorageOptions) => Promise<AccountSettingsSettings>;
/**
* @deprecated Do not call this directly, use SettingsService
*/
setSettings: (value: AccountSettingsSettings, options?: StorageOptions) => Promise<void>;
getSsoCodeVerifier: (options?: StorageOptions) => Promise<string>;
setSsoCodeVerifier: (value: string, options?: StorageOptions) => Promise<void>;
getSsoOrgIdentifier: (options?: StorageOptions) => Promise<string>;

View File

@ -137,11 +137,15 @@ export class AccountSettings {
generatorOptions?: any;
pinProtected?: EncryptionPair<string, EncString> = new EncryptionPair<string, EncString>();
protectedPin?: string;
settings?: any; // TODO: Merge whatever is going on here into the AccountSettings model properly
settings?: AccountSettingsSettings; // TODO: Merge whatever is going on here into the AccountSettings model properly
vaultTimeout?: number;
vaultTimeoutAction?: string = "lock";
}
export type AccountSettingsSettings = {
equivalentDomains?: { [id: string]: any };
};
export class AccountTokens {
accessToken?: string;
decodedToken?: any;

View File

@ -1,3 +1,5 @@
import { firstValueFrom } from "rxjs";
import { ApiService } from "../abstractions/api.service";
import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service";
import { CryptoService } from "../abstractions/crypto.service";
@ -13,6 +15,7 @@ import { UriMatchType } from "../enums/uriMatchType";
import { sequentialize } from "../misc/sequentialize";
import { Utils } from "../misc/utils";
import { CipherData } from "../models/data/cipherData";
import { AccountSettingsSettings } from "../models/domain/account";
import { Attachment } from "../models/domain/attachment";
import { Card } from "../models/domain/card";
import { Cipher } from "../models/domain/cipher";
@ -387,20 +390,22 @@ export class CipherService implements CipherServiceAbstraction {
const eqDomainsPromise =
domain == null
? Promise.resolve([])
: this.settingsService.getEquivalentDomains().then((eqDomains: any[][]) => {
let matches: any[] = [];
eqDomains.forEach((eqDomain) => {
if (eqDomain.length && eqDomain.indexOf(domain) >= 0) {
matches = matches.concat(eqDomain);
: firstValueFrom(this.settingsService.settings$).then(
(settings: AccountSettingsSettings) => {
let matches: any[] = [];
settings.equivalentDomains.forEach((eqDomain: any) => {
if (eqDomain.length && eqDomain.indexOf(domain) >= 0) {
matches = matches.concat(eqDomain);
}
});
if (!matches.length) {
matches.push(domain);
}
});
if (!matches.length) {
matches.push(domain);
return matches;
}
return matches;
});
);
const result = await Promise.all([eqDomainsPromise, this.getAllDecrypted()]);
const matchingDomains = result[0];

View File

@ -1,56 +1,50 @@
import { BehaviorSubject, concatMap } from "rxjs";
import { SettingsService as SettingsServiceAbstraction } from "../abstractions/settings.service";
import { StateService } from "../abstractions/state.service";
const Keys = {
settingsPrefix: "settings_",
equivalentDomains: "equivalentDomains",
};
import { Utils } from "../misc/utils";
import { AccountSettingsSettings } from "../models/domain/account";
export class SettingsService implements SettingsServiceAbstraction {
constructor(private stateService: StateService) {}
private _settings: BehaviorSubject<AccountSettingsSettings> = new BehaviorSubject({});
async clearCache(): Promise<void> {
await this.stateService.setSettings(null);
}
settings$ = this._settings.asObservable();
getEquivalentDomains(): Promise<any> {
return this.getSettingsKey(Keys.equivalentDomains);
constructor(private stateService: StateService) {
this.stateService.activeAccountUnlocked$
.pipe(
concatMap(async (unlocked) => {
if (Utils.global.bitwardenContainerService == null) {
return;
}
if (!unlocked) {
this._settings.next({});
return;
}
const data = await this.stateService.getSettings();
this._settings.next(data);
})
)
.subscribe();
}
async setEquivalentDomains(equivalentDomains: string[][]): Promise<void> {
await this.setSettingsKey(Keys.equivalentDomains, equivalentDomains);
const settings = this._settings.getValue() ?? {};
settings.equivalentDomains = equivalentDomains;
this._settings.next(settings);
await this.stateService.setSettings(settings);
}
async clear(userId?: string): Promise<void> {
if (userId == null || userId == (await this.stateService.getUserId())) {
this._settings.next({});
}
await this.stateService.setSettings(null, { userId: userId });
}
// Helpers
private async getSettings(): Promise<any> {
const settings = await this.stateService.getSettings();
if (settings == null) {
// eslint-disable-next-line
const userId = await this.stateService.getUserId();
}
return settings;
}
private async getSettingsKey(key: string): Promise<any> {
const settings = await this.getSettings();
if (settings != null && settings[key]) {
return settings[key];
}
return null;
}
private async setSettingsKey(key: string, value: any): Promise<void> {
let settings = await this.getSettings();
if (!settings) {
settings = {};
}
settings[key] = value;
await this.stateService.setSettings(settings);
}
}

View File

@ -20,7 +20,12 @@ import { OrganizationData } from "../models/data/organizationData";
import { PolicyData } from "../models/data/policyData";
import { ProviderData } from "../models/data/providerData";
import { SendData } from "../models/data/sendData";
import { Account, AccountData, AccountSettings } from "../models/domain/account";
import {
Account,
AccountData,
AccountSettings,
AccountSettingsSettings,
} from "../models/domain/account";
import { EncString } from "../models/domain/encString";
import { EnvironmentUrls } from "../models/domain/environmentUrls";
import { GeneratedPasswordHistory } from "../models/domain/generatedPasswordHistory";
@ -2075,13 +2080,13 @@ export class StateService<
);
}
async getSettings(options?: StorageOptions): Promise<any> {
async getSettings(options?: StorageOptions): Promise<AccountSettingsSettings> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()))
)?.settings?.settings;
}
async setSettings(value: string, options?: StorageOptions): Promise<void> {
async setSettings(value: AccountSettingsSettings, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions())
);

View File

@ -12,7 +12,7 @@ import { OrganizationData } from "../models/data/organizationData";
import { PolicyData } from "../models/data/policyData";
import { ProviderData } from "../models/data/providerData";
import { SendData } from "../models/data/sendData";
import { Account, AccountSettings } from "../models/domain/account";
import { Account, AccountSettings, AccountSettingsSettings } from "../models/domain/account";
import { EnvironmentUrls } from "../models/domain/environmentUrls";
import { GeneratedPasswordHistory } from "../models/domain/generatedPasswordHistory";
import { GlobalState } from "../models/domain/globalState";
@ -319,7 +319,10 @@ export class StateMigrationService<
encrypted: await this.get<string>(v1Keys.pinProtected),
},
protectedPin: await this.get<string>(v1Keys.protectedPin),
settings: userId == null ? null : await this.get<any>(v1KeyPrefixes.settings + userId),
settings:
userId == null
? null
: await this.get<AccountSettingsSettings>(v1KeyPrefixes.settings + userId),
vaultTimeout:
(await this.get<number>(v1Keys.vaultTimeout)) ?? defaultAccount.settings.vaultTimeout,
vaultTimeoutAction: