From a5c78fbe0dfd4d745218b35557d7b998eb828fd6 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 7 Mar 2024 17:15:03 +0000 Subject: [PATCH 01/31] [deps] Tools: Update electron to v28.2.6 (#8238) * [deps] Tools: Update electron to v28.2.6 * Update version in electron-builder.json --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel James Smith --- apps/desktop/electron-builder.json | 2 +- package-lock.json | 8 ++++---- package.json | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/desktop/electron-builder.json b/apps/desktop/electron-builder.json index 83a2179d58c..1bbf0402da8 100644 --- a/apps/desktop/electron-builder.json +++ b/apps/desktop/electron-builder.json @@ -24,7 +24,7 @@ "**/node_modules/argon2/package.json", "**/node_modules/argon2/lib/binding/napi-v3/argon2.node" ], - "electronVersion": "28.2.5", + "electronVersion": "28.2.6", "generateUpdatesFilesForAllChannels": true, "publish": { "provider": "generic", diff --git a/package-lock.json b/package-lock.json index 3bf0136e7f0..b4ba333c878 100644 --- a/package-lock.json +++ b/package-lock.json @@ -128,7 +128,7 @@ "copy-webpack-plugin": "11.0.0", "cross-env": "7.0.3", "css-loader": "6.8.1", - "electron": "28.2.5", + "electron": "28.2.6", "electron-builder": "24.9.1", "electron-log": "5.0.1", "electron-reload": "2.0.0-alpha.1", @@ -17807,9 +17807,9 @@ } }, "node_modules/electron": { - "version": "28.2.5", - "resolved": "https://registry.npmjs.org/electron/-/electron-28.2.5.tgz", - "integrity": "sha512-qlvQkDNVAzN647NpiJJw7GYJqE0NwK4+1evkhrQ0Xv6Qgab1EtN50G4oDr4/x/+O5pGUG2P5d3isXu+37O3RDw==", + "version": "28.2.6", + "resolved": "https://registry.npmjs.org/electron/-/electron-28.2.6.tgz", + "integrity": "sha512-RuhbW+ifvh3DqnVlHCcCKhKIFOxTktq1GN1gkIkEZ8y5LEZfcjOkxB2s6Fd1S6MzsMZbiJti+ZJG5hXS4SDVLQ==", "dev": true, "hasInstallScript": true, "dependencies": { diff --git a/package.json b/package.json index 854aff73bbb..7d273fa4954 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,7 @@ "copy-webpack-plugin": "11.0.0", "cross-env": "7.0.3", "css-loader": "6.8.1", - "electron": "28.2.5", + "electron": "28.2.6", "electron-builder": "24.9.1", "electron-log": "5.0.1", "electron-reload": "2.0.0-alpha.1", From c7318311af85ea4148cda7889cd427c745b46153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Thu, 7 Mar 2024 12:45:56 -0500 Subject: [PATCH 02/31] [PM-5974] introduce ForwarderGeneratorStrategy (#8207) * update defaults to include `website` parameter * update utilities tests to include `website` parameter --- .../tools/generator/key-definition.spec.ts | 54 ++++++++++++++ .../src/tools/generator/key-definitions.ts | 54 ++++++++++++++ .../forwarder-generator-strategy.spec.ts | 73 +++++++++++++++++++ .../username/forwarder-generator-strategy.ts | 73 +++++++++++++++++++ .../username/forwarders/addy-io.spec.ts | 47 ++++++++---- .../generator/username/forwarders/addy-io.ts | 43 ++++++++--- .../username/forwarders/duck-duck-go.spec.ts | 32 +++++--- .../username/forwarders/duck-duck-go.ts | 30 ++++++-- .../username/forwarders/fastmail.spec.ts | 42 +++++++---- .../generator/username/forwarders/fastmail.ts | 35 ++++++--- .../username/forwarders/firefox-relay.spec.ts | 32 +++++--- .../username/forwarders/firefox-relay.ts | 40 +++++++--- .../username/forwarders/forward-email.spec.ts | 52 ++++++++----- .../username/forwarders/forward-email.ts | 45 ++++++++---- .../username/forwarders/simple-login.spec.ts | 42 +++++++---- .../username/forwarders/simple-login.ts | 36 ++++++--- .../generator/username/options/constants.ts | 6 ++ .../username/options/forwarder-options.ts | 24 +++--- .../username/options/utilities.spec.ts | 6 ++ 19 files changed, 610 insertions(+), 156 deletions(-) create mode 100644 libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts create mode 100644 libs/common/src/tools/generator/username/forwarder-generator-strategy.ts diff --git a/libs/common/src/tools/generator/key-definition.spec.ts b/libs/common/src/tools/generator/key-definition.spec.ts index 6ee9288820f..735377a5ba2 100644 --- a/libs/common/src/tools/generator/key-definition.spec.ts +++ b/libs/common/src/tools/generator/key-definition.spec.ts @@ -5,6 +5,12 @@ import { SUBADDRESS_SETTINGS, PASSPHRASE_SETTINGS, PASSWORD_SETTINGS, + SIMPLE_LOGIN_FORWARDER, + FORWARD_EMAIL_FORWARDER, + FIREFOX_RELAY_FORWARDER, + FASTMAIL_FORWARDER, + DUCK_DUCK_GO_FORWARDER, + ADDY_IO_FORWARDER, } from "./key-definitions"; describe("Key definitions", () => { @@ -48,6 +54,54 @@ describe("Key definitions", () => { }); }); + describe("ADDY_IO_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = ADDY_IO_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + + describe("DUCK_DUCK_GO_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = DUCK_DUCK_GO_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + + describe("FASTMAIL_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = FASTMAIL_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + + describe("FIREFOX_RELAY_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = FIREFOX_RELAY_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + + describe("FORWARD_EMAIL_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = FORWARD_EMAIL_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + + describe("SIMPLE_LOGIN_FORWARDER", () => { + it("should pass through deserialization", () => { + const value: any = {}; + const result = SIMPLE_LOGIN_FORWARDER.deserializer(value); + expect(result).toBe(value); + }); + }); + describe("ENCRYPTED_HISTORY", () => { it("should pass through deserialization", () => { const value = {}; diff --git a/libs/common/src/tools/generator/key-definitions.ts b/libs/common/src/tools/generator/key-definitions.ts index fc51e430dd8..bb7c4e8a086 100644 --- a/libs/common/src/tools/generator/key-definitions.ts +++ b/libs/common/src/tools/generator/key-definitions.ts @@ -5,6 +5,12 @@ import { GeneratedPasswordHistory } from "./password/generated-password-history" import { PasswordGenerationOptions } from "./password/password-generation-options"; import { CatchallGenerationOptions } from "./username/catchall-generator-options"; import { EffUsernameGenerationOptions } from "./username/eff-username-generator-options"; +import { + ApiOptions, + EmailDomainOptions, + EmailPrefixOptions, + SelfHostedApiOptions, +} from "./username/options/forwarder-options"; import { SubaddressGenerationOptions } from "./username/subaddress-generator-options"; /** plaintext password generation options */ @@ -52,6 +58,54 @@ export const SUBADDRESS_SETTINGS = new KeyDefinition( + GENERATOR_DISK, + "addyIoForwarder", + { + deserializer: (value) => value, + }, +); + +export const DUCK_DUCK_GO_FORWARDER = new KeyDefinition( + GENERATOR_DISK, + "duckDuckGoForwarder", + { + deserializer: (value) => value, + }, +); + +export const FASTMAIL_FORWARDER = new KeyDefinition( + GENERATOR_DISK, + "fastmailForwarder", + { + deserializer: (value) => value, + }, +); + +export const FIREFOX_RELAY_FORWARDER = new KeyDefinition( + GENERATOR_DISK, + "firefoxRelayForwarder", + { + deserializer: (value) => value, + }, +); + +export const FORWARD_EMAIL_FORWARDER = new KeyDefinition( + GENERATOR_DISK, + "forwardEmailForwarder", + { + deserializer: (value) => value, + }, +); + +export const SIMPLE_LOGIN_FORWARDER = new KeyDefinition( + GENERATOR_DISK, + "simpleLoginForwarder", + { + deserializer: (value) => value, + }, +); + /** encrypted password generation history */ export const ENCRYPTED_HISTORY = new KeyDefinition( GENERATOR_DISK, diff --git a/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts b/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts new file mode 100644 index 00000000000..96a7bca2b1d --- /dev/null +++ b/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts @@ -0,0 +1,73 @@ +import { mock } from "jest-mock-extended"; + +import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; +import { CryptoService } from "../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../platform/abstractions/encrypt.service"; +import { StateProvider } from "../../../platform/state"; +import { UserId } from "../../../types/guid"; +import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; +import { DUCK_DUCK_GO_FORWARDER } from "../key-definitions"; +import { SecretState } from "../state/secret-state"; + +import { ForwarderGeneratorStrategy } from "./forwarder-generator-strategy"; +import { ApiOptions } from "./options/forwarder-options"; + +class TestForwarder extends ForwarderGeneratorStrategy { + constructor( + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } + + get key() { + // arbitrary. + return DUCK_DUCK_GO_FORWARDER; + } +} + +const SomeUser = "some user" as UserId; +const AnotherUser = "another user" as UserId; + +describe("ForwarderGeneratorStrategy", () => { + const encryptService = mock(); + const keyService = mock(); + const stateProvider = new FakeStateProvider(mockAccountServiceWith(SomeUser)); + + describe("durableState", () => { + it("constructs a secret state", () => { + const strategy = new TestForwarder(encryptService, keyService, stateProvider); + + const result = strategy.durableState(SomeUser); + + expect(result).toBeInstanceOf(SecretState); + }); + + it("returns the same secret state for a single user", () => { + const strategy = new TestForwarder(encryptService, keyService, stateProvider); + + const firstResult = strategy.durableState(SomeUser); + const secondResult = strategy.durableState(SomeUser); + + expect(firstResult).toBe(secondResult); + }); + + it("returns a different secret state for a different user", () => { + const strategy = new TestForwarder(encryptService, keyService, stateProvider); + + const firstResult = strategy.durableState(SomeUser); + const secondResult = strategy.durableState(AnotherUser); + + expect(firstResult).not.toBe(secondResult); + }); + }); + + it("evaluator returns the default policy evaluator", () => { + const strategy = new TestForwarder(null, null, null); + + const result = strategy.evaluator(null); + + expect(result).toBeInstanceOf(DefaultPolicyEvaluator); + }); +}); diff --git a/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts b/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts new file mode 100644 index 00000000000..554bbfca62a --- /dev/null +++ b/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts @@ -0,0 +1,73 @@ +import { PolicyType } from "../../../admin-console/enums"; +import { Policy } from "../../../admin-console/models/domain/policy"; +import { CryptoService } from "../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../platform/abstractions/encrypt.service"; +import { KeyDefinition, StateProvider } from "../../../platform/state"; +import { UserId } from "../../../types/guid"; +import { GeneratorStrategy } from "../abstractions"; +import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; +import { NoPolicy } from "../no-policy"; +import { PaddedDataPacker } from "../state/padded-data-packer"; +import { SecretClassifier } from "../state/secret-classifier"; +import { SecretState } from "../state/secret-state"; +import { UserKeyEncryptor } from "../state/user-key-encryptor"; + +import { ApiOptions } from "./options/forwarder-options"; + +const ONE_MINUTE = 60 * 1000; +const OPTIONS_FRAME_SIZE = 512; + +/** An email forwarding service configurable through an API. */ +export abstract class ForwarderGeneratorStrategy< + Options extends ApiOptions, +> extends GeneratorStrategy { + /** Initializes the generator strategy + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage + */ + constructor( + private readonly encryptService: EncryptService, + private readonly keyService: CryptoService, + private stateProvider: StateProvider, + ) { + super(); + // Uses password generator since there aren't policies + // specific to usernames. + this.policy = PolicyType.PasswordGenerator; + + this.cache_ms = ONE_MINUTE; + } + + private durableStates = new Map>>(); + + /** {@link GeneratorStrategy.durableState} */ + durableState = (userId: UserId) => { + let state = this.durableStates.get(userId); + + if (!state) { + const encryptor = this.createEncryptor(); + state = SecretState.from(userId, this.key, this.stateProvider, encryptor); + this.durableStates.set(userId, state); + } + + return state; + }; + + private createEncryptor() { + // always exclude request properties + const classifier = SecretClassifier.allSecret().exclude("website"); + + // construct the encryptor + const packer = new PaddedDataPacker(OPTIONS_FRAME_SIZE); + return new UserKeyEncryptor(this.encryptService, this.keyService, classifier, packer); + } + + /** Determine where forwarder configuration is stored */ + protected abstract readonly key: KeyDefinition; + + /** {@link GeneratorStrategy.evaluator} */ + evaluator = (_policy: Policy) => { + return new DefaultPolicyEvaluator(); + }; +} diff --git a/libs/common/src/tools/generator/username/forwarders/addy-io.spec.ts b/libs/common/src/tools/generator/username/forwarders/addy-io.spec.ts index cc742dc9209..c2428aefcaa 100644 --- a/libs/common/src/tools/generator/username/forwarders/addy-io.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/addy-io.spec.ts @@ -2,22 +2,30 @@ * include Request in test environment. * @jest-environment ../../../../shared/test.environment.ts */ +import { ADDY_IO_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { AddyIoForwarder } from "./addy-io"; import { mockApiService, mockI18nService } from "./mocks.jest"; describe("Addy.io Forwarder", () => { + it("key returns the Addy IO forwarder key", () => { + const forwarder = new AddyIoForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(ADDY_IO_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, domain: "example.com", baseUrl: "https://api.example.com", @@ -34,11 +42,12 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain, baseUrl: "https://api.example.com", @@ -56,11 +65,12 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", baseUrl, @@ -83,9 +93,10 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); - await forwarder.generate(website, { + await forwarder.generate({ + website, token: "token", domain: "example.com", baseUrl: "https://api.example.com", @@ -107,9 +118,10 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(status, { data: { email } }); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", domain: "example.com", baseUrl: "https://api.example.com", @@ -124,11 +136,12 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(401, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", baseUrl: "https://api.example.com", @@ -148,11 +161,12 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(500, {}); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", baseUrl: "https://api.example.com", @@ -181,11 +195,12 @@ describe("Addy.io Forwarder", () => { const apiService = mockApiService(statusCode, {}, statusText); const i18nService = mockI18nService(); - const forwarder = new AddyIoForwarder(apiService, i18nService); + const forwarder = new AddyIoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", baseUrl: "https://api.example.com", diff --git a/libs/common/src/tools/generator/username/forwarders/addy-io.ts b/libs/common/src/tools/generator/username/forwarders/addy-io.ts index 96e02033fe2..2db69e2396c 100644 --- a/libs/common/src/tools/generator/username/forwarders/addy-io.ts +++ b/libs/common/src/tools/generator/username/forwarders/addy-io.ts @@ -1,24 +1,41 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; +import { StateProvider } from "../../../../platform/state"; +import { ADDY_IO_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { EmailDomainOptions, Forwarder, SelfHostedApiOptions } from "../options/forwarder-options"; +import { EmailDomainOptions, SelfHostedApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for addy.io (formerly anon addy) */ -export class AddyIoForwarder implements Forwarder { +export class AddyIoForwarder extends ForwarderGeneratorStrategy< + SelfHostedApiOptions & EmailDomainOptions +> { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate( - website: string | null, - options: SelfHostedApiOptions & EmailDomainOptions, - ): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return ADDY_IO_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: SelfHostedApiOptions & EmailDomainOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.AddyIo.name); throw error; @@ -32,9 +49,11 @@ export class AddyIoForwarder implements Forwarder { throw error; } - const descriptionId = - website && website !== "" ? "forwarderGeneratedByWithWebsite" : "forwarderGeneratedBy"; - const description = this.i18nService.t(descriptionId, website ?? ""); + let descriptionId = "forwarderGeneratedByWithWebsite"; + if (!options.website || options.website === "") { + descriptionId = "forwarderGeneratedBy"; + } + const description = this.i18nService.t(descriptionId, options.website ?? ""); const url = options.baseUrl + "/api/v1/aliases"; const request = new Request(url, { @@ -70,5 +89,5 @@ export class AddyIoForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.AddyIo.name); throw error; } - } + }; } diff --git a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.spec.ts b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.spec.ts index 7b5765f9a7e..211eaead6dc 100644 --- a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.spec.ts @@ -2,22 +2,30 @@ * include Request in test environment. * @jest-environment ../../../../shared/test.environment.ts */ +import { DUCK_DUCK_GO_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { DuckDuckGoForwarder } from "./duck-duck-go"; import { mockApiService, mockI18nService } from "./mocks.jest"; describe("DuckDuckGo Forwarder", () => { + it("key returns the Duck Duck Go forwarder key", () => { + const forwarder = new DuckDuckGoForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(DUCK_DUCK_GO_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new DuckDuckGoForwarder(apiService, i18nService); + const forwarder = new DuckDuckGoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, }), ).rejects.toEqual("forwaderInvalidToken"); @@ -40,9 +48,10 @@ describe("DuckDuckGo Forwarder", () => { const apiService = mockApiService(status, { address }); const i18nService = mockI18nService(); - const forwarder = new DuckDuckGoForwarder(apiService, i18nService); + const forwarder = new DuckDuckGoForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", }); @@ -55,11 +64,12 @@ describe("DuckDuckGo Forwarder", () => { const apiService = mockApiService(401, {}); const i18nService = mockI18nService(); - const forwarder = new DuckDuckGoForwarder(apiService, i18nService); + const forwarder = new DuckDuckGoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", }), ).rejects.toEqual("forwaderInvalidToken"); @@ -76,11 +86,12 @@ describe("DuckDuckGo Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new DuckDuckGoForwarder(apiService, i18nService); + const forwarder = new DuckDuckGoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", }), ).rejects.toEqual("forwarderUnknownError"); @@ -99,11 +110,12 @@ describe("DuckDuckGo Forwarder", () => { const apiService = mockApiService(statusCode, {}); const i18nService = mockI18nService(); - const forwarder = new DuckDuckGoForwarder(apiService, i18nService); + const forwarder = new DuckDuckGoForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", }), ).rejects.toEqual("forwarderUnknownError"); diff --git a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts index 8078230b3a8..daf4f7b4445 100644 --- a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts +++ b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts @@ -1,21 +1,39 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; +import { StateProvider } from "../../../../platform/state"; +import { DUCK_DUCK_GO_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { ApiOptions, Forwarder } from "../options/forwarder-options"; +import { ApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for DuckDuckGo */ -export class DuckDuckGoForwarder implements Forwarder { +export class DuckDuckGoForwarder extends ForwarderGeneratorStrategy { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate(_website: string | null, options: ApiOptions): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return DUCK_DUCK_GO_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: ApiOptions): Promise => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.DuckDuckGo.name); throw error; @@ -48,5 +66,5 @@ export class DuckDuckGoForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.DuckDuckGo.name); throw error; } - } + }; } diff --git a/libs/common/src/tools/generator/username/forwarders/fastmail.spec.ts b/libs/common/src/tools/generator/username/forwarders/fastmail.spec.ts index 6d557399aa5..bab2b93966a 100644 --- a/libs/common/src/tools/generator/username/forwarders/fastmail.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/fastmail.spec.ts @@ -3,6 +3,7 @@ * @jest-environment ../../../../shared/test.environment.ts */ import { ApiService } from "../../../../abstractions/api.service"; +import { FASTMAIL_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { FastmailForwarder } from "./fastmail"; @@ -45,16 +46,23 @@ const AccountIdSuccess: MockResponse = Object.freeze({ // the tests describe("Fastmail Forwarder", () => { + it("key returns the Fastmail forwarder key", () => { + const forwarder = new FastmailForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(FASTMAIL_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(AccountIdSuccess, EmptyResponse); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, domain: "example.com", prefix: "prefix", @@ -71,11 +79,12 @@ describe("Fastmail Forwarder", () => { const apiService = mockApiService({ status, body: {} }, EmptyResponse); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", @@ -105,9 +114,10 @@ describe("Fastmail Forwarder", () => { }); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", @@ -138,11 +148,12 @@ describe("Fastmail Forwarder", () => { }); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", @@ -165,11 +176,12 @@ describe("Fastmail Forwarder", () => { const apiService = mockApiService(AccountIdSuccess, { status, body: {} }); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", @@ -206,11 +218,12 @@ describe("Fastmail Forwarder", () => { }); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", @@ -232,11 +245,12 @@ describe("Fastmail Forwarder", () => { const apiService = mockApiService(AccountIdSuccess, { status: statusCode, body: {} }); const i18nService = mockI18nService(); - const forwarder = new FastmailForwarder(apiService, i18nService); + const forwarder = new FastmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", prefix: "prefix", diff --git a/libs/common/src/tools/generator/username/forwarders/fastmail.ts b/libs/common/src/tools/generator/username/forwarders/fastmail.ts index b6b40946a8d..b4e2b56695b 100644 --- a/libs/common/src/tools/generator/username/forwarders/fastmail.ts +++ b/libs/common/src/tools/generator/username/forwarders/fastmail.ts @@ -1,24 +1,39 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; +import { StateProvider } from "../../../../platform/state"; +import { FASTMAIL_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { EmailPrefixOptions, Forwarder, ApiOptions } from "../options/forwarder-options"; +import { EmailPrefixOptions, ApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for Fastmail */ -export class FastmailForwarder implements Forwarder { +export class FastmailForwarder extends ForwarderGeneratorStrategy { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate( - website: string | null, - options: ApiOptions & EmailPrefixOptions, - ): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return FASTMAIL_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: ApiOptions & EmailPrefixOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.Fastmail.name); throw error; @@ -41,7 +56,7 @@ export class FastmailForwarder implements Forwarder { "new-masked-email": { state: "enabled", description: "", - forDomain: website, + forDomain: options.website, emailPrefix: options.prefix, }, }, @@ -104,7 +119,7 @@ export class FastmailForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.Fastmail.name); throw error; - } + }; private async getAccountId(options: ApiOptions): Promise { const requestInit: RequestInit = { diff --git a/libs/common/src/tools/generator/username/forwarders/firefox-relay.spec.ts b/libs/common/src/tools/generator/username/forwarders/firefox-relay.spec.ts index b5c1c66d6d3..5ba8d3f2f12 100644 --- a/libs/common/src/tools/generator/username/forwarders/firefox-relay.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/firefox-relay.spec.ts @@ -2,22 +2,30 @@ * include Request in test environment. * @jest-environment ../../../../shared/test.environment.ts */ +import { FIREFOX_RELAY_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { FirefoxRelayForwarder } from "./firefox-relay"; import { mockApiService, mockI18nService } from "./mocks.jest"; describe("Firefox Relay Forwarder", () => { + it("key returns the Firefox Relay forwarder key", () => { + const forwarder = new FirefoxRelayForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(FIREFOX_RELAY_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new FirefoxRelayForwarder(apiService, i18nService); + const forwarder = new FirefoxRelayForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, }), ).rejects.toEqual("forwaderInvalidToken"); @@ -40,9 +48,10 @@ describe("Firefox Relay Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new FirefoxRelayForwarder(apiService, i18nService); + const forwarder = new FirefoxRelayForwarder(apiService, i18nService, null, null, null); - await forwarder.generate(website, { + await forwarder.generate({ + website, token: "token", }); @@ -62,9 +71,10 @@ describe("Firefox Relay Forwarder", () => { const apiService = mockApiService(status, { full_address }); const i18nService = mockI18nService(); - const forwarder = new FirefoxRelayForwarder(apiService, i18nService); + const forwarder = new FirefoxRelayForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", }); @@ -77,11 +87,12 @@ describe("Firefox Relay Forwarder", () => { const apiService = mockApiService(401, {}); const i18nService = mockI18nService(); - const forwarder = new FirefoxRelayForwarder(apiService, i18nService); + const forwarder = new FirefoxRelayForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", }), ).rejects.toEqual("forwaderInvalidToken"); @@ -101,11 +112,12 @@ describe("Firefox Relay Forwarder", () => { const apiService = mockApiService(statusCode, {}); const i18nService = mockI18nService(); - const forwarder = new FirefoxRelayForwarder(apiService, i18nService); + const forwarder = new FirefoxRelayForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", }), ).rejects.toEqual("forwarderUnknownError"); diff --git a/libs/common/src/tools/generator/username/forwarders/firefox-relay.ts b/libs/common/src/tools/generator/username/forwarders/firefox-relay.ts index 1bb19ed7fea..1308852224c 100644 --- a/libs/common/src/tools/generator/username/forwarders/firefox-relay.ts +++ b/libs/common/src/tools/generator/username/forwarders/firefox-relay.ts @@ -1,21 +1,39 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; +import { StateProvider } from "../../../../platform/state"; +import { FIREFOX_RELAY_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { Forwarder, ApiOptions } from "../options/forwarder-options"; +import { ApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for Firefox Relay */ -export class FirefoxRelayForwarder implements Forwarder { +export class FirefoxRelayForwarder extends ForwarderGeneratorStrategy { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate(website: string | null, options: ApiOptions): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return FIREFOX_RELAY_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: ApiOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.FirefoxRelay.name); throw error; @@ -23,9 +41,11 @@ export class FirefoxRelayForwarder implements Forwarder { const url = "https://relay.firefox.com/api/v1/relayaddresses/"; - const descriptionId = - website && website !== "" ? "forwarderGeneratedByWithWebsite" : "forwarderGeneratedBy"; - const description = this.i18nService.t(descriptionId, website ?? ""); + let descriptionId = "forwarderGeneratedByWithWebsite"; + if (!options.website || options.website === "") { + descriptionId = "forwarderGeneratedBy"; + } + const description = this.i18nService.t(descriptionId, options.website ?? ""); const request = new Request(url, { redirect: "manual", @@ -37,7 +57,7 @@ export class FirefoxRelayForwarder implements Forwarder { }), body: JSON.stringify({ enabled: true, - generated_for: website, + generated_for: options.website, description, }), }); @@ -53,5 +73,5 @@ export class FirefoxRelayForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.FirefoxRelay.name); throw error; } - } + }; } diff --git a/libs/common/src/tools/generator/username/forwarders/forward-email.spec.ts b/libs/common/src/tools/generator/username/forwarders/forward-email.spec.ts index a5c2e14d378..daf0f3d7f1f 100644 --- a/libs/common/src/tools/generator/username/forwarders/forward-email.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/forward-email.spec.ts @@ -2,22 +2,30 @@ * include Request in test environment. * @jest-environment ../../../../shared/test.environment.ts */ +import { FORWARD_EMAIL_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { ForwardEmailForwarder } from "./forward-email"; import { mockApiService, mockI18nService } from "./mocks.jest"; describe("ForwardEmail Forwarder", () => { + it("key returns the Forward Email forwarder key", () => { + const forwarder = new ForwardEmailForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(FORWARD_EMAIL_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, domain: "example.com", }), @@ -36,11 +44,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain, }), @@ -65,9 +74,10 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); - await forwarder.generate(website, { + await forwarder.generate({ + website, token: "token", domain: "example.com", }); @@ -92,9 +102,10 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(status, response); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", domain: "example.com", }); @@ -108,11 +119,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(401, {}); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", }), @@ -132,11 +144,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(401, { message: "A message" }); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", }), @@ -158,11 +171,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(500, json); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", }), @@ -191,11 +205,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(statusCode, { message }); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", }), @@ -225,11 +240,12 @@ describe("ForwardEmail Forwarder", () => { const apiService = mockApiService(statusCode, { error }); const i18nService = mockI18nService(); - const forwarder = new ForwardEmailForwarder(apiService, i18nService); + const forwarder = new ForwardEmailForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", domain: "example.com", }), diff --git a/libs/common/src/tools/generator/username/forwarders/forward-email.ts b/libs/common/src/tools/generator/username/forwarders/forward-email.ts index 7fc727e4711..eb6e3cd0c67 100644 --- a/libs/common/src/tools/generator/username/forwarders/forward-email.ts +++ b/libs/common/src/tools/generator/username/forwarders/forward-email.ts @@ -1,25 +1,42 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { Utils } from "../../../../platform/misc/utils"; +import { StateProvider } from "../../../../platform/state"; +import { FORWARD_EMAIL_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { EmailDomainOptions, Forwarder, ApiOptions } from "../options/forwarder-options"; +import { EmailDomainOptions, ApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for Forward Email */ -export class ForwardEmailForwarder implements Forwarder { +export class ForwardEmailForwarder extends ForwarderGeneratorStrategy< + ApiOptions & EmailDomainOptions +> { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate( - website: string | null, - options: ApiOptions & EmailDomainOptions, - ): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return FORWARD_EMAIL_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: ApiOptions & EmailDomainOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.ForwardEmail.name); throw error; @@ -31,9 +48,11 @@ export class ForwardEmailForwarder implements Forwarder { const url = `https://api.forwardemail.net/v1/domains/${options.domain}/aliases`; - const descriptionId = - website && website !== "" ? "forwarderGeneratedByWithWebsite" : "forwarderGeneratedBy"; - const description = this.i18nService.t(descriptionId, website ?? ""); + let descriptionId = "forwarderGeneratedByWithWebsite"; + if (!options.website || options.website === "") { + descriptionId = "forwarderGeneratedBy"; + } + const description = this.i18nService.t(descriptionId, options.website ?? ""); const request = new Request(url, { redirect: "manual", @@ -44,7 +63,7 @@ export class ForwardEmailForwarder implements Forwarder { "Content-Type": "application/json", }), body: JSON.stringify({ - labels: website, + labels: options.website, description, }), }); @@ -75,5 +94,5 @@ export class ForwardEmailForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.ForwardEmail.name); throw error; } - } + }; } diff --git a/libs/common/src/tools/generator/username/forwarders/simple-login.spec.ts b/libs/common/src/tools/generator/username/forwarders/simple-login.spec.ts index 60ca329e8ba..1120d49ce31 100644 --- a/libs/common/src/tools/generator/username/forwarders/simple-login.spec.ts +++ b/libs/common/src/tools/generator/username/forwarders/simple-login.spec.ts @@ -2,22 +2,30 @@ * include Request in test environment. * @jest-environment ../../../../shared/test.environment.ts */ +import { SIMPLE_LOGIN_FORWARDER } from "../../key-definitions"; import { Forwarders } from "../options/constants"; import { mockApiService, mockI18nService } from "./mocks.jest"; import { SimpleLoginForwarder } from "./simple-login"; describe("SimpleLogin Forwarder", () => { + it("key returns the Simple Login forwarder key", () => { + const forwarder = new SimpleLoginForwarder(null, null, null, null, null); + + expect(forwarder.key).toBe(SIMPLE_LOGIN_FORWARDER); + }); + describe("generate(string | null, SelfHostedApiOptions & EmailDomainOptions)", () => { it.each([null, ""])("throws an error if the token is missing (token = %p)", async (token) => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token, baseUrl: "https://api.example.com", }), @@ -36,11 +44,12 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", baseUrl, }), @@ -62,9 +71,10 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(200, {}); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); - await forwarder.generate(website, { + await forwarder.generate({ + website, token: "token", baseUrl: "https://api.example.com", }); @@ -85,9 +95,10 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(status, { alias }); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); - const result = await forwarder.generate(null, { + const result = await forwarder.generate({ + website: null, token: "token", baseUrl: "https://api.example.com", }); @@ -101,11 +112,12 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(401, {}); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", baseUrl: "https://api.example.com", }), @@ -126,11 +138,12 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(500, body); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", baseUrl: "https://api.example.com", }), @@ -159,11 +172,12 @@ describe("SimpleLogin Forwarder", () => { const apiService = mockApiService(statusCode, { error }); const i18nService = mockI18nService(); - const forwarder = new SimpleLoginForwarder(apiService, i18nService); + const forwarder = new SimpleLoginForwarder(apiService, i18nService, null, null, null); await expect( async () => - await forwarder.generate(null, { + await forwarder.generate({ + website: null, token: "token", baseUrl: "https://api.example.com", }), diff --git a/libs/common/src/tools/generator/username/forwarders/simple-login.ts b/libs/common/src/tools/generator/username/forwarders/simple-login.ts index 0000b680e36..33bd8e3d4e0 100644 --- a/libs/common/src/tools/generator/username/forwarders/simple-login.ts +++ b/libs/common/src/tools/generator/username/forwarders/simple-login.ts @@ -1,21 +1,39 @@ import { ApiService } from "../../../../abstractions/api.service"; +import { CryptoService } from "../../../../platform/abstractions/crypto.service"; +import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; +import { StateProvider } from "../../../../platform/state"; +import { SIMPLE_LOGIN_FORWARDER } from "../../key-definitions"; +import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; -import { Forwarder, SelfHostedApiOptions } from "../options/forwarder-options"; +import { SelfHostedApiOptions } from "../options/forwarder-options"; /** Generates a forwarding address for Simple Login */ -export class SimpleLoginForwarder implements Forwarder { +export class SimpleLoginForwarder extends ForwarderGeneratorStrategy { /** Instantiates the forwarder * @param apiService used for ajax requests to the forwarding service * @param i18nService used to look up error strings + * @param encryptService protects sensitive forwarder options + * @param keyService looks up the user key when protecting data. + * @param stateProvider creates the durable state for options storage */ constructor( private apiService: ApiService, private i18nService: I18nService, - ) {} + encryptService: EncryptService, + keyService: CryptoService, + stateProvider: StateProvider, + ) { + super(encryptService, keyService, stateProvider); + } - /** {@link Forwarder.generate} */ - async generate(website: string, options: SelfHostedApiOptions): Promise { + /** {@link ForwarderGeneratorStrategy.key} */ + get key() { + return SIMPLE_LOGIN_FORWARDER; + } + + /** {@link ForwarderGeneratorStrategy.generate} */ + generate = async (options: SelfHostedApiOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.SimpleLogin.name); throw error; @@ -27,11 +45,11 @@ export class SimpleLoginForwarder implements Forwarder { let url = options.baseUrl + "/api/alias/random/new"; let noteId = "forwarderGeneratedBy"; - if (website && website !== "") { - url += "?hostname=" + website; + if (options.website && options.website !== "") { + url += "?hostname=" + options.website; noteId = "forwarderGeneratedByWithWebsite"; } - const note = this.i18nService.t(noteId, website ?? ""); + const note = this.i18nService.t(noteId, options.website ?? ""); const request = new Request(url, { redirect: "manual", @@ -60,5 +78,5 @@ export class SimpleLoginForwarder implements Forwarder { const error = this.i18nService.t("forwarderUnknownError", Forwarders.SimpleLogin.name); throw error; } - } + }; } diff --git a/libs/common/src/tools/generator/username/options/constants.ts b/libs/common/src/tools/generator/username/options/constants.ts index 66f5d83af9c..8f7013d48be 100644 --- a/libs/common/src/tools/generator/username/options/constants.ts +++ b/libs/common/src/tools/generator/username/options/constants.ts @@ -85,27 +85,33 @@ export const DefaultOptions: UsernameGeneratorOptions = Object.freeze({ forwarders: Object.freeze({ service: Forwarders.Fastmail.id, fastMail: Object.freeze({ + website: null, domain: "", prefix: "", token: "", }), addyIo: Object.freeze({ + website: null, baseUrl: "https://app.addy.io", domain: "", token: "", }), forwardEmail: Object.freeze({ + website: null, token: "", domain: "", }), simpleLogin: Object.freeze({ + website: null, baseUrl: "https://app.simplelogin.io", token: "", }), duckDuckGo: Object.freeze({ + website: null, token: "", }), firefoxRelay: Object.freeze({ + website: null, token: "", }), }), diff --git a/libs/common/src/tools/generator/username/options/forwarder-options.ts b/libs/common/src/tools/generator/username/options/forwarder-options.ts index 02375726c81..f36a58a0db4 100644 --- a/libs/common/src/tools/generator/username/options/forwarder-options.ts +++ b/libs/common/src/tools/generator/username/options/forwarder-options.ts @@ -1,5 +1,3 @@ -import { EncString } from "../../../../platform/models/domain/enc-string"; - /** Identifiers for email forwarding services. * @remarks These are used to select forwarder-specific options. * The must be kept in sync with the forwarder implementations. @@ -24,26 +22,24 @@ export type ForwarderMetadata = { validForSelfHosted: boolean; }; -/** An email forwarding service configurable through an API. */ -export interface Forwarder { - /** Generate a forwarding email. - * @param website The website to generate a username for. - * @param options The options to use when generating the username. - */ - generate(website: string | null, options: ApiOptions): Promise; -} - /** Options common to all forwarder APIs */ export type ApiOptions = { /** bearer token that authenticates bitwarden to the forwarder. * This is required to issue an API request. */ token?: string; +} & RequestOptions; - /** encrypted bearer token that authenticates bitwarden to the forwarder. - * This is used to store the token at rest and must be decoded before use. +/** Options that provide contextual information about the application state + * when a forwarder is invoked. + * @remarks these fields should always be omitted when saving options. + */ +export type RequestOptions = { + /** @param website The domain of the website the generated email is used + * within. This should be set to `null` when the request is not specific + * to any website. */ - encryptedToken?: EncString; + website: string | null; }; /** Api configuration for forwarders that support self-hosted installations. */ diff --git a/libs/common/src/tools/generator/username/options/utilities.spec.ts b/libs/common/src/tools/generator/username/options/utilities.spec.ts index 904ac6dbfc6..7ab1d9dcfdb 100644 --- a/libs/common/src/tools/generator/username/options/utilities.spec.ts +++ b/libs/common/src/tools/generator/username/options/utilities.spec.ts @@ -24,27 +24,33 @@ const TestOptions: UsernameGeneratorOptions = { forwarders: { service: Forwarders.Fastmail.id, fastMail: { + website: null, domain: "httpbin.com", prefix: "foo", token: "some-token", }, addyIo: { + website: null, baseUrl: "https://app.addy.io", domain: "example.com", token: "some-token", }, forwardEmail: { + website: null, token: "some-token", domain: "example.com", }, simpleLogin: { + website: null, baseUrl: "https://app.simplelogin.io", token: "some-token", }, duckDuckGo: { + website: null, token: "some-token", }, firefoxRelay: { + website: null, token: "some-token", }, }, From bdaa95bd7f2c860243feecaf53838ae259db9192 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 7 Mar 2024 13:53:59 -0600 Subject: [PATCH 03/31] Remove unneeded provider override (#8230) --- apps/browser/src/popup/services/services.module.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d4c8ec57fda..82f719dab18 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -323,7 +323,6 @@ function getBgService(service: keyof MainBackground) { useFactory: getBgService("passwordGenerationService"), deps: [], }, - { provide: ApiService, useFactory: getBgService("apiService"), deps: [] }, { provide: SendService, useFactory: ( From 0f9acf50f759b23d0856a9df648ebc313e40f0d9 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 7 Mar 2024 13:58:58 -0600 Subject: [PATCH 04/31] Remove usage of `getBgService` from browser services (#8234) --- apps/browser/src/popup/services/services.module.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 82f719dab18..2e6dfaee7a7 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -220,10 +220,6 @@ function getBgService(service: keyof MainBackground) { useFactory: () => new WebCryptoFunctionService(window), deps: [], }, - { - provide: FileUploadService, - useFactory: getBgService("fileUploadService"), - }, { provide: InternalFolderService, useExisting: FolderServiceAbstraction, From 73504d9bb0303157bc34e69a8a10c605585a7ead Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Thu, 7 Mar 2024 14:37:16 -0600 Subject: [PATCH 05/31] [PM-6469] Fix inline menu UI visibility race condition bug (#8104) --- .../autofill-overlay-content.service.spec.ts | 33 +++++++++++++++++-- .../autofill-overlay-content.service.ts | 8 ++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts index 4b3641208b0..8926f5b298e 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.spec.ts @@ -1214,7 +1214,10 @@ describe("AutofillOverlayContentService", () => { autofillOverlayContentService as any, "removeAutofillOverlay", ); + autofillOverlayContentService["mostRecentlyFocusedField"] = undefined; + autofillOverlayContentService["overlayButtonElement"] = document.createElement("div"); + autofillOverlayContentService["overlayListElement"] = document.createElement("div"); globalThis.dispatchEvent(new Event(EVENTS.SCROLL)); jest.advanceTimersByTime(800); @@ -1222,8 +1225,8 @@ describe("AutofillOverlayContentService", () => { expect(sendExtensionMessageSpy).toHaveBeenCalledWith("updateAutofillOverlayHidden", { display: "block", }); - expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(true); - expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(true); + expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(false); + expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(false); expect(removeAutofillOverlaySpy).toHaveBeenCalled(); }); @@ -1280,6 +1283,32 @@ describe("AutofillOverlayContentService", () => { expect(removeAutofillOverlaySpy).toHaveBeenCalled(); }); + + it("defaults overlay elements to a visibility of `false` if the element is not rendered on the page", async () => { + jest.useFakeTimers(); + jest + .spyOn(autofillOverlayContentService as any, "updateMostRecentlyFocusedField") + .mockImplementation(() => { + autofillOverlayContentService["focusedFieldData"] = { + focusedFieldRects: { + top: 100, + }, + focusedFieldStyles: {}, + }; + }); + jest + .spyOn(autofillOverlayContentService as any, "updateOverlayElementsPosition") + .mockImplementation(); + autofillOverlayContentService["overlayButtonElement"] = document.createElement("div"); + autofillOverlayContentService["overlayListElement"] = undefined; + + globalThis.dispatchEvent(new Event(EVENTS.SCROLL)); + jest.advanceTimersByTime(800); + await flushPromises(); + + expect(autofillOverlayContentService["isOverlayButtonVisible"]).toEqual(true); + expect(autofillOverlayContentService["isOverlayListVisible"]).toEqual(false); + }); }); describe("handleOverlayElementMutationObserverUpdate", () => { diff --git a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts index efbc9732b6c..2cf063a5ba8 100644 --- a/apps/browser/src/autofill/services/autofill-overlay-content.service.ts +++ b/apps/browser/src/autofill/services/autofill-overlay-content.service.ts @@ -646,12 +646,10 @@ class AutofillOverlayContentService implements AutofillOverlayContentServiceInte */ private toggleOverlayHidden(isHidden: boolean) { const displayValue = isHidden ? "none" : "block"; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendExtensionMessage("updateAutofillOverlayHidden", { display: displayValue }); + void this.sendExtensionMessage("updateAutofillOverlayHidden", { display: displayValue }); - this.isOverlayButtonVisible = !isHidden; - this.isOverlayListVisible = !isHidden; + this.isOverlayButtonVisible = !!this.overlayButtonElement && !isHidden; + this.isOverlayListVisible = !!this.overlayListElement && !isHidden; } /** From eedd6f08817bd9e1766fb26086c46280a0ac3862 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 8 Mar 2024 10:26:00 +1000 Subject: [PATCH 06/31] [AC-2008] [AC-2123] [Pt 2] Transition PolicyService to use StateProvider (#7977) * fully wire up StateProvider within PolicyService * migrate old policy data to new location * minor update to existing interfaces --- .../policy-service.factory.ts | 10 +- .../services/browser-policy.service.ts | 16 - .../browser/src/background/main.background.ts | 8 +- .../src/popup/services/services.module.ts | 12 - apps/cli/src/bw.ts | 8 +- .../src/services/jslib-services.module.ts | 2 +- .../src/tools/send/add-edit.component.ts | 9 +- .../policy/policy.service.abstraction.ts | 49 +- .../admin-console/models/data/policy.data.ts | 3 +- .../src/admin-console/models/domain/policy.ts | 3 +- .../models/response/policy.response.ts | 3 +- .../services/policy/policy.service.spec.ts | 664 +++++++++--------- .../services/policy/policy.service.ts | 167 +---- .../platform/abstractions/state.service.ts | 21 - .../src/platform/models/domain/account.ts | 3 - .../src/platform/services/state.service.ts | 41 -- .../vault-timeout-settings.service.spec.ts | 10 +- .../vault-timeout-settings.service.ts | 28 +- libs/common/src/state-migrations/migrate.ts | 6 +- ...ove-policy-state-to-state-provider.spec.ts | 192 +++++ .../30-move-policy-state-to-state-provider.ts | 76 ++ 21 files changed, 678 insertions(+), 653 deletions(-) delete mode 100644 apps/browser/src/admin-console/services/browser-policy.service.ts create mode 100644 libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts diff --git a/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts b/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts index efdb743196f..b00693bd564 100644 --- a/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts +++ b/apps/browser/src/admin-console/background/service-factories/policy-service.factory.ts @@ -1,4 +1,5 @@ import { PolicyService as AbstractPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { CachedServices, @@ -9,11 +10,6 @@ import { stateProviderFactory, StateProviderInitOptions, } from "../../../platform/background/service-factories/state-provider.factory"; -import { - stateServiceFactory as stateServiceFactory, - StateServiceInitOptions, -} from "../../../platform/background/service-factories/state-service.factory"; -import { BrowserPolicyService } from "../../services/browser-policy.service"; import { organizationServiceFactory, @@ -23,7 +19,6 @@ import { type PolicyServiceFactoryOptions = FactoryOptions; export type PolicyServiceInitOptions = PolicyServiceFactoryOptions & - StateServiceInitOptions & StateProviderInitOptions & OrganizationServiceInitOptions; @@ -36,8 +31,7 @@ export function policyServiceFactory( "policyService", opts, async () => - new BrowserPolicyService( - await stateServiceFactory(cache, opts), + new PolicyService( await stateProviderFactory(cache, opts), await organizationServiceFactory(cache, opts), ), diff --git a/apps/browser/src/admin-console/services/browser-policy.service.ts b/apps/browser/src/admin-console/services/browser-policy.service.ts deleted file mode 100644 index 2022cfec583..00000000000 --- a/apps/browser/src/admin-console/services/browser-policy.service.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { BehaviorSubject } from "rxjs"; -import { Jsonify } from "type-fest"; - -import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; - -import { browserSession, sessionSync } from "../../platform/decorators/session-sync-observable"; - -@browserSession -export class BrowserPolicyService extends PolicyService { - @sessionSync({ - initializer: (obj: Jsonify) => Object.assign(new Policy(), obj), - initializeAs: "array", - }) - protected _policies: BehaviorSubject; -} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index ce0c67efa11..c5608dc830e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -22,6 +22,7 @@ import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abs import { InternalPolicyService as InternalPolicyServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { ProviderService as ProviderServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/provider.service"; import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; +import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -175,7 +176,6 @@ import { } from "@bitwarden/vault-export-core"; import { BrowserOrganizationService } from "../admin-console/services/browser-organization.service"; -import { BrowserPolicyService } from "../admin-console/services/browser-policy.service"; import ContextMenusBackground from "../autofill/background/context-menus.background"; import NotificationBackground from "../autofill/background/notification.background"; import OverlayBackground from "../autofill/background/overlay.background"; @@ -501,11 +501,7 @@ export default class MainBackground { this.stateService, this.stateProvider, ); - this.policyService = new BrowserPolicyService( - this.stateService, - this.stateProvider, - this.organizationService, - ); + this.policyService = new PolicyService(this.stateProvider, this.organizationService); this.autofillSettingsService = new AutofillSettingsService( this.stateProvider, this.policyService, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 2e6dfaee7a7..40d68eb630f 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -102,7 +102,6 @@ import { ImportServiceAbstraction } from "@bitwarden/importer/core"; import { VaultExportServiceAbstraction } from "@bitwarden/vault-export-core"; import { BrowserOrganizationService } from "../../admin-console/services/browser-organization.service"; -import { BrowserPolicyService } from "../../admin-console/services/browser-policy.service"; import { UnauthGuardService } from "../../auth/popup/services"; import { AutofillService } from "../../autofill/services/abstractions/autofill.service"; import MainBackground from "../../background/main.background"; @@ -293,17 +292,6 @@ function getBgService(service: keyof MainBackground) { useFactory: getBgService("eventCollectionService"), deps: [], }, - { - provide: PolicyService, - useFactory: ( - stateService: StateServiceAbstraction, - stateProvider: StateProvider, - organizationService: OrganizationService, - ) => { - return new BrowserPolicyService(stateService, stateProvider, organizationService); - }, - deps: [StateServiceAbstraction, StateProvider, OrganizationService], - }, { provide: PlatformUtilsService, useFactory: getBgService("platformUtilsService"), diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index e46937f71f7..581d37c9e6a 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -394,11 +394,7 @@ export class Main { this.organizationUserService = new OrganizationUserServiceImplementation(this.apiService); - this.policyService = new PolicyService( - this.stateService, - this.stateProvider, - this.organizationService, - ); + this.policyService = new PolicyService(this.stateProvider, this.organizationService); this.policyApiService = new PolicyApiService(this.policyService, this.apiService); @@ -653,7 +649,7 @@ export class Main { this.cipherService.clear(userId), this.folderService.clear(userId), this.collectionService.clear(userId as UserId), - this.policyService.clear(userId), + this.policyService.clear(userId as UserId), this.passwordGenerationService.clear(), this.providerService.save(null, userId as UserId), ]); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index be3a65dcf24..5c83bc880c0 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -678,7 +678,7 @@ import { ModalService } from "./modal.service"; { provide: PolicyServiceAbstraction, useClass: PolicyService, - deps: [StateServiceAbstraction, StateProvider, OrganizationServiceAbstraction], + deps: [StateProvider, OrganizationServiceAbstraction], }, { provide: InternalPolicyService, diff --git a/libs/angular/src/tools/send/add-edit.component.ts b/libs/angular/src/tools/send/add-edit.component.ts index 9d213cfca5c..2e4b2cd57bb 100644 --- a/libs/angular/src/tools/send/add-edit.component.ts +++ b/libs/angular/src/tools/send/add-edit.component.ts @@ -1,7 +1,7 @@ import { DatePipe } from "@angular/common"; import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { Subject, takeUntil } from "rxjs"; +import { Subject, map, takeUntil } from "rxjs"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; @@ -151,8 +151,11 @@ export class AddEditComponent implements OnInit, OnDestroy { }); this.policyService - .policyAppliesToActiveUser$(PolicyType.SendOptions, (p) => p.data.disableHideEmail) - .pipe(takeUntil(this.destroy$)) + .getAll$(PolicyType.SendOptions) + .pipe( + map((policies) => policies?.some((p) => p.data.disableHideEmail)), + takeUntil(this.destroy$), + ) .subscribe((policyAppliesToActiveUser) => { if ( (this.disableHideEmail = policyAppliesToActiveUser) && diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index a9f544ad779..1a85fd79fce 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -1,6 +1,7 @@ import { Observable } from "rxjs"; import { ListResponse } from "../../../models/response/list.response"; +import { UserId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../../models/data/policy.data"; import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options"; @@ -10,9 +11,9 @@ import { PolicyResponse } from "../../models/response/policy.response"; export abstract class PolicyService { /** - * All {@link Policy} objects for the active user (from sync data). - * May include policies that are disabled or otherwise do not apply to the user. - * @see {@link get$} or {@link policyAppliesToActiveUser$} if you want to know when a policy applies to a user. + * All policies for the active user from sync data. + * May include policies that are disabled or otherwise do not apply to the user. Be careful using this! + * Consider using {@link get$} or {@link getAll$} instead, which will only return policies that should be enforced against the user. */ policies$: Observable; @@ -20,37 +21,33 @@ export abstract class PolicyService { * @returns the first {@link Policy} found that applies to the active user. * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). * @param policyType the {@link PolicyType} to search for - * @param policyFilter Optional predicate to apply when filtering policies + * @see {@link getAll$} if you need all policies of a given type */ - get$: (policyType: PolicyType, policyFilter?: (policy: Policy) => boolean) => Observable; + get$: (policyType: PolicyType) => Observable; + + /** + * @returns all {@link Policy} objects of a given type that apply to the specified user (or the active user if not specified). + * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * @param policyType the {@link PolicyType} to search for + */ + getAll$: (policyType: PolicyType, userId?: UserId) => Observable; /** * All {@link Policy} objects for the specified user (from sync data). * May include policies that are disabled or otherwise do not apply to the user. - * @see {@link policyAppliesToUser} if you want to know when a policy applies to the user. - * @deprecated Use {@link policies$} instead + * Consider using {@link getAll$} instead, which will only return policies that should be enforced against the user. */ - getAll: (type?: PolicyType, userId?: string) => Promise; + getAll: (policyType: PolicyType) => Promise; /** - * @returns true if the {@link PolicyType} applies to the current user, otherwise false. - * @remarks A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * @returns true if a policy of the specified type applies to the active user, otherwise false. + * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). + * This does not take into account the policy's configuration - if that is important, use {@link getAll$} to get the + * {@link Policy} objects and then filter by Policy.data. */ - policyAppliesToActiveUser$: ( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - ) => Observable; + policyAppliesToActiveUser$: (policyType: PolicyType) => Observable; - /** - * @returns true if the {@link PolicyType} applies to the specified user, otherwise false. - * @remarks A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). - * @see {@link policyAppliesToActiveUser$} if you only want to know about the current user. - */ - policyAppliesToUser: ( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) => Promise; + policyAppliesToUser: (policyType: PolicyType) => Promise; // Policy specific interfaces @@ -93,7 +90,7 @@ export abstract class PolicyService { } export abstract class InternalPolicyService extends PolicyService { - upsert: (policy: PolicyData) => Promise; + upsert: (policy: PolicyData) => Promise; replace: (policies: { [id: string]: PolicyData }) => Promise; - clear: (userId?: string) => Promise; + clear: (userId?: string) => Promise; } diff --git a/libs/common/src/admin-console/models/data/policy.data.ts b/libs/common/src/admin-console/models/data/policy.data.ts index 21ed810952f..35846f20726 100644 --- a/libs/common/src/admin-console/models/data/policy.data.ts +++ b/libs/common/src/admin-console/models/data/policy.data.ts @@ -1,8 +1,9 @@ +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyResponse } from "../response/policy.response"; export class PolicyData { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: Record; diff --git a/libs/common/src/admin-console/models/domain/policy.ts b/libs/common/src/admin-console/models/domain/policy.ts index 70e9e537047..65af09b5888 100644 --- a/libs/common/src/admin-console/models/domain/policy.ts +++ b/libs/common/src/admin-console/models/domain/policy.ts @@ -1,9 +1,10 @@ import Domain from "../../../platform/models/domain/domain-base"; +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../data/policy.data"; export class Policy extends Domain { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: any; diff --git a/libs/common/src/admin-console/models/response/policy.response.ts b/libs/common/src/admin-console/models/response/policy.response.ts index c3c4a5752b8..25a1f208a0b 100644 --- a/libs/common/src/admin-console/models/response/policy.response.ts +++ b/libs/common/src/admin-console/models/response/policy.response.ts @@ -1,8 +1,9 @@ import { BaseResponse } from "../../../models/response/base.response"; +import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; export class PolicyResponse extends BaseResponse { - id: string; + id: PolicyId; organizationId: string; type: PolicyType; data: any; diff --git a/libs/common/src/admin-console/services/policy/policy.service.spec.ts b/libs/common/src/admin-console/services/policy/policy.service.spec.ts index 9d585114e0f..672a53d34e5 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.spec.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.spec.ts @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom } from "rxjs"; +import { firstValueFrom, of } from "rxjs"; import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeActiveUserState } from "../../../../spec/fake-state"; @@ -19,71 +19,51 @@ import { ResetPasswordPolicyOptions } from "../../../admin-console/models/domain import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; import { POLICIES, PolicyService } from "../../../admin-console/services/policy/policy.service"; import { ListResponse } from "../../../models/response/list.response"; -import { CryptoService } from "../../../platform/abstractions/crypto.service"; -import { EncryptService } from "../../../platform/abstractions/encrypt.service"; -import { ContainerService } from "../../../platform/services/container.service"; -import { StateService } from "../../../platform/services/state.service"; import { PolicyId, UserId } from "../../../types/guid"; describe("PolicyService", () => { - let policyService: PolicyService; - - let cryptoService: MockProxy; - let stateService: MockProxy; let stateProvider: FakeStateProvider; let organizationService: MockProxy; - let encryptService: MockProxy; - let activeAccount: BehaviorSubject; - let activeAccountUnlocked: BehaviorSubject; + let activeUserState: FakeActiveUserState>; + + let policyService: PolicyService; beforeEach(() => { - stateService = mock(); - const accountService = mockAccountServiceWith("userId" as UserId); stateProvider = new FakeStateProvider(accountService); organizationService = mock(); - organizationService.getAll - .calledWith("user") - .mockResolvedValue([ - new Organization( - organizationData( - "test-organization", - true, - true, - OrganizationUserStatusType.Accepted, - false, - ), - ), - ]); - organizationService.getAll.calledWith(undefined).mockResolvedValue([]); - organizationService.getAll.calledWith(null).mockResolvedValue([]); - activeAccount = new BehaviorSubject("123"); - activeAccountUnlocked = new BehaviorSubject(true); - stateService.getDecryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue(null); - stateService.getEncryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue({ - "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { - minutes: 14, - }), - }); - stateService.getEncryptedPolicies.mockResolvedValue({ - "1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { - minutes: 14, - }), - }); - stateService.activeAccount$ = activeAccount; - stateService.activeAccountUnlocked$ = activeAccountUnlocked; - stateService.getUserId.mockResolvedValue("user"); - (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService); - policyService = new PolicyService(stateService, stateProvider, organizationService); - }); + activeUserState = stateProvider.activeUser.getFake(POLICIES); + organizationService.organizations$ = of([ + // User + organization("org1", true, true, OrganizationUserStatusType.Confirmed, false), + // Owner + organization( + "org2", + true, + true, + OrganizationUserStatusType.Confirmed, + false, + OrganizationUserType.Owner, + ), + // Does not use policies + organization("org3", true, false, OrganizationUserStatusType.Confirmed, false), + // Another User + organization("org4", true, true, OrganizationUserStatusType.Confirmed, false), + // Another User + organization("org5", true, true, OrganizationUserStatusType.Confirmed, false), + ]); - afterEach(() => { - activeAccount.complete(); - activeAccountUnlocked.complete(); + policyService = new PolicyService(stateProvider, organizationService); }); it("upsert", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { minutes: 14 }), + ]), + ); + await policyService.upsert(policyData("99", "test-organization", PolicyType.DisableSend, true)); expect(await firstValueFrom(policyService.policies$)).toEqual([ @@ -104,6 +84,12 @@ describe("PolicyService", () => { }); it("replace", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { minutes: 14 }), + ]), + ); + await policyService.replace({ "2": policyData("2", "test-organization", PolicyType.DisableSend, true), }); @@ -118,37 +104,63 @@ describe("PolicyService", () => { ]); }); - it("locking should clear", async () => { - activeAccountUnlocked.next(false); - // Sleep for 100ms to avoid timing issues - await new Promise((r) => setTimeout(r, 100)); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); - }); - describe("clear", () => { - it("null userId", async () => { + beforeEach(() => { + activeUserState.nextState( + arrayToRecord([ + policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, { + minutes: 14, + }), + ]), + ); + }); + + it("clears state for the active user", async () => { await policyService.clear(); - expect(stateService.setEncryptedPolicies).toBeCalledTimes(1); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); + expect(await firstValueFrom(policyService.policies$)).toEqual([]); + expect(await firstValueFrom(activeUserState.state$)).toEqual(null); + expect(stateProvider.activeUser.getFake(POLICIES).nextMock).toHaveBeenCalledWith([ + "userId", + null, + ]); }); - it("matching userId", async () => { - await policyService.clear("user"); + it("clears state for an inactive user", async () => { + const inactiveUserId = "someOtherUserId" as UserId; + const inactiveUserState = stateProvider.singleUser.getFake(inactiveUserId, POLICIES); + inactiveUserState.nextState( + arrayToRecord([ + policyData("10", "another-test-organization", PolicyType.PersonalOwnership, true), + ]), + ); - expect(stateService.setEncryptedPolicies).toBeCalledTimes(1); + await policyService.clear(inactiveUserId); - expect((await firstValueFrom(policyService.policies$)).length).toBe(0); - }); + // Active user is not affected + const expectedActiveUserPolicy: Partial = { + id: "1" as PolicyId, + organizationId: "test-organization", + type: PolicyType.MaximumVaultTimeout, + enabled: true, + data: { minutes: 14 }, + }; + expect(await firstValueFrom(policyService.policies$)).toEqual([expectedActiveUserPolicy]); + expect(await firstValueFrom(activeUserState.state$)).toEqual({ + "1": expectedActiveUserPolicy, + }); + expect(stateProvider.activeUser.getFake(POLICIES).nextMock).not.toHaveBeenCalled(); - it("mismatching userId", async () => { - await policyService.clear("12"); - - expect(stateService.setEncryptedPolicies).toBeCalledTimes(1); - - expect((await firstValueFrom(policyService.policies$)).length).toBe(1); + // Non-active user is cleared + expect( + await firstValueFrom( + policyService.getAll$(PolicyType.PersonalOwnership, "someOtherUserId" as UserId), + ), + ).toEqual([]); + expect(await firstValueFrom(inactiveUserState.state$)).toEqual(null); + expect( + stateProvider.singleUser.getFake("someOtherUserId" as UserId, POLICIES).nextMock, + ).toHaveBeenCalledWith(null); }); }); @@ -313,300 +325,260 @@ describe("PolicyService", () => { }); }); + describe("get$", () => { + it("returns the specified PolicyType", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual({ + id: "policy2", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }); + }); + + it("does not return disabled policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, false), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toBeNull(); + }); + + it("does not return policies that do not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org1", PolicyType.ActivateAutofill, true), + policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, false), + ]), + ); + + const result = await firstValueFrom( + policyService.get$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toBeNull(); + }); + + it("does not return policies for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org3", PolicyType.ActivateAutofill, true), + policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom(policyService.get$(PolicyType.ActivateAutofill)); + + expect(result).toBeNull(); + }); + }); + + describe("getAll$", () => { + it("returns the specified PolicyTypes", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy3", + organizationId: "org5", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return disabled policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return policies that do not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy3", + organizationId: "org5", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + + it("does not return policies for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), + ); + + const result = await firstValueFrom( + policyService.getAll$(PolicyType.DisablePersonalVaultExport), + ); + + expect(result).toEqual([ + { + id: "policy1", + organizationId: "org4", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + { + id: "policy4", + organizationId: "org1", + type: PolicyType.DisablePersonalVaultExport, + enabled: true, + }, + ]); + }); + }); + describe("policyAppliesToActiveUser$", () => { - it("MasterPassword does not apply", async () => { - const result = await firstValueFrom( - policyService.policyAppliesToActiveUser$(PolicyType.MasterPassword), + it("returns true when the policyType applies to the user", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), + policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), + ]), ); - expect(result).toEqual(false); - }); - - it("MaximumVaultTimeout applies", async () => { - const result = await firstValueFrom( - policyService.policyAppliesToActiveUser$(PolicyType.MaximumVaultTimeout), - ); - - expect(result).toEqual(true); - }); - - it("PolicyFilter filters result", async () => { - const result = await firstValueFrom( - policyService.policyAppliesToActiveUser$(PolicyType.MaximumVaultTimeout, (p) => false), - ); - - expect(result).toEqual(false); - }); - - it("DisablePersonalVaultExport does not apply", async () => { const result = await firstValueFrom( policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), ); - expect(result).toEqual(false); + expect(result).toBe(true); }); - }); - describe("policyAppliesToUser", () => { - it("MasterPassword does not apply", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MasterPassword, - null, - "user", + it("returns false when policyType is disabled", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled + ]), ); - expect(result).toEqual(false); - }); - - it("MaximumVaultTimeout applies", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MaximumVaultTimeout, - null, - "user", + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), ); - expect(result).toEqual(true); + expect(result).toBe(false); }); - it("PolicyFilter filters result", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.MaximumVaultTimeout, - (p) => false, - "user", + it("returns false when the policyType does not apply to the user because the user's role is exempt", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner + ]), ); - expect(result).toEqual(false); - }); - - it("DisablePersonalVaultExport does not apply", async () => { - const result = await policyService.policyAppliesToUser( - PolicyType.DisablePersonalVaultExport, - null, - "user", + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), ); - expect(result).toEqual(false); - }); - }); - - // TODO: remove this nesting once fully migrated to StateProvider - describe("stateProvider methods", () => { - let policyState$: FakeActiveUserState>; - - beforeEach(() => { - policyState$ = stateProvider.activeUser.getFake(POLICIES); - organizationService.organizations$ = new BehaviorSubject([ - // User - organization("org1", true, true, OrganizationUserStatusType.Confirmed, false), - // Owner - organization( - "org2", - true, - true, - OrganizationUserStatusType.Confirmed, - false, - OrganizationUserType.Owner, - ), - // Does not use policies - organization("org3", true, false, OrganizationUserStatusType.Confirmed, false), - // Another User - organization("org4", true, true, OrganizationUserStatusType.Confirmed, false), - // Another User - organization("org5", true, true, OrganizationUserStatusType.Confirmed, false), - ]); + expect(result).toBe(false); }); - describe("get_vNext$", () => { - it("returns the specified PolicyType", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); + it("returns false for organizations that do not use policies", async () => { + activeUserState.nextState( + arrayToRecord([ + policyData("policy2", "org1", PolicyType.ActivateAutofill, true), + policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies + ]), + ); - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); + const result = await firstValueFrom( + policyService.policyAppliesToActiveUser$(PolicyType.DisablePersonalVaultExport), + ); - expect(result).toEqual({ - id: "policy2", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }); - }); - - it("does not return disabled policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org1", PolicyType.DisablePersonalVaultExport, false), - ]), - ); - - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toBeNull(); - }); - - it("does not return policies that do not apply to the user because the user's role is exempt", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org1", PolicyType.ActivateAutofill, true), - policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, false), - ]), - ); - - const result = await firstValueFrom( - policyService.get_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toBeNull(); - }); - - it("does not return policies for organizations that do not use policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org3", PolicyType.ActivateAutofill, true), - policyData("policy2", "org2", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom(policyService.get_vNext$(PolicyType.ActivateAutofill)); - - expect(result).toBeNull(); - }); - }); - - describe("getAll_vNext$", () => { - it("returns the specified PolicyTypes", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy3", - organizationId: "org5", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return disabled policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, false), // disabled - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return policies that do not apply to the user because the user's role is exempt", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org5", PolicyType.DisablePersonalVaultExport, true), - policyData("policy4", "org2", PolicyType.DisablePersonalVaultExport, true), // owner - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy3", - organizationId: "org5", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); - - it("does not return policies for organizations that do not use policies", async () => { - policyState$.nextState( - arrayToRecord([ - policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), - policyData("policy2", "org1", PolicyType.ActivateAutofill, true), - policyData("policy3", "org3", PolicyType.DisablePersonalVaultExport, true), // does not use policies - policyData("policy4", "org1", PolicyType.DisablePersonalVaultExport, true), - ]), - ); - - const result = await firstValueFrom( - policyService.getAll_vNext$(PolicyType.DisablePersonalVaultExport), - ); - - expect(result).toEqual([ - { - id: "policy1", - organizationId: "org4", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - { - id: "policy4", - organizationId: "org1", - type: PolicyType.DisablePersonalVaultExport, - enabled: true, - }, - ]); - }); + expect(result).toBe(false); }); }); @@ -618,7 +590,7 @@ describe("PolicyService", () => { data?: any, ) { const policyData = new PolicyData({} as any); - policyData.id = id; + policyData.id = id as PolicyId; policyData.organizationId = organizationId; policyData.type = type; policyData.enabled = enabled; diff --git a/libs/common/src/admin-console/services/policy/policy.service.ts b/libs/common/src/admin-console/services/policy/policy.service.ts index 2f8c8ed8025..4672d9b810e 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.ts @@ -1,8 +1,6 @@ -import { BehaviorSubject, combineLatest, concatMap, map, Observable, of } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, of } from "rxjs"; import { ListResponse } from "../../../models/response/list.response"; -import { StateService } from "../../../platform/abstractions/state.service"; -import { Utils } from "../../../platform/misc/utils"; import { KeyDefinition, POLICIES_DISK, StateProvider } from "../../../platform/state"; import { PolicyId, UserId } from "../../../types/guid"; import { OrganizationService } from "../../abstractions/organization/organization.service.abstraction"; @@ -23,42 +21,19 @@ export const POLICIES = KeyDefinition.record(POLICIES_DISK }); export class PolicyService implements InternalPolicyServiceAbstraction { - protected _policies: BehaviorSubject = new BehaviorSubject([]); - - policies$ = this._policies.asObservable(); - private activeUserPolicyState = this.stateProvider.getActive(POLICIES); - activeUserPolicies$ = this.activeUserPolicyState.state$.pipe( + private activeUserPolicies$ = this.activeUserPolicyState.state$.pipe( map((policyData) => policyRecordToArray(policyData)), ); + policies$ = this.activeUserPolicies$; + constructor( - protected stateService: StateService, private stateProvider: StateProvider, private organizationService: OrganizationService, - ) { - this.stateService.activeAccountUnlocked$ - .pipe( - concatMap(async (unlocked) => { - if (Utils.global.bitwardenContainerService == null) { - return; - } + ) {} - if (!unlocked) { - this._policies.next([]); - return; - } - - const data = await this.stateService.getEncryptedPolicies(); - - await this.updateObservables(data); - }), - ) - .subscribe(); - } - - // --- StateProvider methods - not yet wired up - get_vNext$(policyType: PolicyType) { + get$(policyType: PolicyType) { const filteredPolicies$ = this.activeUserPolicies$.pipe( map((policies) => policies.filter((p) => p.type === policyType)), ); @@ -71,7 +46,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - getAll_vNext$(policyType: PolicyType, userId?: UserId) { + getAll$(policyType: PolicyType, userId?: UserId) { const filteredPolicies$ = this.stateProvider.getUserState$(POLICIES, userId).pipe( map((policyData) => policyRecordToArray(policyData)), map((policies) => policies.filter((p) => p.type === policyType)), @@ -82,8 +57,18 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - policyAppliesToActiveUser_vNext$(policyType: PolicyType) { - return this.get_vNext$(policyType).pipe(map((policy) => policy != null)); + async getAll(policyType: PolicyType) { + return await firstValueFrom( + this.policies$.pipe(map((policies) => policies.filter((p) => p.type === policyType))), + ); + } + + policyAppliesToActiveUser$(policyType: PolicyType) { + return this.get$(policyType).pipe(map((policy) => policy != null)); + } + + async policyAppliesToUser(policyType: PolicyType) { + return await firstValueFrom(this.policyAppliesToActiveUser$(policyType)); } private enforcedPolicyFilter(policies: Policy[], organizations: Organization[]) { @@ -105,45 +90,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); }); } - // --- End StateProvider methods - - get$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean): Observable { - return this.policies$.pipe( - concatMap(async (policies) => { - const userId = await this.stateService.getUserId(); - const appliesToCurrentUser = await this.checkPoliciesThatApplyToUser( - policies, - policyType, - policyFilter, - userId, - ); - if (appliesToCurrentUser) { - return policies.find((policy) => policy.type === policyType && policy.enabled); - } - }), - ); - } - - async getAll(type?: PolicyType, userId?: string): Promise { - let response: Policy[] = []; - const decryptedPolicies = await this.stateService.getDecryptedPolicies({ userId: userId }); - if (decryptedPolicies != null) { - response = decryptedPolicies; - } else { - const diskPolicies = await this.stateService.getEncryptedPolicies({ userId: userId }); - for (const id in diskPolicies) { - if (Object.prototype.hasOwnProperty.call(diskPolicies, id)) { - response.push(new Policy(diskPolicies[id])); - } - } - await this.stateService.setDecryptedPolicies(response, { userId: userId }); - } - if (type != null) { - return response.filter((policy) => policy.type === type); - } else { - return response; - } - } masterPasswordPolicyOptions$(policies?: Policy[]): Observable { const observable = policies ? of(policies) : this.policies$; @@ -205,15 +151,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { ); } - policyAppliesToActiveUser$(policyType: PolicyType, policyFilter?: (policy: Policy) => boolean) { - return this.policies$.pipe( - concatMap(async (policies) => { - const userId = await this.stateService.getUserId(); - return await this.checkPoliciesThatApplyToUser(policies, policyType, policyFilter, userId); - }), - ); - } - evaluateMasterPassword( passwordStrength: number, newPassword: string, @@ -288,68 +225,20 @@ export class PolicyService implements InternalPolicyServiceAbstraction { return policiesResponse.data.map((response) => this.mapPolicyFromResponse(response)); } - async policyAppliesToUser( - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) { - const policies = await this.getAll(policyType, userId); - - return this.checkPoliciesThatApplyToUser(policies, policyType, policyFilter, userId); - } - - async upsert(policy: PolicyData): Promise { - let policies = await this.stateService.getEncryptedPolicies(); - if (policies == null) { - policies = {}; - } - - policies[policy.id] = policy; - - await this.updateObservables(policies); - await this.stateService.setDecryptedPolicies(null); - await this.stateService.setEncryptedPolicies(policies); + async upsert(policy: PolicyData): Promise { + await this.activeUserPolicyState.update((policies) => { + policies ??= {}; + policies[policy.id] = policy; + return policies; + }); } async replace(policies: { [id: string]: PolicyData }): Promise { - await this.updateObservables(policies); - await this.stateService.setDecryptedPolicies(null); - await this.stateService.setEncryptedPolicies(policies); + await this.activeUserPolicyState.update(() => policies); } - async clear(userId?: string): Promise { - if (userId == null || userId == (await this.stateService.getUserId())) { - this._policies.next([]); - } - await this.stateService.setDecryptedPolicies(null, { userId: userId }); - await this.stateService.setEncryptedPolicies(null, { userId: userId }); - } - - private async updateObservables(policiesMap: { [id: string]: PolicyData }) { - const policies = Object.values(policiesMap || {}).map((f) => new Policy(f)); - - this._policies.next(policies); - } - - private async checkPoliciesThatApplyToUser( - policies: Policy[], - policyType: PolicyType, - policyFilter?: (policy: Policy) => boolean, - userId?: string, - ) { - const organizations = await this.organizationService.getAll(userId); - const filteredPolicies = policies.filter( - (p) => p.type === policyType && p.enabled && (policyFilter == null || policyFilter(p)), - ); - const policySet = new Set(filteredPolicies.map((p) => p.organizationId)); - - return organizations.some( - (o) => - o.status >= OrganizationUserStatusType.Accepted && - o.usePolicies && - policySet.has(o.id) && - !this.isExemptFromPolicy(policyType, o), - ); + async clear(userId?: UserId): Promise { + await this.stateProvider.setUserState(POLICIES, null, userId); } /** diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 18b2f79483b..40617694619 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -1,8 +1,6 @@ import { Observable } from "rxjs"; import { OrganizationData } from "../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../admin-console/models/data/policy.data"; -import { Policy } from "../../admin-console/models/domain/policy"; import { AdminAuthRequestStorable } from "../../auth/models/domain/admin-auth-req-storable"; import { ForceSetPasswordReason } from "../../auth/models/domain/force-set-password-reason"; import { KdfConfig } from "../../auth/models/domain/kdf-config"; @@ -181,14 +179,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use setDecryptedUserKeyPin instead */ setDecryptedPinProtected: (value: EncString, options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this, use PolicyService - */ - getDecryptedPolicies: (options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this, use PolicyService - */ - setDecryptedPolicies: (value: Policy[], options?: StorageOptions) => Promise; /** * @deprecated Do not call this directly, use SendService */ @@ -279,17 +269,6 @@ export abstract class StateService { * @deprecated For migration purposes only, use setEncryptedUserKeyPin instead */ setEncryptedPinProtected: (value: string, options?: StorageOptions) => Promise; - /** - * @deprecated Do not call this directly, use PolicyService - */ - getEncryptedPolicies: (options?: StorageOptions) => Promise<{ [id: string]: PolicyData }>; - /** - * @deprecated Do not call this directly, use PolicyService - */ - setEncryptedPolicies: ( - value: { [id: string]: PolicyData }, - options?: StorageOptions, - ) => Promise; /** * @deprecated Do not call this directly, use SendService */ diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 2b2024270ff..32b1de9c8f9 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -1,8 +1,6 @@ import { Jsonify } from "type-fest"; import { OrganizationData } from "../../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../../admin-console/models/data/policy.data"; -import { Policy } from "../../../admin-console/models/domain/policy"; import { AdminAuthRequestStorable } from "../../../auth/models/domain/admin-auth-req-storable"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { KeyConnectorUserDecryptionOption } from "../../../auth/models/domain/user-decryption-options/key-connector-user-decryption-option"; @@ -87,7 +85,6 @@ export class AccountData { >(); localData?: any; sends?: DataEncryptionPair = new DataEncryptionPair(); - policies?: DataEncryptionPair = new DataEncryptionPair(); passwordGenerationHistory?: EncryptionPair< GeneratedPasswordHistory[], GeneratedPasswordHistory[] diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 67c71c610d9..357bd27173f 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -2,8 +2,6 @@ import { BehaviorSubject, Observable, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { OrganizationData } from "../../admin-console/models/data/organization.data"; -import { PolicyData } from "../../admin-console/models/data/policy.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"; @@ -799,24 +797,6 @@ export class StateService< ); } - @withPrototypeForArrayMembers(Policy) - async getDecryptedPolicies(options?: StorageOptions): Promise { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions())) - )?.data?.policies?.decrypted; - } - - async setDecryptedPolicies(value: Policy[], options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - account.data.policies.decrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultInMemoryOptions()), - ); - } - @withPrototypeForArrayMembers(SendView) async getDecryptedSends(options?: StorageOptions): Promise { return ( @@ -1350,27 +1330,6 @@ export class StateService< ); } - @withPrototypeForObjectValues(PolicyData) - async getEncryptedPolicies(options?: StorageOptions): Promise<{ [id: string]: PolicyData }> { - return ( - await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) - )?.data?.policies?.encrypted; - } - - async setEncryptedPolicies( - value: { [id: string]: PolicyData }, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.data.policies.encrypted = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - @withPrototypeForObjectValues(SendData) async getEncryptedSends(options?: StorageOptions): Promise<{ [id: string]: SendData }> { return ( diff --git a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts index f1881080d9c..7eee4567754 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts @@ -110,9 +110,8 @@ describe("VaultTimeoutSettingsService", () => { stateService.getAccountDecryptionOptions.mockResolvedValue( new AccountDecryptionOptions({ hasMasterPassword: true }), ); - policyService.policyAppliesToUser.mockResolvedValue(policy === null ? false : true); - policyService.getAll.mockResolvedValue( - policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[]), + policyService.getAll$.mockReturnValue( + of(policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[])), ); stateService.getVaultTimeoutAction.mockResolvedValue(userPreference); @@ -140,9 +139,8 @@ describe("VaultTimeoutSettingsService", () => { stateService.getAccountDecryptionOptions.mockResolvedValue( new AccountDecryptionOptions({ hasMasterPassword: false }), ); - policyService.policyAppliesToUser.mockResolvedValue(policy === null ? false : true); - policyService.getAll.mockResolvedValue( - policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[]), + policyService.getAll$.mockReturnValue( + of(policy === null ? [] : ([{ data: { action: policy } }] as unknown as Policy[])), ); stateService.getVaultTimeoutAction.mockResolvedValue(userPreference); diff --git a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts index e84d561fe6a..0d0eb508cb2 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts @@ -84,18 +84,18 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return await biometricUnlockPromise; } - async getVaultTimeout(userId?: string): Promise { + async getVaultTimeout(userId?: UserId): Promise { const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); + const policies = await firstValueFrom( + this.policyService.getAll$(PolicyType.MaximumVaultTimeout, userId), + ); - if ( - await this.policyService.policyAppliesToUser(PolicyType.MaximumVaultTimeout, null, userId) - ) { - const policy = await this.policyService.getAll(PolicyType.MaximumVaultTimeout, userId); + if (policies?.length) { // Remove negative values, and ensure it's smaller than maximum allowed value according to policy - let timeout = Math.min(vaultTimeout, policy[0].data.minutes); + let timeout = Math.min(vaultTimeout, policies[0].data.minutes); if (vaultTimeout == null || timeout < 0) { - timeout = policy[0].data.minutes; + timeout = policies[0].data.minutes; } // TODO @jlf0dev: Can we move this somwhere else? Maybe add it to the initialization process? @@ -111,23 +111,23 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return vaultTimeout; } - vaultTimeoutAction$(userId?: string) { + vaultTimeoutAction$(userId?: UserId) { return defer(() => this.getVaultTimeoutAction(userId)); } - async getVaultTimeoutAction(userId?: string): Promise { + async getVaultTimeoutAction(userId?: UserId): Promise { const availableActions = await this.getAvailableVaultTimeoutActions(); if (availableActions.length === 1) { return availableActions[0]; } const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId: userId }); + const policies = await firstValueFrom( + this.policyService.getAll$(PolicyType.MaximumVaultTimeout, userId), + ); - if ( - await this.policyService.policyAppliesToUser(PolicyType.MaximumVaultTimeout, null, userId) - ) { - const policy = await this.policyService.getAll(PolicyType.MaximumVaultTimeout, userId); - const action = policy[0].data.action; + if (policies?.length) { + const action = policies[0].data.action; // We really shouldn't need to set the value here, but multiple services relies on this value being correct. if (action && vaultTimeoutAction !== action) { await this.stateService.setVaultTimeoutAction(action, { userId: userId }); diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 3b217135394..db29ddf26fe 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -25,6 +25,7 @@ import { BadgeSettingsMigrator } from "./migrations/27-move-badge-settings-to-st import { MoveBiometricUnlockToStateProviders } from "./migrations/28-move-biometric-unlock-to-state-providers"; import { UserNotificationSettingsKeyMigrator } from "./migrations/29-move-user-notification-settings-to-state-provider"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; +import { PolicyMigrator } from "./migrations/30-move-policy-state-to-state-provider"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; @@ -34,7 +35,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 29; +export const CURRENT_VERSION = 30; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -66,7 +67,8 @@ export function createMigrationBuilder() { .with(RevertLastSyncMigrator, 25, 26) .with(BadgeSettingsMigrator, 26, 27) .with(MoveBiometricUnlockToStateProviders, 27, 28) - .with(UserNotificationSettingsKeyMigrator, 28, CURRENT_VERSION); + .with(UserNotificationSettingsKeyMigrator, 28, 29) + .with(PolicyMigrator, 29, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts new file mode 100644 index 00000000000..23ed3778a72 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.spec.ts @@ -0,0 +1,192 @@ +import { MockProxy, any } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { PolicyMigrator } from "./30-move-policy-state-to-state-provider"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + policies: { + encrypted: { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, // max vault timeout + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, // single org + enabled: true, + }, + }, + }, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_policies_policies": { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }, + "user_user-2_policies_policies": null as any, + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2"], + "user-1": { + data: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + data: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +describe("PoliciesMigrator", () => { + let helper: MockProxy; + let sut: PolicyMigrator; + const keyDefinitionLike = { + key: "policies", + stateDefinition: { + name: "policies", + }, + }; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 22); + sut = new PolicyMigrator(29, 30); + }); + + it("should remove policies from all old accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should set policies value in StateProvider framework for each account", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", keyDefinitionLike, { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 23); + sut = new PolicyMigrator(29, 30); + }); + + it.each(["user-1", "user-2"])("should null out new values", async (userId) => { + await sut.rollback(helper); + expect(helper.setToUser).toHaveBeenCalledWith(userId, keyDefinitionLike, null); + }); + + it("should add policy values back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalled(); + expect(helper.set).toHaveBeenCalledWith("user-1", { + data: { + policies: { + encrypted: { + "policy-1": { + id: "policy-1", + organizationId: "fe1ff6ef-d2d4-49f3-9c07-b0c7013998f9", + type: 9, + enabled: true, + data: { + hours: 1, + minutes: 30, + action: "lock", + }, + }, + "policy-2": { + id: "policy-2", + organizationId: "5f277723-6391-4b5c-add9-b0c200ee6967", + type: 3, + enabled: true, + }, + }, + }, + otherStuff: "otherStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should not try to restore values to missing accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith("user-3", any()); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts new file mode 100644 index 00000000000..5f05442c4e1 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/30-move-policy-state-to-state-provider.ts @@ -0,0 +1,76 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +enum PolicyType { + TwoFactorAuthentication = 0, // Requires users to have 2fa enabled + MasterPassword = 1, // Sets minimum requirements for master password complexity + PasswordGenerator = 2, // Sets minimum requirements/default type for generated passwords/passphrases + SingleOrg = 3, // Allows users to only be apart of one organization + RequireSso = 4, // Requires users to authenticate with SSO + PersonalOwnership = 5, // Disables personal vault ownership for adding/cloning items + DisableSend = 6, // Disables the ability to create and edit Bitwarden Sends + SendOptions = 7, // Sets restrictions or defaults for Bitwarden Sends + ResetPassword = 8, // Allows orgs to use reset password : also can enable auto-enrollment during invite flow + MaximumVaultTimeout = 9, // Sets the maximum allowed vault timeout + DisablePersonalVaultExport = 10, // Disable personal vault export + ActivateAutofill = 11, // Activates autofill with page load on the browser extension +} + +type PolicyDataType = { + id: string; + organizationId: string; + type: PolicyType; + data: Record; + enabled: boolean; +}; + +type ExpectedAccountType = { + data?: { + policies?: { + encrypted?: Record; + }; + }; +}; + +const POLICIES_KEY: KeyDefinitionLike = { + key: "policies", + stateDefinition: { + name: "policies", + }, +}; + +export class PolicyMigrator extends Migrator<29, 30> { + async migrate(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function migrateAccount(userId: string, account: ExpectedAccountType): Promise { + const value = account?.data?.policies?.encrypted; + if (value != null) { + await helper.setToUser(userId, POLICIES_KEY, value); + delete account.data.policies; + await helper.set(userId, account); + } + } + + await Promise.all(accounts.map(({ userId, account }) => migrateAccount(userId, account))); + } + + async rollback(helper: MigrationHelper): Promise { + const accounts = await helper.getAccounts(); + + async function rollbackAccount(userId: string, account: ExpectedAccountType): Promise { + const value = await helper.getFromUser(userId, POLICIES_KEY); + if (account) { + account.data = Object.assign(account.data ?? {}, { + policies: { + encrypted: value, + }, + }); + + await helper.set(userId, account); + } + await helper.setToUser(userId, POLICIES_KEY, null); + } + await Promise.all(accounts.map(({ userId, account }) => rollbackAccount(userId, account))); + } +} From f83dcf2b24665780089f9300defffaeb16e5f2f5 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 7 Mar 2024 19:41:56 -0600 Subject: [PATCH 07/31] Move fingerprint validated to biometric state povider (#8058) --- .../browser/src/background/main.background.ts | 1 + .../background/nativeMessaging.background.ts | 12 ++------ .../src/popup/settings/settings.component.ts | 2 +- .../src/app/services/services.module.ts | 1 + .../biometric-state.service.spec.ts | 30 +++++++++++++++++++ .../biometrics/biometric-state.service.ts | 21 ++++++++++++- .../biometrics/biometric.state.spec.ts | 4 ++- .../platform/biometrics/biometric.state.ts | 11 +++++++ .../src/platform/services/system.service.ts | 7 +++-- 9 files changed, 74 insertions(+), 15 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index c5608dc830e..ea51fa8805d 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -814,6 +814,7 @@ export default class MainBackground { this.stateService, this.autofillSettingsService, this.vaultTimeoutSettingsService, + this.biometricStateService, ); // Other fields diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index e4fb46d960f..2717a7b2b56 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -84,10 +84,6 @@ export class NativeMessagingBackground { private authService: AuthService, private biometricStateService: BiometricStateService, ) { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.stateService.setBiometricFingerprintValidated(false); - if (chrome?.permissions?.onAdded) { // Reload extension to activate nativeMessaging chrome.permissions.onAdded.addListener((permissions) => { @@ -100,9 +96,7 @@ export class NativeMessagingBackground { async connect() { this.appId = await this.appIdService.getAppId(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.stateService.setBiometricFingerprintValidated(false); + await this.biometricStateService.setFingerprintValidated(false); return new Promise((resolve, reject) => { this.port = BrowserApi.connectNative("com.8bit.bitwarden"); @@ -148,9 +142,7 @@ export class NativeMessagingBackground { if (this.validatingFingerprint) { this.validatingFingerprint = false; - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.stateService.setBiometricFingerprintValidated(true); + await this.biometricStateService.setFingerprintValidated(true); } this.sharedSecret = new SymmetricCryptoKey(decrypted); this.secureSetupResolve(); diff --git a/apps/browser/src/popup/settings/settings.component.ts b/apps/browser/src/popup/settings/settings.component.ts index f622cffd3e4..c83c7b5e72c 100644 --- a/apps/browser/src/popup/settings/settings.component.ts +++ b/apps/browser/src/popup/settings/settings.component.ts @@ -415,7 +415,7 @@ export class SettingsComponent implements OnInit { ]); } else { await this.biometricStateService.setBiometricUnlockEnabled(false); - await this.stateService.setBiometricFingerprintValidated(false); + await this.biometricStateService.setFingerprintValidated(false); } } diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 2fde9744b90..3b50b6ced7a 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -126,6 +126,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); StateServiceAbstraction, AutofillSettingsServiceAbstraction, VaultTimeoutSettingsService, + BiometricStateService, ], }, { diff --git a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts index 54471a25cd0..716ad627c12 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts @@ -12,6 +12,7 @@ import { BIOMETRIC_UNLOCK_ENABLED, DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, ENCRYPTED_CLIENT_KEY_HALF, + FINGERPRINT_VALIDATED, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, REQUIRE_PASSWORD_ON_START, @@ -67,6 +68,19 @@ describe("BiometricStateService", () => { }); }); + describe("fingerprintValidated$", () => { + it("emits when the fingerprint validated state changes", async () => { + const state = stateProvider.global.getFake(FINGERPRINT_VALIDATED); + state.stateSubject.next(undefined); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toBe(false); + + state.stateSubject.next(true); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toEqual(true); + }); + }); + describe("setEncryptedClientKeyHalf", () => { it("updates encryptedClientKeyHalf$", async () => { await sut.setEncryptedClientKeyHalf(encClientKeyHalf); @@ -207,4 +221,20 @@ describe("BiometricStateService", () => { expect(await sut.getBiometricUnlockEnabled(userId)).toBe(false); }); }); + + describe("setFingerprintValidated", () => { + it("updates fingerprintValidated$", async () => { + await sut.setFingerprintValidated(true); + + expect(await firstValueFrom(sut.fingerprintValidated$)).toBe(true); + }); + + it("updates state", async () => { + await sut.setFingerprintValidated(true); + + expect(stateProvider.global.getFake(FINGERPRINT_VALIDATED).nextMock).toHaveBeenCalledWith( + true, + ); + }); + }); }); diff --git a/libs/common/src/platform/biometrics/biometric-state.service.ts b/libs/common/src/platform/biometrics/biometric-state.service.ts index b00090eb264..2047d137b53 100644 --- a/libs/common/src/platform/biometrics/biometric-state.service.ts +++ b/libs/common/src/platform/biometrics/biometric-state.service.ts @@ -2,7 +2,7 @@ import { Observable, firstValueFrom, map } from "rxjs"; import { UserId } from "../../types/guid"; import { EncryptedString, EncString } from "../models/domain/enc-string"; -import { ActiveUserState, StateProvider } from "../state"; +import { ActiveUserState, GlobalState, StateProvider } from "../state"; import { BIOMETRIC_UNLOCK_ENABLED, @@ -11,6 +11,7 @@ import { DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, + FINGERPRINT_VALIDATED, } from "./biometric.state"; export abstract class BiometricStateService { @@ -49,6 +50,10 @@ export abstract class BiometricStateService { * tracks the currently active user */ promptAutomatically$: Observable; + /** + * Whether or not IPC fingerprint has been validated by the user this session. + */ + fingerprintValidated$: Observable; /** * Updates the require password on start state for the currently active user. @@ -88,6 +93,11 @@ export abstract class BiometricStateService { * @param prompt Whether or not to prompt for biometrics on application start. */ abstract setPromptAutomatically(prompt: boolean): Promise; + /** + * Updates whether or not IPC has been validated by the user this session + * @param validated the value to save + */ + abstract setFingerprintValidated(validated: boolean): Promise; abstract logout(userId: UserId): Promise; } @@ -99,12 +109,14 @@ export class DefaultBiometricStateService implements BiometricStateService { private dismissedRequirePasswordOnStartCalloutState: ActiveUserState; private promptCancelledState: ActiveUserState; private promptAutomaticallyState: ActiveUserState; + private fingerprintValidatedState: GlobalState; biometricUnlockEnabled$: Observable; encryptedClientKeyHalf$: Observable; requirePasswordOnStart$: Observable; dismissedRequirePasswordOnStartCallout$: Observable; promptCancelled$: Observable; promptAutomatically$: Observable; + fingerprintValidated$: Observable; constructor(private stateProvider: StateProvider) { this.biometricUnlockEnabledState = this.stateProvider.getActive(BIOMETRIC_UNLOCK_ENABLED); @@ -130,6 +142,9 @@ export class DefaultBiometricStateService implements BiometricStateService { this.promptCancelled$ = this.promptCancelledState.state$.pipe(map(Boolean)); this.promptAutomaticallyState = this.stateProvider.getActive(PROMPT_AUTOMATICALLY); this.promptAutomatically$ = this.promptAutomaticallyState.state$.pipe(map(Boolean)); + + this.fingerprintValidatedState = this.stateProvider.getGlobal(FINGERPRINT_VALIDATED); + this.fingerprintValidated$ = this.fingerprintValidatedState.state$.pipe(map(Boolean)); } async setBiometricUnlockEnabled(enabled: boolean): Promise { @@ -207,6 +222,10 @@ export class DefaultBiometricStateService implements BiometricStateService { async setPromptAutomatically(prompt: boolean): Promise { await this.promptAutomaticallyState.update(() => prompt); } + + async setFingerprintValidated(validated: boolean): Promise { + await this.fingerprintValidatedState.update(() => validated); + } } function encryptedClientKeyHalfToEncString( diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts index 4f79f8da733..a3b110c77cf 100644 --- a/libs/common/src/platform/biometrics/biometric.state.spec.ts +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -5,6 +5,7 @@ import { BIOMETRIC_UNLOCK_ENABLED, DISMISSED_REQUIRE_PASSWORD_ON_START_CALLOUT, ENCRYPTED_CLIENT_KEY_HALF, + FINGERPRINT_VALIDATED, PROMPT_AUTOMATICALLY, PROMPT_CANCELLED, REQUIRE_PASSWORD_ON_START, @@ -16,7 +17,8 @@ describe.each([ [PROMPT_CANCELLED, true], [PROMPT_AUTOMATICALLY, true], [REQUIRE_PASSWORD_ON_START, true], - [BIOMETRIC_UNLOCK_ENABLED, "test"], + [BIOMETRIC_UNLOCK_ENABLED, true], + [FINGERPRINT_VALIDATED, true], ])( "deserializes state %s", ( diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts index 8796366c884..a5041ca8d01 100644 --- a/libs/common/src/platform/biometrics/biometric.state.ts +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -74,3 +74,14 @@ export const PROMPT_AUTOMATICALLY = new KeyDefinition( deserializer: (obj) => obj, }, ); + +/** + * Stores whether or not IPC handshake has been validated this session. + */ +export const FINGERPRINT_VALIDATED = new KeyDefinition( + BIOMETRIC_SETTINGS_DISK, + "fingerprintValidated", + { + deserializer: (obj) => obj, + }, +); diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index 06f9fcf8fb6..d19390c45e0 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -9,6 +9,7 @@ import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; import { SystemService as SystemServiceAbstraction } from "../abstractions/system.service"; +import { BiometricStateService } from "../biometrics/biometric-state.service"; import { Utils } from "../misc/utils"; export class SystemService implements SystemServiceAbstraction { @@ -23,6 +24,7 @@ export class SystemService implements SystemServiceAbstraction { private stateService: StateService, private autofillSettingsService: AutofillSettingsServiceAbstraction, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private biometricStateService: BiometricStateService, ) {} async startProcessReload(authService: AuthService): Promise { @@ -54,8 +56,9 @@ export class SystemService implements SystemServiceAbstraction { } private async executeProcessReload() { - const biometricLockedFingerprintValidated = - await this.stateService.getBiometricFingerprintValidated(); + const biometricLockedFingerprintValidated = await firstValueFrom( + this.biometricStateService.fingerprintValidated$, + ); if (!biometricLockedFingerprintValidated) { clearInterval(this.reloadInterval); this.reloadInterval = null; From 2d49a7985486a5f6e74f167152bf0dc28b61dd64 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 7 Mar 2024 19:43:19 -0600 Subject: [PATCH 08/31] Remove usage of `getBgService` (#8217) --- apps/browser/src/popup/services/services.module.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 40d68eb630f..b5138274d36 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -237,8 +237,9 @@ function getBgService(service: keyof MainBackground) { }, { provide: LogServiceAbstraction, - useFactory: getBgService("logService"), - deps: [], + useFactory: (platformUtilsService: PlatformUtilsService) => + new ConsoleLogService(platformUtilsService.isDev()), + deps: [PlatformUtilsService], }, { provide: BrowserEnvironmentService, From 551e43031e272ecc3852325cb56c7f3f82021631 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 8 Mar 2024 12:00:25 +0000 Subject: [PATCH 09/31] Autosync the updated translations (#8251) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- apps/browser/src/_locales/de/messages.json | 2 +- apps/browser/src/_locales/es/messages.json | 28 +++++++++++----------- apps/browser/src/_locales/ko/messages.json | 18 +++++++------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/apps/browser/src/_locales/de/messages.json b/apps/browser/src/_locales/de/messages.json index b377809ab1c..7a013add6e7 100644 --- a/apps/browser/src/_locales/de/messages.json +++ b/apps/browser/src/_locales/de/messages.json @@ -2709,7 +2709,7 @@ "message": "Starte DUO und folge den Schritten, um die Anmeldung zu abzuschließen." }, "duoRequiredForAccount": { - "message": "Duo two-step login is required for your account." + "message": "Für dein Konto ist die Duo Zwei-Faktor-Authentifizierung erforderlich." }, "popoutTheExtensionToCompleteLogin": { "message": "Popout the extension to complete login." diff --git a/apps/browser/src/_locales/es/messages.json b/apps/browser/src/_locales/es/messages.json index 471c1981d7a..58172b1a7af 100644 --- a/apps/browser/src/_locales/es/messages.json +++ b/apps/browser/src/_locales/es/messages.json @@ -2005,7 +2005,7 @@ "message": "Seleccione carpeta..." }, "noFoldersFound": { - "message": "No folders found", + "message": "Ninguna carpeta encontrada", "description": "Used as a message within the notification bar when no folders are found" }, "orgPermissionsUpdatedMustSetPassword": { @@ -2670,25 +2670,25 @@ "message": "Verificar biométricamente" }, "awaitingConfirmation": { - "message": "Awaiting confirmation" + "message": "Esperando confirmación" }, "couldNotCompleteBiometrics": { - "message": "Could not complete biometrics." + "message": "No se pudo completar la biométrica." }, "needADifferentMethod": { "message": "¿Necesita un método distinto?" }, "useMasterPassword": { - "message": "Use master password" + "message": "Usar contraseña maestra" }, "usePin": { "message": "Usar NIP" }, "useBiometrics": { - "message": "Use biometrics" + "message": "Usar biométrica" }, "enterVerificationCodeSentToEmail": { - "message": "Enter the verification code that was sent to your email." + "message": "Introduzca el código de verificación que se ha enviado a su correo electrónico." }, "resendCode": { "message": "Volver a enviar código" @@ -2706,19 +2706,19 @@ } }, "launchDuoAndFollowStepsToFinishLoggingIn": { - "message": "Launch Duo and follow the steps to finish logging in." + "message": "Abra Duo y siga los pasos para terminar de iniciar sesión." }, "duoRequiredForAccount": { - "message": "Duo two-step login is required for your account." + "message": "Se requiere el inicio de sesión en dos pasos Duo para su cuenta." }, "popoutTheExtensionToCompleteLogin": { - "message": "Popout the extension to complete login." + "message": "Abra la extensión para completar el inicio de sesión." }, "popoutExtension": { - "message": "Popout extension" + "message": "Abrir extensión" }, "launchDuo": { - "message": "Launch Duo" + "message": "Iniciar Duo" }, "importFormatError": { "message": "Los datos no están formateados correctamente. Por favor, comprueba tu archivo de importación e inténtalo de nuevo." @@ -2995,15 +2995,15 @@ "description": "Button text for the setting that allows overriding the default browser autofill settings" }, "saveCipherAttemptSuccess": { - "message": "Credentials saved successfully!", + "message": "¡Credenciales guardadas con éxito!", "description": "Notification message for when saving credentials has succeeded." }, "updateCipherAttemptSuccess": { - "message": "Credentials updated successfully!", + "message": "¡Credenciales actualizadas con éxito!", "description": "Notification message for when updating credentials has succeeded." }, "saveCipherAttemptFailed": { - "message": "Error saving credentials. Check console for details.", + "message": "Se produjo un error al guardar las credenciales. Revise la consola para obtener detalles.", "description": "Notification message for when saving credentials has failed." } } diff --git a/apps/browser/src/_locales/ko/messages.json b/apps/browser/src/_locales/ko/messages.json index a4186df93bf..2943c7234d9 100644 --- a/apps/browser/src/_locales/ko/messages.json +++ b/apps/browser/src/_locales/ko/messages.json @@ -92,13 +92,13 @@ "message": "자동 완성" }, "autoFillLogin": { - "message": "Auto-fill login" + "message": "로그인 자동 완성" }, "autoFillCard": { - "message": "Auto-fill card" + "message": "카드 자동 완성" }, "autoFillIdentity": { - "message": "Auto-fill identity" + "message": "신원 자동 완성" }, "generatePasswordCopied": { "message": "비밀번호 생성 및 클립보드에 복사" @@ -110,19 +110,19 @@ "message": "사용할 수 있는 로그인이 없습니다." }, "noCards": { - "message": "No cards" + "message": "카드 없음" }, "noIdentities": { - "message": "No identities" + "message": "신원 없음" }, "addLoginMenu": { - "message": "Add login" + "message": "로그인 추가" }, "addCardMenu": { - "message": "Add card" + "message": "카드 추가" }, "addIdentityMenu": { - "message": "Add identity" + "message": "신원 추가" }, "unlockVaultMenu": { "message": "보관함 잠금 해제" @@ -220,7 +220,7 @@ "message": "도움말 및 의견" }, "helpCenter": { - "message": "Bitwarden Help center" + "message": "Bitwarden 도움말 센터" }, "communityForums": { "message": "Explore Bitwarden community forums" From c09b446e63210f68bcbfd91f5e2f3b5c4dedd609 Mon Sep 17 00:00:00 2001 From: Will Martin Date: Fri, 8 Mar 2024 09:15:07 -0500 Subject: [PATCH 10/31] [PM-5054] migrate emergency access to CL (#7850) Co-authored-by: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> --- .../emergency-access-confirm.component.html | 87 ++- .../emergency-access-confirm.component.ts | 53 +- .../emergency-access-add-edit.component.html | 192 ++----- .../emergency-access-add-edit.component.ts | 98 +++- .../emergency-access.component.html | 506 +++++++++--------- .../emergency-access.component.ts | 139 +++-- .../emergency-access-takeover.component.html | 129 ++--- .../emergency-access-takeover.component.ts | 62 ++- .../view/emergency-access-view.component.html | 77 +-- apps/web/src/locales/en/messages.json | 3 + 10 files changed, 671 insertions(+), 675 deletions(-) diff --git a/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.html b/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.html index 77620677849..a8a4bd53d90 100644 --- a/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.html +++ b/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.html @@ -1,52 +1,37 @@ - + {{ "learnMore" | i18n }} +

+

+ {{ fingerprint }} +

+ + + + {{ "dontAskFingerprintAgain" | i18n }} + + +
+ + +
+ + diff --git a/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.ts b/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.ts index 711a7ba9a5b..4afc60c9be3 100644 --- a/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.ts +++ b/apps/web/src/app/auth/settings/emergency-access/confirm/emergency-access-confirm.component.ts @@ -1,39 +1,52 @@ -import { Component, EventEmitter, Input, OnInit, Output } from "@angular/core"; +import { DialogConfig, DialogRef, DIALOG_DATA } from "@angular/cdk/dialog"; +import { Component, OnInit, Inject } from "@angular/core"; +import { FormBuilder } from "@angular/forms"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { DialogService } from "@bitwarden/components"; +export enum EmergencyAccessConfirmDialogResult { + Confirmed = "confirmed", +} +type EmergencyAccessConfirmDialogData = { + /** display name of the account requesting emergency access */ + name: string; + /** identifies the account requesting emergency access */ + userId: string; + /** traces a unique emergency request */ + emergencyAccessId: string; +}; @Component({ selector: "emergency-access-confirm", templateUrl: "emergency-access-confirm.component.html", }) export class EmergencyAccessConfirmComponent implements OnInit { - @Input() name: string; - @Input() userId: string; - @Input() emergencyAccessId: string; - @Input() formPromise: Promise; - @Output() onConfirmed = new EventEmitter(); - - dontAskAgain = false; loading = true; fingerprint: string; + confirmForm = this.formBuilder.group({ + dontAskAgain: [false], + }); constructor( + @Inject(DIALOG_DATA) protected params: EmergencyAccessConfirmDialogData, + private formBuilder: FormBuilder, private apiService: ApiService, private cryptoService: CryptoService, private stateService: StateService, private logService: LogService, + private dialogRef: DialogRef, ) {} async ngOnInit() { try { - const publicKeyResponse = await this.apiService.getUserPublicKey(this.userId); + const publicKeyResponse = await this.apiService.getUserPublicKey(this.params.userId); if (publicKeyResponse != null) { const publicKey = Utils.fromB64ToArray(publicKeyResponse.publicKey); - const fingerprint = await this.cryptoService.getFingerprint(this.userId, publicKey); + const fingerprint = await this.cryptoService.getFingerprint(this.params.userId, publicKey); if (fingerprint != null) { this.fingerprint = fingerprint.join("-"); } @@ -44,19 +57,33 @@ export class EmergencyAccessConfirmComponent implements OnInit { this.loading = false; } - async submit() { + submit = async () => { if (this.loading) { return; } - if (this.dontAskAgain) { + if (this.confirmForm.get("dontAskAgain").value) { await this.stateService.setAutoConfirmFingerprints(true); } try { - this.onConfirmed.emit(); + this.dialogRef.close(EmergencyAccessConfirmDialogResult.Confirmed); } catch (e) { this.logService.error(e); } + }; + /** + * Strongly typed helper to open a EmergencyAccessConfirmComponent + * @param dialogService Instance of the dialog service that will be used to open the dialog + * @param config Configuration for the dialog + */ + static open( + dialogService: DialogService, + config: DialogConfig, + ) { + return dialogService.open( + EmergencyAccessConfirmComponent, + config, + ); } } diff --git a/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.html b/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.html index 43961d6a13c..1e61585e422 100644 --- a/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.html +++ b/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.html @@ -1,142 +1,68 @@ - + + + {{ "view" | i18n }} + {{ "viewDesc" | i18n }} + + + + {{ "takeover" | i18n }} + {{ "takeoverDesc" | i18n }} + + + + + {{ "waitTime" | i18n }} + + + + {{ "waitTimeDesc" | i18n }} + + + + + + + + + diff --git a/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.ts b/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.ts index 4aec390a56a..d99c693e73e 100644 --- a/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.ts +++ b/apps/web/src/app/auth/settings/emergency-access/emergency-access-add-edit.component.ts @@ -1,45 +1,59 @@ -import { Component, EventEmitter, Input, OnInit, Output } from "@angular/core"; +import { DialogConfig, DialogRef, DIALOG_DATA } from "@angular/cdk/dialog"; +import { Component, Inject, OnInit } from "@angular/core"; +import { FormBuilder, Validators } from "@angular/forms"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { DialogService } from "@bitwarden/components"; import { EmergencyAccessService } from "../../emergency-access"; import { EmergencyAccessType } from "../../emergency-access/enums/emergency-access-type"; +export type EmergencyAccessAddEditDialogData = { + /** display name of the account requesting emergency access */ + name: string; + /** traces a unique emergency request */ + emergencyAccessId: string; + /** A boolean indicating whether the emergency access request is in read-only mode (true for view-only, false for editing). */ + readOnly: boolean; +}; + +export enum EmergencyAccessAddEditDialogResult { + Saved = "saved", + Canceled = "canceled", + Deleted = "deleted", +} @Component({ selector: "emergency-access-add-edit", templateUrl: "emergency-access-add-edit.component.html", }) export class EmergencyAccessAddEditComponent implements OnInit { - @Input() name: string; - @Input() emergencyAccessId: string; - @Output() onSaved = new EventEmitter(); - @Output() onDeleted = new EventEmitter(); - loading = true; readOnly = false; editMode = false; title: string; - email: string; type: EmergencyAccessType = EmergencyAccessType.View; - formPromise: Promise; - emergencyAccessType = EmergencyAccessType; waitTimes: { name: string; value: number }[]; - waitTime: number; + addEditForm = this.formBuilder.group({ + email: ["", [Validators.email, Validators.required]], + emergencyAccessType: [this.emergencyAccessType.View], + waitTime: [{ value: null, disabled: this.readOnly }, [Validators.required]], + }); constructor( + @Inject(DIALOG_DATA) protected params: EmergencyAccessAddEditDialogData, + private formBuilder: FormBuilder, private emergencyAccessService: EmergencyAccessService, private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, private logService: LogService, + private dialogRef: DialogRef, ) {} - async ngOnInit() { - this.editMode = this.loading = this.emergencyAccessId != null; - + this.editMode = this.loading = this.params.emergencyAccessId != null; this.waitTimes = [ { name: this.i18nService.t("oneDay"), value: 1 }, { name: this.i18nService.t("days", "2"), value: 2 }, @@ -50,46 +64,72 @@ export class EmergencyAccessAddEditComponent implements OnInit { ]; if (this.editMode) { - this.editMode = true; this.title = this.i18nService.t("editEmergencyContact"); try { const emergencyAccess = await this.emergencyAccessService.getEmergencyAccess( - this.emergencyAccessId, + this.params.emergencyAccessId, ); - this.type = emergencyAccess.type; - this.waitTime = emergencyAccess.waitTimeDays; + this.addEditForm.patchValue({ + email: emergencyAccess.email, + waitTime: emergencyAccess.waitTimeDays, + emergencyAccessType: emergencyAccess.type, + }); } catch (e) { this.logService.error(e); } } else { this.title = this.i18nService.t("inviteEmergencyContact"); - this.waitTime = this.waitTimes[2].value; + this.addEditForm.patchValue({ waitTime: this.waitTimes[2].value }); } this.loading = false; } - async submit() { + submit = async () => { + if (this.addEditForm.invalid) { + this.addEditForm.markAllAsTouched(); + return; + } try { if (this.editMode) { - await this.emergencyAccessService.update(this.emergencyAccessId, this.type, this.waitTime); + await this.emergencyAccessService.update( + this.params.emergencyAccessId, + this.addEditForm.value.emergencyAccessType, + this.addEditForm.value.waitTime, + ); } else { - await this.emergencyAccessService.invite(this.email, this.type, this.waitTime); + await this.emergencyAccessService.invite( + this.addEditForm.value.email, + this.addEditForm.value.emergencyAccessType, + this.addEditForm.value.waitTime, + ); } - - await this.formPromise; this.platformUtilsService.showToast( "success", null, - this.i18nService.t(this.editMode ? "editedUserId" : "invitedUsers", this.name), + this.i18nService.t(this.editMode ? "editedUserId" : "invitedUsers", this.params.name), ); - this.onSaved.emit(); + this.dialogRef.close(EmergencyAccessAddEditDialogResult.Saved); } catch (e) { this.logService.error(e); } - } + }; - async delete() { - this.onDeleted.emit(); - } + delete = async () => { + this.dialogRef.close(EmergencyAccessAddEditDialogResult.Deleted); + }; + /** + * Strongly typed helper to open a EmergencyAccessAddEditComponent + * @param dialogService Instance of the dialog service that will be used to open the dialog + * @param config Configuration for the dialog + */ + static open = ( + dialogService: DialogService, + config: DialogConfig, + ) => { + return dialogService.open( + EmergencyAccessAddEditComponent, + config, + ); + }; } diff --git a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.html b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.html index 27bfc673ec4..41c62db6095 100644 --- a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.html +++ b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.html @@ -1,264 +1,276 @@ - -

- {{ "emergencyAccessDesc" | i18n }} - - {{ "learnMore" | i18n }}. - -

- -

- {{ "warning" | i18n }}: {{ "emergencyAccessOwnerWarning" | i18n }} -

- -
-

- {{ "trustedEmergencyContacts" | i18n }} - -

-
- + {{ "learnMore" | i18n }}. + +

+ {{ + "emergencyAccessOwnerWarning" | i18n + }} + + +
+

+ {{ "trustedEmergencyContacts" | i18n }} +

+ +
+ +
-
+ + + + {{ "name" | i18n }} + {{ "accessLevel" | i18n }} + {{ "options" | i18n }} + + + + + + + + {{ c.email }} + {{ "invited" | i18n }} + {{ "accepted" | i18n }} + {{ "emergencyAccessRecoveryInitiated" | i18n }} + {{ + "emergencyAccessRecoveryApproved" | i18n + }} - - - - - - + + - - -
- - - {{ c.email }} - {{ "invited" | i18n }} - {{ "accepted" | i18n }} - {{ "emergencyAccessRecoveryInitiated" | i18n }} - {{ - "emergencyAccessRecoveryApproved" | i18n - }} - - {{ "view" | i18n }} - {{ - "takeover" | i18n - }} - - {{ c.name }} - - - + {{ c.name }} + + + {{ "view" | i18n }} + {{ + "takeover" | i18n + }} + - - - - - -
- - -

{{ "noTrustedContacts" | i18n }}

- - - {{ "loading" | i18n }} + appA11yTitle="{{ 'options' | i18n }}" + bitIconButton="bwi-ellipsis-v" + buttonType="main" + > + + + + + + + + + + + + +

{{ "noTrustedContacts" | i18n }}

+ + + {{ "loading" | i18n }} + - + -
-

{{ "designatedEmergencyContacts" | i18n }}

-
+ +

{{ "designatedEmergencyContacts" | i18n }}

- - - - - + + + + + + + + - + + - - -
- - - {{ c.email }} - {{ - "invited" | i18n - }} - {{ "accepted" | i18n }} - {{ "emergencyAccessRecoveryInitiated" | i18n }} - {{ "emergencyAccessRecoveryApproved" | i18n }} + + +
{{ "name" | i18n }}{{ "accessLevel" | i18n }}{{ "options" | i18n }}
+ + + {{ c.email }} + {{ + "invited" | i18n + }} + {{ "accepted" | i18n }} + {{ "emergencyAccessRecoveryInitiated" | i18n }} + {{ "emergencyAccessRecoveryApproved" | i18n }} - {{ "view" | i18n }} - {{ - "takeover" | i18n - }} - - {{ c.name }} - - - + {{ c.name }} + + + {{ "view" | i18n }} + {{ + "takeover" | i18n + }} + - - - - -
- - -

{{ "noGrantedAccess" | i18n }}

- - - {{ "loading" | i18n }} + appA11yTitle="{{ 'options' | i18n }}" + bitIconButton="bwi-ellipsis-v" + buttonType="main" + > + + + + + + + + + + + +

{{ "noGrantedAccess" | i18n }}

+ + + {{ "loading" | i18n }} + - + diff --git a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts index 084a80a582e..7293860bea6 100644 --- a/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts +++ b/apps/web/src/app/auth/settings/emergency-access/emergency-access.component.ts @@ -1,7 +1,7 @@ import { Component, OnInit, ViewChild, ViewContainerRef } from "@angular/core"; +import { lastValueFrom } from "rxjs"; import { UserNamePipe } from "@bitwarden/angular/pipes/user-name.pipe"; -import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -18,9 +18,18 @@ import { GrantorEmergencyAccess, } from "../../emergency-access/models/emergency-access"; -import { EmergencyAccessConfirmComponent } from "./confirm/emergency-access-confirm.component"; -import { EmergencyAccessAddEditComponent } from "./emergency-access-add-edit.component"; -import { EmergencyAccessTakeoverComponent } from "./takeover/emergency-access-takeover.component"; +import { + EmergencyAccessConfirmComponent, + EmergencyAccessConfirmDialogResult, +} from "./confirm/emergency-access-confirm.component"; +import { + EmergencyAccessAddEditComponent, + EmergencyAccessAddEditDialogResult, +} from "./emergency-access-add-edit.component"; +import { + EmergencyAccessTakeoverComponent, + EmergencyAccessTakeoverResultType, +} from "./takeover/emergency-access-takeover.component"; @Component({ selector: "emergency-access", @@ -46,7 +55,6 @@ export class EmergencyAccessComponent implements OnInit { constructor( private emergencyAccessService: EmergencyAccessService, private i18nService: I18nService, - private modalService: ModalService, private platformUtilsService: PlatformUtilsService, private messagingService: MessagingService, private userNamePipe: UserNamePipe, @@ -78,37 +86,26 @@ export class EmergencyAccessComponent implements OnInit { } } - async edit(details: GranteeEmergencyAccess) { - const [modal] = await this.modalService.openViewRef( - EmergencyAccessAddEditComponent, - this.addEditModalRef, - (comp) => { - comp.name = this.userNamePipe.transform(details); - comp.emergencyAccessId = details?.id; - comp.readOnly = !this.canAccessPremium; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - comp.onSaved.subscribe(() => { - modal.close(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.load(); - }); - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - comp.onDeleted.subscribe(() => { - modal.close(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.remove(details); - }); + edit = async (details: GranteeEmergencyAccess) => { + const dialogRef = EmergencyAccessAddEditComponent.open(this.dialogService, { + data: { + name: this.userNamePipe.transform(details), + emergencyAccessId: details?.id, + readOnly: !this.canAccessPremium, }, - ); - } + }); - invite() { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.edit(null); - } + const result = await lastValueFrom(dialogRef.closed); + if (result === EmergencyAccessAddEditDialogResult.Saved) { + await this.load(); + } else if (result === EmergencyAccessAddEditDialogResult.Deleted) { + await this.remove(details); + } + }; + + invite = async () => { + await this.edit(null); + }; async reinvite(contact: GranteeEmergencyAccess) { if (this.actionPromise != null) { @@ -135,29 +132,23 @@ export class EmergencyAccessComponent implements OnInit { const autoConfirm = await this.stateService.getAutoConfirmFingerPrints(); if (autoConfirm == null || !autoConfirm) { - const [modal] = await this.modalService.openViewRef( - EmergencyAccessConfirmComponent, - this.confirmModalRef, - (comp) => { - comp.name = this.userNamePipe.transform(contact); - comp.emergencyAccessId = contact.id; - comp.userId = contact?.granteeId; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - comp.onConfirmed.subscribe(async () => { - modal.close(); - - comp.formPromise = this.emergencyAccessService.confirm(contact.id, contact.granteeId); - await comp.formPromise; - - updateUser(); - this.platformUtilsService.showToast( - "success", - null, - this.i18nService.t("hasBeenConfirmed", this.userNamePipe.transform(contact)), - ); - }); + const dialogRef = EmergencyAccessConfirmComponent.open(this.dialogService, { + data: { + name: this.userNamePipe.transform(contact), + emergencyAccessId: contact.id, + userId: contact?.granteeId, }, - ); + }); + const result = await lastValueFrom(dialogRef.closed); + if (result === EmergencyAccessConfirmDialogResult.Confirmed) { + await this.emergencyAccessService.confirm(contact.id, contact.granteeId); + updateUser(); + this.platformUtilsService.showToast( + "success", + null, + this.i18nService.t("hasBeenConfirmed", this.userNamePipe.transform(contact)), + ); + } return; } @@ -267,27 +258,23 @@ export class EmergencyAccessComponent implements OnInit { ); } - async takeover(details: GrantorEmergencyAccess) { - const [modal] = await this.modalService.openViewRef( - EmergencyAccessTakeoverComponent, - this.takeoverModalRef, - (comp) => { - comp.name = this.userNamePipe.transform(details); - comp.email = details.email; - comp.emergencyAccessId = details != null ? details.id : null; - - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - comp.onDone.subscribe(() => { - modal.close(); - this.platformUtilsService.showToast( - "success", - null, - this.i18nService.t("passwordResetFor", this.userNamePipe.transform(details)), - ); - }); + takeover = async (details: GrantorEmergencyAccess) => { + const dialogRef = EmergencyAccessTakeoverComponent.open(this.dialogService, { + data: { + name: this.userNamePipe.transform(details), + email: details.email, + emergencyAccessId: details.id ?? null, }, - ); - } + }); + const result = await lastValueFrom(dialogRef.closed); + if (result === EmergencyAccessTakeoverResultType.Done) { + this.platformUtilsService.showToast( + "success", + null, + this.i18nService.t("passwordResetFor", this.userNamePipe.transform(details)), + ); + } + }; private removeGrantee(details: GranteeEmergencyAccess) { const index = this.trustedContacts.indexOf(details); diff --git a/apps/web/src/app/auth/settings/emergency-access/takeover/emergency-access-takeover.component.html b/apps/web/src/app/auth/settings/emergency-access/takeover/emergency-access-takeover.component.html index 99aa194c5d6..5c72efbd619 100644 --- a/apps/web/src/app/auth/settings/emergency-access/takeover/emergency-access-takeover.component.html +++ b/apps/web/src/app/auth/settings/emergency-access/takeover/emergency-access-takeover.component.html @@ -1,79 +1,54 @@ -
-
+
- +

{{ "secretsManager" | i18n }}
{{ "userPermissionOverrideHelper" | i18n }}
-
+
@@ -434,7 +440,7 @@ [columnHeader]="'collection' | i18n" [selectorLabelText]="'selectCollections' | i18n" [emptySelectionText]="'noCollectionsAdded' | i18n" - [flexibleCollectionsEnabled]="flexibleCollectionsEnabled" + [flexibleCollectionsEnabled]="organization.flexibleCollections" > diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index 5461aecfcc1..4d6442e8988 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -1,7 +1,16 @@ import { DIALOG_DATA, DialogConfig, DialogRef } from "@angular/cdk/dialog"; -import { Component, Inject, OnDestroy, OnInit } from "@angular/core"; +import { Component, Inject, OnDestroy } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; -import { combineLatest, of, shareReplay, Subject, switchMap, takeUntil } from "rxjs"; +import { + combineLatest, + firstValueFrom, + Observable, + of, + shareReplay, + Subject, + switchMap, + takeUntil, +} from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationUserService } from "@bitwarden/common/admin-console/abstractions/organization-user/organization-user.service"; @@ -18,7 +27,6 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { DialogService } from "@bitwarden/components"; -import { flagEnabled } from "../../../../../../utils/flags"; import { CollectionAdminService } from "../../../../../vault/core/collection-admin.service"; import { CollectionAccessSelectionView, @@ -66,7 +74,7 @@ export enum MemberDialogResult { @Component({ templateUrl: "member-dialog.component.html", }) -export class MemberDialogComponent implements OnInit, OnDestroy { +export class MemberDialogComponent implements OnDestroy { loading = true; editMode = false; isRevoked = false; @@ -74,12 +82,10 @@ export class MemberDialogComponent implements OnInit, OnDestroy { access: "all" | "selected" = "selected"; collections: CollectionView[] = []; organizationUserType = OrganizationUserType; - canUseCustomPermissions: boolean; PermissionMode = PermissionMode; - canUseSecretsManager: boolean; showNoMasterPasswordWarning = false; - protected organization: Organization; + protected organization$: Observable; protected collectionAccessItems: AccessItemView[] = []; protected groupAccessItems: AccessItemView[] = []; protected tabIndex: MemberDialogTab; @@ -130,7 +136,6 @@ export class MemberDialogComponent implements OnInit, OnDestroy { private dialogRef: DialogRef, private i18nService: I18nService, private platformUtilsService: PlatformUtilsService, - private organizationService: OrganizationService, private formBuilder: FormBuilder, // TODO: We should really look into consolidating naming conventions for these services private collectionAdminService: CollectionAdminService, @@ -139,28 +144,26 @@ export class MemberDialogComponent implements OnInit, OnDestroy { private organizationUserService: OrganizationUserService, private dialogService: DialogService, private configService: ConfigServiceAbstraction, - ) {} + organizationService: OrganizationService, + ) { + this.organization$ = organizationService + .get$(this.params.organizationId) + .pipe(shareReplay({ refCount: true, bufferSize: 1 })); - async ngOnInit() { this.editMode = this.params.organizationUserId != null; this.tabIndex = this.params.initialTab ?? MemberDialogTab.Role; this.title = this.i18nService.t(this.editMode ? "editMember" : "inviteMember"); - const organization$ = of(this.organizationService.get(this.params.organizationId)).pipe( - shareReplay({ refCount: true, bufferSize: 1 }), - ); - const groups$ = organization$.pipe( - switchMap((organization) => { - if (!organization.useGroups) { - return of([] as GroupView[]); - } - - return this.groupService.getAll(this.params.organizationId); - }), + const groups$ = this.organization$.pipe( + switchMap((organization) => + organization.useGroups + ? this.groupService.getAll(this.params.organizationId) + : of([] as GroupView[]), + ), ); combineLatest({ - organization: organization$, + organization: this.organization$, collections: this.collectionAdminService.getAll(this.params.organizationId), userDetails: this.params.organizationUserId ? this.userService.get(this.params.organizationId, this.params.organizationUserId) @@ -169,23 +172,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { }) .pipe(takeUntil(this.destroy$)) .subscribe(({ organization, collections, userDetails, groups }) => { - this.organization = organization; - this.canUseCustomPermissions = organization.useCustomPermissions; - this.canUseSecretsManager = organization.useSecretsManager && flagEnabled("secretsManager"); - - const emailsControlValidators = [ - Validators.required, - commaSeparatedEmails, - orgSeatLimitReachedValidator( - this.organization, - this.params.allOrganizationUserEmails, - this.i18nService.t("subscriptionUpgrade", organization.seats), - ), - ]; - - const emailsControl = this.formGroup.get("emails"); - emailsControl.setValidators(emailsControlValidators); - emailsControl.updateValueAndValidity(); + this.setFormValidators(organization); this.collectionAccessItems = [].concat( collections.map((c) => mapCollectionToAccessItemView(c)), @@ -196,77 +183,101 @@ export class MemberDialogComponent implements OnInit, OnDestroy { ); if (this.params.organizationUserId) { - if (!userDetails) { - throw new Error("Could not find user to edit."); - } - this.isRevoked = userDetails.status === OrganizationUserStatusType.Revoked; - this.showNoMasterPasswordWarning = - userDetails.status > OrganizationUserStatusType.Invited && - userDetails.hasMasterPassword === false; - const assignedCollectionsPermissions = { - editAssignedCollections: userDetails.permissions.editAssignedCollections, - deleteAssignedCollections: userDetails.permissions.deleteAssignedCollections, - manageAssignedCollections: - userDetails.permissions.editAssignedCollections && - userDetails.permissions.deleteAssignedCollections, - }; - const allCollectionsPermissions = { - createNewCollections: userDetails.permissions.createNewCollections, - editAnyCollection: userDetails.permissions.editAnyCollection, - deleteAnyCollection: userDetails.permissions.deleteAnyCollection, - manageAllCollections: - userDetails.permissions.createNewCollections && - userDetails.permissions.editAnyCollection && - userDetails.permissions.deleteAnyCollection, - }; - if (userDetails.type === OrganizationUserType.Custom) { - this.permissionsGroup.patchValue({ - accessEventLogs: userDetails.permissions.accessEventLogs, - accessImportExport: userDetails.permissions.accessImportExport, - accessReports: userDetails.permissions.accessReports, - manageGroups: userDetails.permissions.manageGroups, - manageSso: userDetails.permissions.manageSso, - managePolicies: userDetails.permissions.managePolicies, - manageUsers: userDetails.permissions.manageUsers, - manageResetPassword: userDetails.permissions.manageResetPassword, - manageAssignedCollectionsGroup: assignedCollectionsPermissions, - manageAllCollectionsGroup: allCollectionsPermissions, - }); - } - - const collectionsFromGroups = groups - .filter((group) => userDetails.groups.includes(group.id)) - .flatMap((group) => - group.collections.map((accessSelection) => { - const collection = collections.find((c) => c.id === accessSelection.id); - return { group, collection, accessSelection }; - }), - ); - - this.collectionAccessItems = this.collectionAccessItems.concat( - collectionsFromGroups.map(({ collection, accessSelection, group }) => - mapCollectionToAccessItemView(collection, accessSelection, group), - ), - ); - - const accessSelections = mapToAccessSelections(userDetails); - const groupAccessSelections = mapToGroupAccessSelections(userDetails.groups); - - this.formGroup.removeControl("emails"); - this.formGroup.patchValue({ - type: userDetails.type, - externalId: userDetails.externalId, - accessAllCollections: userDetails.accessAll, - access: accessSelections, - accessSecretsManager: userDetails.accessSecretsManager, - groups: groupAccessSelections, - }); + this.loadOrganizationUser(userDetails, groups, collections); } this.loading = false; }); } + private setFormValidators(organization: Organization) { + const emailsControlValidators = [ + Validators.required, + commaSeparatedEmails, + orgSeatLimitReachedValidator( + organization, + this.params.allOrganizationUserEmails, + this.i18nService.t("subscriptionUpgrade", organization.seats), + ), + ]; + + const emailsControl = this.formGroup.get("emails"); + emailsControl.setValidators(emailsControlValidators); + emailsControl.updateValueAndValidity(); + } + + private loadOrganizationUser( + userDetails: OrganizationUserAdminView, + groups: GroupView[], + collections: CollectionView[], + ) { + if (!userDetails) { + throw new Error("Could not find user to edit."); + } + this.isRevoked = userDetails.status === OrganizationUserStatusType.Revoked; + this.showNoMasterPasswordWarning = + userDetails.status > OrganizationUserStatusType.Invited && + userDetails.hasMasterPassword === false; + const assignedCollectionsPermissions = { + editAssignedCollections: userDetails.permissions.editAssignedCollections, + deleteAssignedCollections: userDetails.permissions.deleteAssignedCollections, + manageAssignedCollections: + userDetails.permissions.editAssignedCollections && + userDetails.permissions.deleteAssignedCollections, + }; + const allCollectionsPermissions = { + createNewCollections: userDetails.permissions.createNewCollections, + editAnyCollection: userDetails.permissions.editAnyCollection, + deleteAnyCollection: userDetails.permissions.deleteAnyCollection, + manageAllCollections: + userDetails.permissions.createNewCollections && + userDetails.permissions.editAnyCollection && + userDetails.permissions.deleteAnyCollection, + }; + if (userDetails.type === OrganizationUserType.Custom) { + this.permissionsGroup.patchValue({ + accessEventLogs: userDetails.permissions.accessEventLogs, + accessImportExport: userDetails.permissions.accessImportExport, + accessReports: userDetails.permissions.accessReports, + manageGroups: userDetails.permissions.manageGroups, + manageSso: userDetails.permissions.manageSso, + managePolicies: userDetails.permissions.managePolicies, + manageUsers: userDetails.permissions.manageUsers, + manageResetPassword: userDetails.permissions.manageResetPassword, + manageAssignedCollectionsGroup: assignedCollectionsPermissions, + manageAllCollectionsGroup: allCollectionsPermissions, + }); + } + + const collectionsFromGroups = groups + .filter((group) => userDetails.groups.includes(group.id)) + .flatMap((group) => + group.collections.map((accessSelection) => { + const collection = collections.find((c) => c.id === accessSelection.id); + return { group, collection, accessSelection }; + }), + ); + + this.collectionAccessItems = this.collectionAccessItems.concat( + collectionsFromGroups.map(({ collection, accessSelection, group }) => + mapCollectionToAccessItemView(collection, accessSelection, group), + ), + ); + + const accessSelections = mapToAccessSelections(userDetails); + const groupAccessSelections = mapToGroupAccessSelections(userDetails.groups); + + this.formGroup.removeControl("emails"); + this.formGroup.patchValue({ + type: userDetails.type, + externalId: userDetails.externalId, + accessAllCollections: userDetails.accessAll, + access: accessSelections, + accessSecretsManager: userDetails.accessSecretsManager, + groups: groupAccessSelections, + }); + } + check(c: CollectionView, select?: boolean) { (c as any).checked = select == null ? !(c as any).checked : select; if (!(c as any).checked) { @@ -335,7 +346,9 @@ export class MemberDialogComponent implements OnInit, OnDestroy { return; } - if (!this.canUseCustomPermissions && this.customUserTypeSelected) { + const organization = await firstValueFrom(this.organization$); + + if (!organization.useCustomPermissions && this.customUserTypeSelected) { this.platformUtilsService.showToast( "error", null, @@ -363,8 +376,7 @@ export class MemberDialogComponent implements OnInit, OnDestroy { await this.userService.save(userView); } else { userView.id = this.params.organizationUserId; - const maxEmailsCount = - this.organization.planProductType === ProductType.TeamsStarter ? 10 : 20; + const maxEmailsCount = organization.planProductType === ProductType.TeamsStarter ? 10 : 20; const emails = [...new Set(this.formGroup.value.emails.trim().split(/\s*,\s*/))]; if (emails.length > maxEmailsCount) { this.formGroup.controls.emails.setErrors({ @@ -373,8 +385,8 @@ export class MemberDialogComponent implements OnInit, OnDestroy { return; } if ( - this.organization.hasReseller && - this.params.numConfirmedMembers + emails.length > this.organization.seats + organization.hasReseller && + this.params.numConfirmedMembers + emails.length > organization.seats ) { this.formGroup.controls.emails.setErrors({ tooManyEmails: { message: this.i18nService.t("seatLimitReachedContactYourProvider") }, @@ -515,10 +527,6 @@ export class MemberDialogComponent implements OnInit, OnDestroy { }); } - protected get flexibleCollectionsEnabled() { - return this.organization?.flexibleCollections; - } - protected readonly ProductType = ProductType; }