1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-19 10:54:00 +00:00

Revert "Remove feature flag check from password generation (#18003)" (#18794)

This reverts commit 7c6d98b50e.
This commit is contained in:
Alex Dragovich
2026-02-05 13:33:44 -08:00
committed by jaasen-livefront
parent 48d06c3a45
commit 3a8f956222
13 changed files with 54 additions and 35 deletions

View File

@@ -1110,7 +1110,6 @@ export default class MainBackground {
this.logService,
this.platformUtilsService,
this.configService,
this.sdkService,
),
);

View File

@@ -928,7 +928,6 @@ export class ServiceContainer {
this.logService,
this.platformUtilsService,
this.configService,
this.sdkService,
),
);

View File

@@ -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<PlatformUtilsService>;
let mockConfigService: ConfigService;
let mockSdkService: SdkService;
beforeEach(() => {
jest.resetAllMocks();
@@ -33,7 +31,6 @@ describe("SystemServiceProvider", () => {
mockLogger = mock<LogService>();
mockEnvironment = mock<PlatformUtilsService>();
mockConfigService = mock<ConfigService>();
mockSdkService = mock<SdkService>();
});
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);

View File

@@ -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,
};
}

View File

@@ -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({

View File

@@ -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<SystemServiceProvi
}),
safeProvider({
provide: GENERATOR_SERVICE_PROVIDER,
useFactory: async (
useFactory: (
system: SystemServiceProvider,
random: Randomizer,
encryptor: LegacyEncryptorProvider,
@@ -139,19 +141,25 @@ export const SYSTEM_SERVICE_PROVIDER = new SafeInjectionToken<SystemServiceProvi
now: Date.now,
} satisfies UserStateSubjectDependencyProvider;
const featureFlagObs$ = from(
system.configService.getFeatureFlag(FeatureFlag.UseSdkPasswordGenerators),
);
let featureFlag: boolean = false;
featureFlagObs$.pipe(take(1)).subscribe((ff) => (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,
};

View File

@@ -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<PasswordGenerationOptions>
{
/** 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,

View File

@@ -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<GeneratorDependencyProvider>();
nonSdkDependencyProvider.sdk = undefined;
it("returns a password randomizer", () => {
expect(effPassphrase.engine.create(nonSdkDependencyProvider)).toBeInstanceOf(
PasswordRandomizer,
);
});
});
describe("profiles[account]", () => {
let accountProfile: CoreProfileMetadata<PassphraseGenerationOptions> | null = null;
beforeEach(() => {

View File

@@ -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<PassphraseGenerationOptions> = {
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<PassphraseGenerationOptions> {
if (dependencies.sdk == undefined) {
return new PasswordRandomizer(dependencies.randomizer, dependencies.now);
}
return new SdkPasswordRandomizer(dependencies.sdk, dependencies.now);
},
},

View File

@@ -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<GeneratorDependencyProvider>();
nonSdkDependencyProvider.sdk = undefined;
it("returns a password randomizer", () => {
expect(password.engine.create(nonSdkDependencyProvider)).toBeInstanceOf(PasswordRandomizer);
});
});
describe("profiles[account]", () => {
let accountProfile: CoreProfileMetadata<PasswordGenerationOptions> = null!;
beforeEach(() => {

View File

@@ -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<PasswordGeneratorSettings> = deepFreeze({
create(
dependencies: GeneratorDependencyProvider,
): CredentialGenerator<PasswordGeneratorSettings> {
if (dependencies.sdk == undefined) {
return new PasswordRandomizer(dependencies.randomizer, dependencies.now);
}
return new SdkPasswordRandomizer(dependencies.sdk, dependencies.now);
},
},

View File

@@ -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;
};

View File

@@ -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<PolicyService>();
const SomeExtensionService = mock<ExtensionService>();
const SomeConfigService = mock<ConfigService>;
const SomeSdkService = mock<BitwardenClient>;
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;