From 3a8f95622216fc16be9a6bad8f9ad851fb8da142 Mon Sep 17 00:00:00 2001 From: Alex Dragovich <46065570+itsadrago@users.noreply.github.com> Date: Thu, 5 Feb 2026 13:33:44 -0800 Subject: [PATCH] Revert "Remove feature flag check from password generation (#18003)" (#18794) This reverts commit 7c6d98b50ec9143f9e587122d8f880cdd2758caf. --- apps/browser/src/background/main.background.ts | 1 - .../cli/src/service-container/service-container.ts | 1 - libs/common/src/tools/providers.spec.ts | 11 ----------- libs/common/src/tools/providers.ts | 6 ++---- libs/importer/src/components/importer-providers.ts | 2 -- .../components/src/generator-services.module.ts | 12 ++++++++++-- .../core/src/engine/sdk-password-randomizer.ts | 14 ++++++-------- .../src/metadata/password/eff-word-list.spec.ts | 12 +++++++++++- .../core/src/metadata/password/eff-word-list.ts | 5 ++++- .../src/metadata/password/random-password.spec.ts | 10 +++++++++- .../core/src/metadata/password/random-password.ts | 5 ++++- .../src/providers/generator-dependency-provider.ts | 4 ++-- .../providers/generator-metadata-provider.spec.ts | 6 ++++++ 13 files changed, 54 insertions(+), 35 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index e0cbcdc9f96..eb6d26357eb 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1110,7 +1110,6 @@ export default class MainBackground { this.logService, this.platformUtilsService, this.configService, - this.sdkService, ), ); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 105d80b290b..3e78eb36577 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -928,7 +928,6 @@ export class ServiceContainer { this.logService, this.platformUtilsService, this.configService, - this.sdkService, ), ); diff --git a/libs/common/src/tools/providers.spec.ts b/libs/common/src/tools/providers.spec.ts index d457b1df85e..5953e5ebab2 100644 --- a/libs/common/src/tools/providers.spec.ts +++ b/libs/common/src/tools/providers.spec.ts @@ -4,7 +4,6 @@ import { PolicyService } from "../admin-console/abstractions/policy/policy.servi import { ConfigService } from "../platform/abstractions/config/config.service"; import { LogService } from "../platform/abstractions/log.service"; import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service"; -import { SdkService } from "../platform/abstractions/sdk/sdk.service"; import { StateProvider } from "../platform/state"; import { LegacyEncryptorProvider } from "./cryptography/legacy-encryptor-provider"; @@ -21,7 +20,6 @@ describe("SystemServiceProvider", () => { let mockLogger: LogService; let mockEnvironment: MockProxy; let mockConfigService: ConfigService; - let mockSdkService: SdkService; beforeEach(() => { jest.resetAllMocks(); @@ -33,7 +31,6 @@ describe("SystemServiceProvider", () => { mockLogger = mock(); mockEnvironment = mock(); mockConfigService = mock(); - mockSdkService = mock(); }); describe("createSystemServiceProvider", () => { @@ -48,7 +45,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result).toHaveProperty("policy", mockPolicy); @@ -70,7 +66,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result.extension).toBeInstanceOf(ExtensionService); @@ -88,7 +83,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(mockEnvironment.isDev).toHaveBeenCalledTimes(1); @@ -108,7 +102,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(mockEnvironment.isDev).toHaveBeenCalledTimes(1); @@ -128,7 +121,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result.extension).toBeInstanceOf(ExtensionService); @@ -146,7 +138,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result.policy).toBe(mockPolicy); @@ -163,7 +154,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result.configService).toBe(mockConfigService); @@ -180,7 +170,6 @@ describe("SystemServiceProvider", () => { mockLogger, mockEnvironment, mockConfigService, - mockSdkService, ); expect(result.environment).toBe(mockEnvironment); diff --git a/libs/common/src/tools/providers.ts b/libs/common/src/tools/providers.ts index b1621f19c21..ac42c556042 100644 --- a/libs/common/src/tools/providers.ts +++ b/libs/common/src/tools/providers.ts @@ -1,10 +1,10 @@ import { LogService } from "@bitwarden/logging"; +import { BitwardenClient } from "@bitwarden/sdk-internal"; import { StateProvider } from "@bitwarden/state"; import { PolicyService } from "../admin-console/abstractions/policy/policy.service.abstraction"; import { ConfigService } from "../platform/abstractions/config/config.service"; import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service"; -import { SdkService } from "../platform/abstractions/sdk/sdk.service"; import { LegacyEncryptorProvider } from "./cryptography/legacy-encryptor-provider"; import { ExtensionRegistry } from "./extension/extension-registry.abstraction"; @@ -29,7 +29,7 @@ export type SystemServiceProvider = { readonly environment: PlatformUtilsService; /** SDK Service */ - readonly sdk: SdkService; + readonly sdk?: BitwardenClient; }; /** Constructs a system service provider. */ @@ -41,7 +41,6 @@ export function createSystemServiceProvider( logger: LogService, environment: PlatformUtilsService, configService: ConfigService, - sdk: SdkService, ): SystemServiceProvider { let log: LogProvider; if (environment.isDev()) { @@ -63,6 +62,5 @@ export function createSystemServiceProvider( log, configService, environment, - sdk, }; } diff --git a/libs/importer/src/components/importer-providers.ts b/libs/importer/src/components/importer-providers.ts index eb7e58e9259..18c148ebe2e 100644 --- a/libs/importer/src/components/importer-providers.ts +++ b/libs/importer/src/components/importer-providers.ts @@ -13,7 +13,6 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co 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 { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { KeyServiceLegacyEncryptorProvider } from "@bitwarden/common/tools/cryptography/key-service-legacy-encryptor-provider"; import { LegacyEncryptorProvider } from "@bitwarden/common/tools/cryptography/legacy-encryptor-provider"; import { ExtensionRegistry } from "@bitwarden/common/tools/extension/extension-registry.abstraction"; @@ -72,7 +71,6 @@ export const ImporterProviders: SafeProvider[] = [ LogService, PlatformUtilsService, ConfigService, - SdkService, ], }), safeProvider({ diff --git a/libs/tools/generator/components/src/generator-services.module.ts b/libs/tools/generator/components/src/generator-services.module.ts index 39d0dd298a2..935f7dc2d60 100644 --- a/libs/tools/generator/components/src/generator-services.module.ts +++ b/libs/tools/generator/components/src/generator-services.module.ts @@ -1,10 +1,12 @@ import { NgModule } from "@angular/core"; +import { from, take } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { safeProvider } from "@bitwarden/angular/platform/utils/safe-provider"; import { SafeInjectionToken } from "@bitwarden/angular/services/injection-tokens"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -124,7 +126,7 @@ export const SYSTEM_SERVICE_PROVIDER = new SafeInjectionToken (featureFlag = ff)); const metadata = new providers.GeneratorMetadataProvider( userStateDeps, system, Object.values(BuiltIn), ); + const sdkService = featureFlag ? system.sdk : undefined; const profile = new providers.GeneratorProfileProvider(userStateDeps, system.policy); const generator: providers.GeneratorDependencyProvider = { randomizer: random, client: new RestClient(api, i18n), i18nService: i18n, - sdk: system.sdk, + sdk: sdkService, now: Date.now, }; diff --git a/libs/tools/generator/core/src/engine/sdk-password-randomizer.ts b/libs/tools/generator/core/src/engine/sdk-password-randomizer.ts index 09c7d62b1ad..03be21eeefb 100644 --- a/libs/tools/generator/core/src/engine/sdk-password-randomizer.ts +++ b/libs/tools/generator/core/src/engine/sdk-password-randomizer.ts @@ -1,6 +1,3 @@ -import { firstValueFrom } from "rxjs"; - -import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { BitwardenClient, PassphraseGeneratorRequest, @@ -23,11 +20,11 @@ export class SdkPasswordRandomizer CredentialGenerator { /** Instantiates the password randomizer - * @param service access to SDK client to call upon password/passphrase generation + * @param client access to SDK client to call upon password/passphrase generation * @param currentTime gets the current datetime in epoch time */ constructor( - private service: SdkService, + private client: BitwardenClient, private currentTime: () => number, ) {} @@ -43,9 +40,8 @@ export class SdkPasswordRandomizer request: GenerateRequest, settings: PasswordGenerationOptions | PassphraseGenerationOptions, ) { - const sdk: BitwardenClient = await firstValueFrom(this.service.client$); if (isPasswordGenerationOptions(settings)) { - const password = await sdk.generator().password(convertPasswordRequest(settings)); + const password = await this.client.generator().password(convertPasswordRequest(settings)); return new GeneratedCredential( password, @@ -55,7 +51,9 @@ export class SdkPasswordRandomizer request.website, ); } else if (isPassphraseGenerationOptions(settings)) { - const passphrase = await sdk.generator().passphrase(convertPassphraseRequest(settings)); + const passphrase = await this.client + .generator() + .passphrase(convertPassphraseRequest(settings)); return new GeneratedCredential( passphrase, diff --git a/libs/tools/generator/core/src/metadata/password/eff-word-list.spec.ts b/libs/tools/generator/core/src/metadata/password/eff-word-list.spec.ts index 015cc25a8ec..bdf021c50f3 100644 --- a/libs/tools/generator/core/src/metadata/password/eff-word-list.spec.ts +++ b/libs/tools/generator/core/src/metadata/password/eff-word-list.spec.ts @@ -3,7 +3,7 @@ import { mock } from "jest-mock-extended"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { SdkPasswordRandomizer } from "../../engine"; +import { PasswordRandomizer, SdkPasswordRandomizer } from "../../engine"; import { PassphrasePolicyConstraints } from "../../policies"; import { GeneratorDependencyProvider } from "../../providers"; import { PassphraseGenerationOptions } from "../../types"; @@ -22,6 +22,16 @@ describe("password - eff words generator metadata", () => { }); }); + describe("engine.create", () => { + const nonSdkDependencyProvider = mock(); + nonSdkDependencyProvider.sdk = undefined; + it("returns a password randomizer", () => { + expect(effPassphrase.engine.create(nonSdkDependencyProvider)).toBeInstanceOf( + PasswordRandomizer, + ); + }); + }); + describe("profiles[account]", () => { let accountProfile: CoreProfileMetadata | null = null; beforeEach(() => { diff --git a/libs/tools/generator/core/src/metadata/password/eff-word-list.ts b/libs/tools/generator/core/src/metadata/password/eff-word-list.ts index d6d78c83293..fc96ce46c2b 100644 --- a/libs/tools/generator/core/src/metadata/password/eff-word-list.ts +++ b/libs/tools/generator/core/src/metadata/password/eff-word-list.ts @@ -3,7 +3,7 @@ import { GENERATOR_DISK } from "@bitwarden/common/platform/state"; import { PublicClassifier } from "@bitwarden/common/tools/public-classifier"; import { ObjectKey } from "@bitwarden/common/tools/state/object-key"; -import { SdkPasswordRandomizer } from "../../engine"; +import { PasswordRandomizer, SdkPasswordRandomizer } from "../../engine"; import { passphraseLeastPrivilege, PassphrasePolicyConstraints } from "../../policies"; import { GeneratorDependencyProvider } from "../../providers"; import { CredentialGenerator, PassphraseGenerationOptions } from "../../types"; @@ -30,6 +30,9 @@ const passphrase: GeneratorMetadata = { create( dependencies: GeneratorDependencyProvider, ): CredentialGenerator { + if (dependencies.sdk == undefined) { + return new PasswordRandomizer(dependencies.randomizer, dependencies.now); + } return new SdkPasswordRandomizer(dependencies.sdk, dependencies.now); }, }, diff --git a/libs/tools/generator/core/src/metadata/password/random-password.spec.ts b/libs/tools/generator/core/src/metadata/password/random-password.spec.ts index d066b9f1597..9efd5350c21 100644 --- a/libs/tools/generator/core/src/metadata/password/random-password.spec.ts +++ b/libs/tools/generator/core/src/metadata/password/random-password.spec.ts @@ -3,7 +3,7 @@ import { mock } from "jest-mock-extended"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; -import { SdkPasswordRandomizer } from "../../engine"; +import { PasswordRandomizer, SdkPasswordRandomizer } from "../../engine"; import { DynamicPasswordPolicyConstraints } from "../../policies"; import { GeneratorDependencyProvider } from "../../providers"; import { PasswordGenerationOptions } from "../../types"; @@ -22,6 +22,14 @@ describe("password - characters generator metadata", () => { }); }); + describe("engine.create", () => { + const nonSdkDependencyProvider = mock(); + nonSdkDependencyProvider.sdk = undefined; + it("returns a password randomizer", () => { + expect(password.engine.create(nonSdkDependencyProvider)).toBeInstanceOf(PasswordRandomizer); + }); + }); + describe("profiles[account]", () => { let accountProfile: CoreProfileMetadata = null!; beforeEach(() => { diff --git a/libs/tools/generator/core/src/metadata/password/random-password.ts b/libs/tools/generator/core/src/metadata/password/random-password.ts index d25ea1e8f46..721be8dc3f0 100644 --- a/libs/tools/generator/core/src/metadata/password/random-password.ts +++ b/libs/tools/generator/core/src/metadata/password/random-password.ts @@ -3,7 +3,7 @@ import { GENERATOR_DISK } from "@bitwarden/common/platform/state"; import { PublicClassifier } from "@bitwarden/common/tools/public-classifier"; import { deepFreeze } from "@bitwarden/common/tools/util"; -import { SdkPasswordRandomizer } from "../../engine"; +import { PasswordRandomizer, SdkPasswordRandomizer } from "../../engine"; import { DynamicPasswordPolicyConstraints, passwordLeastPrivilege } from "../../policies"; import { GeneratorDependencyProvider } from "../../providers"; import { CredentialGenerator, PasswordGeneratorSettings } from "../../types"; @@ -30,6 +30,9 @@ const password: GeneratorMetadata = deepFreeze({ create( dependencies: GeneratorDependencyProvider, ): CredentialGenerator { + if (dependencies.sdk == undefined) { + return new PasswordRandomizer(dependencies.randomizer, dependencies.now); + } return new SdkPasswordRandomizer(dependencies.sdk, dependencies.now); }, }, diff --git a/libs/tools/generator/core/src/providers/generator-dependency-provider.ts b/libs/tools/generator/core/src/providers/generator-dependency-provider.ts index 8700bbc8a24..a6dbbeaa537 100644 --- a/libs/tools/generator/core/src/providers/generator-dependency-provider.ts +++ b/libs/tools/generator/core/src/providers/generator-dependency-provider.ts @@ -1,6 +1,6 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { RestClient } from "@bitwarden/common/tools/integration/rpc"; +import { BitwardenClient } from "@bitwarden/sdk-internal"; import { Randomizer } from "../abstractions"; @@ -10,6 +10,6 @@ export type GeneratorDependencyProvider = { // FIXME: introduce `I18nKeyOrLiteral` into forwarder // structures and remove this dependency i18nService: I18nService; - sdk: SdkService; + sdk?: BitwardenClient; now: () => number; }; diff --git a/libs/tools/generator/core/src/providers/generator-metadata-provider.spec.ts b/libs/tools/generator/core/src/providers/generator-metadata-provider.spec.ts index f79bb986325..39ff74ad901 100644 --- a/libs/tools/generator/core/src/providers/generator-metadata-provider.spec.ts +++ b/libs/tools/generator/core/src/providers/generator-metadata-provider.spec.ts @@ -5,6 +5,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { Account } from "@bitwarden/common/auth/abstractions/account.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LegacyEncryptorProvider } from "@bitwarden/common/tools/cryptography/legacy-encryptor-provider"; import { UserEncryptor } from "@bitwarden/common/tools/cryptography/user-encryptor.abstraction"; import { @@ -95,6 +96,8 @@ const SomePolicyService = mock(); const SomeExtensionService = mock(); +const SomeConfigService = mock; + const SomeSdkService = mock; const ApplicationProvider = { @@ -107,6 +110,9 @@ const ApplicationProvider = { /** Event monitoring and diagnostic interfaces */ log: disabledSemanticLoggerProvider, + /** Feature flag retrieval */ + configService: SomeConfigService, + /** SDK access for password generation */ sdk: SomeSdkService, } as unknown as SystemServiceProvider;