mirror of
https://github.com/bitwarden/browser
synced 2025-12-10 13:23:34 +00:00
[PM-26731] Add feature flag for chromium importers with ABE (#16926)
* Add missing browser to SUPPORTED_BROWSERS in windows.rs These were previously removed due to needing ABE support * Add feature flag for chromium importer with ABE * Fix tests for windows * Run cargo fmt --------- Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
aa12700ebc
commit
bbfdb60c34
@@ -173,6 +173,8 @@ mod tests {
|
||||
let map = get_supported_importers::<MockInstalledBrowserRetriever>();
|
||||
|
||||
let expected: HashSet<String> = HashSet::from([
|
||||
"bravecsv".to_string(),
|
||||
"chromecsv".to_string(),
|
||||
"chromiumcsv".to_string(),
|
||||
"edgecsv".to_string(),
|
||||
"operacsv".to_string(),
|
||||
@@ -192,7 +194,14 @@ mod tests {
|
||||
#[test]
|
||||
fn windows_specific_loaders_match_const_array() {
|
||||
let map = get_supported_importers::<MockInstalledBrowserRetriever>();
|
||||
let ids = ["chromiumcsv", "edgecsv", "operacsv", "vivaldicsv"];
|
||||
let ids = [
|
||||
"bravecsv",
|
||||
"chromecsv",
|
||||
"chromiumcsv",
|
||||
"edgecsv",
|
||||
"operacsv",
|
||||
"vivaldicsv",
|
||||
];
|
||||
|
||||
for id in ids {
|
||||
let loaders = get_loaders(&map, id);
|
||||
|
||||
@@ -16,7 +16,15 @@ use crate::util;
|
||||
//
|
||||
|
||||
// IMPORTANT adjust array size when enabling / disabling chromium importers here
|
||||
pub const SUPPORTED_BROWSERS: [BrowserConfig; 4] = [
|
||||
pub const SUPPORTED_BROWSERS: [BrowserConfig; 6] = [
|
||||
BrowserConfig {
|
||||
name: "Brave",
|
||||
data_dir: "AppData/Local/BraveSoftware/Brave-Browser/User Data",
|
||||
},
|
||||
BrowserConfig {
|
||||
name: "Chrome",
|
||||
data_dir: "AppData/Local/Google/Chrome/User Data",
|
||||
},
|
||||
BrowserConfig {
|
||||
name: "Chromium",
|
||||
data_dir: "AppData/Local/Chromium/User Data",
|
||||
|
||||
@@ -41,6 +41,7 @@ export enum FeatureFlag {
|
||||
/* Tools */
|
||||
DesktopSendUIRefresh = "desktop-send-ui-refresh",
|
||||
UseSdkPasswordGenerators = "pm-19976-use-sdk-password-generators",
|
||||
ChromiumImporterWithABE = "pm-25855-chromium-importer-abe",
|
||||
|
||||
/* DIRT */
|
||||
EventBasedOrganizationIntegrations = "event-based-organization-integrations",
|
||||
@@ -86,6 +87,7 @@ export const DefaultFeatureFlagValue = {
|
||||
/* Tools */
|
||||
[FeatureFlag.DesktopSendUIRefresh]: FALSE,
|
||||
[FeatureFlag.UseSdkPasswordGenerators]: FALSE,
|
||||
[FeatureFlag.ChromiumImporterWithABE]: FALSE,
|
||||
|
||||
/* DIRT */
|
||||
[FeatureFlag.EventBasedOrganizationIntegrations]: FALSE,
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import { map, Observable } from "rxjs";
|
||||
import { combineLatest, map, Observable } from "rxjs";
|
||||
|
||||
import { ClientType, DeviceType } from "@bitwarden/common/enums";
|
||||
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
|
||||
import { SemanticLogger } from "@bitwarden/common/tools/log";
|
||||
import { SystemServiceProvider } from "@bitwarden/common/tools/providers";
|
||||
|
||||
import { ImporterMetadata, Importers, ImportersMetadata } from "../metadata";
|
||||
import { DataLoader, ImporterMetadata, Importers, ImportersMetadata, Loader } from "../metadata";
|
||||
import { ImportType } from "../models/import-options";
|
||||
import { availableLoaders } from "../util";
|
||||
|
||||
@@ -13,8 +15,13 @@ export class DefaultImportMetadataService implements ImportMetadataServiceAbstra
|
||||
protected importers: ImportersMetadata = Importers;
|
||||
private logger: SemanticLogger;
|
||||
|
||||
private chromiumWithABE$: Observable<boolean>;
|
||||
|
||||
constructor(protected system: SystemServiceProvider) {
|
||||
this.logger = system.log({ type: "ImportMetadataService" });
|
||||
this.chromiumWithABE$ = this.system.configService.getFeatureFlag$(
|
||||
FeatureFlag.ChromiumImporterWithABE,
|
||||
);
|
||||
}
|
||||
|
||||
async init(): Promise<void> {
|
||||
@@ -23,13 +30,13 @@ export class DefaultImportMetadataService implements ImportMetadataServiceAbstra
|
||||
|
||||
metadata$(type$: Observable<ImportType>): Observable<ImporterMetadata> {
|
||||
const client = this.system.environment.getClientType();
|
||||
const capabilities$ = type$.pipe(
|
||||
map((type) => {
|
||||
const capabilities$ = combineLatest([type$, this.chromiumWithABE$]).pipe(
|
||||
map(([type, enabled]) => {
|
||||
if (!this.importers) {
|
||||
return { type, loaders: [] };
|
||||
}
|
||||
|
||||
const loaders = availableLoaders(this.importers, type, client);
|
||||
const loaders = this.availableLoaders(this.importers, type, client, enabled);
|
||||
|
||||
if (!loaders || loaders.length === 0) {
|
||||
return { type, loaders: [] };
|
||||
@@ -48,4 +55,33 @@ export class DefaultImportMetadataService implements ImportMetadataServiceAbstra
|
||||
|
||||
return capabilities$;
|
||||
}
|
||||
|
||||
/** Determine the available loaders for the given import type and client, considering feature flags and environments */
|
||||
private availableLoaders(
|
||||
importers: ImportersMetadata,
|
||||
type: ImportType,
|
||||
client: ClientType,
|
||||
enabled: boolean,
|
||||
): DataLoader[] | undefined {
|
||||
let loaders = availableLoaders(importers, type, client);
|
||||
let includeABE = false;
|
||||
|
||||
if (enabled && (type === "bravecsv" || type === "chromecsv" || type === "edgecsv")) {
|
||||
try {
|
||||
const device = this.system.environment.getDevice();
|
||||
const isWindowsDesktop = device === DeviceType.WindowsDesktop;
|
||||
if (isWindowsDesktop) {
|
||||
includeABE = true;
|
||||
}
|
||||
} catch {
|
||||
includeABE = true;
|
||||
}
|
||||
}
|
||||
|
||||
// If the browser is unsupported, remove the chromium loader
|
||||
if (!includeABE) {
|
||||
loaders = loaders?.filter((loader) => loader !== Loader.chromium);
|
||||
}
|
||||
return loaders;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,19 +1,19 @@
|
||||
import { mock, MockProxy } from "jest-mock-extended";
|
||||
import { Subject, firstValueFrom } from "rxjs";
|
||||
import { BehaviorSubject, Subject, firstValueFrom } from "rxjs";
|
||||
|
||||
import { ClientType } from "@bitwarden/client-type";
|
||||
import { DeviceType } from "@bitwarden/common/enums";
|
||||
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
|
||||
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
|
||||
import { SystemServiceProvider } from "@bitwarden/common/tools/providers";
|
||||
|
||||
import { ImporterMetadata, Instructions } from "../metadata";
|
||||
import { ImporterMetadata, ImportersMetadata, Instructions, Loader } from "../metadata";
|
||||
import { ImportType } from "../models";
|
||||
|
||||
import { DefaultImportMetadataService } from "./default-import-metadata.service";
|
||||
import { ImportMetadataServiceAbstraction } from "./import-metadata.service.abstraction";
|
||||
|
||||
describe("ImportMetadataService", () => {
|
||||
let sut: ImportMetadataServiceAbstraction;
|
||||
let sut: DefaultImportMetadataService;
|
||||
let systemServiceProvider: MockProxy<SystemServiceProvider>;
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -34,15 +34,18 @@ describe("ImportMetadataService", () => {
|
||||
describe("metadata$", () => {
|
||||
let typeSubject: Subject<ImportType>;
|
||||
let mockLogger: { debug: jest.Mock };
|
||||
let featureFlagSubject: BehaviorSubject<boolean>;
|
||||
|
||||
const environment = mock<PlatformUtilsService>();
|
||||
environment.getClientType.mockReturnValue(ClientType.Desktop);
|
||||
|
||||
beforeEach(() => {
|
||||
typeSubject = new Subject<ImportType>();
|
||||
mockLogger = { debug: jest.fn() };
|
||||
featureFlagSubject = new BehaviorSubject<boolean>(false);
|
||||
|
||||
const configService = mock<ConfigService>();
|
||||
|
||||
const environment = mock<PlatformUtilsService>();
|
||||
environment.getClientType.mockReturnValue(ClientType.Desktop);
|
||||
configService.getFeatureFlag$.mockReturnValue(featureFlagSubject);
|
||||
|
||||
systemServiceProvider = mock<SystemServiceProvider>({
|
||||
configService,
|
||||
@@ -56,6 +59,7 @@ describe("ImportMetadataService", () => {
|
||||
|
||||
afterEach(() => {
|
||||
typeSubject.complete();
|
||||
featureFlagSubject.complete();
|
||||
});
|
||||
|
||||
it("should emit metadata when type$ emits", async () => {
|
||||
@@ -106,5 +110,76 @@ describe("ImportMetadataService", () => {
|
||||
"capabilities updated",
|
||||
);
|
||||
});
|
||||
|
||||
it("should update when feature flag changes", async () => {
|
||||
const testType: ImportType = "bravecsv"; // Use bravecsv which supports chromium loader
|
||||
const emissions: ImporterMetadata[] = [];
|
||||
|
||||
const subscription = sut.metadata$(typeSubject).subscribe((metadata) => {
|
||||
emissions.push(metadata);
|
||||
});
|
||||
|
||||
typeSubject.next(testType);
|
||||
featureFlagSubject.next(true);
|
||||
|
||||
// Wait for emissions
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(emissions).toHaveLength(2);
|
||||
expect(emissions[0].loaders).not.toContain(Loader.chromium);
|
||||
expect(emissions[1].loaders).toContain(Loader.file);
|
||||
|
||||
subscription.unsubscribe();
|
||||
});
|
||||
|
||||
it("should exclude chromium loader when ABE is disabled but on Windows Desktop", async () => {
|
||||
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||
featureFlagSubject.next(false);
|
||||
|
||||
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
||||
typeSubject.next(testType);
|
||||
|
||||
const result = await metadataPromise;
|
||||
|
||||
expect(result.loaders).not.toContain(Loader.chromium);
|
||||
expect(result.loaders).toContain(Loader.file);
|
||||
});
|
||||
|
||||
it("should exclude chromium loader when ABE is enabled but not on Windows Desktop", async () => {
|
||||
environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
|
||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||
featureFlagSubject.next(true);
|
||||
|
||||
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
||||
typeSubject.next(testType);
|
||||
|
||||
const result = await metadataPromise;
|
||||
|
||||
expect(result.loaders).not.toContain(Loader.chromium);
|
||||
expect(result.loaders).toContain(Loader.file);
|
||||
});
|
||||
|
||||
it("should include chromium loader when ABE is enabled and on Windows Desktop", async () => {
|
||||
// Set up importers to include bravecsv with chromium loader
|
||||
sut["importers"] = {
|
||||
bravecsv: {
|
||||
type: "bravecsv",
|
||||
loaders: [Loader.file, Loader.chromium],
|
||||
instructions: Instructions.chromium,
|
||||
},
|
||||
} as ImportersMetadata;
|
||||
|
||||
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||
featureFlagSubject.next(true);
|
||||
|
||||
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
||||
typeSubject.next(testType);
|
||||
|
||||
const result = await metadataPromise;
|
||||
|
||||
expect(result.loaders).toContain(Loader.chromium);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user