diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 6c7c43c4dac..2f402b15dd5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -83,8 +83,6 @@ libs/common/src/platform @bitwarden/team-platform-dev libs/common/spec @bitwarden/team-platform-dev libs/common/src/state-migrations @bitwarden/team-platform-dev libs/platform @bitwarden/team-platform-dev -# Node-specifc platform files -libs/node @bitwarden/team-key-management-dev # Web utils used across app and connectors apps/web/src/utils/ @bitwarden/team-platform-dev # Web core and shared files @@ -146,6 +144,8 @@ apps/cli/src/key-management @bitwarden/team-key-management-dev libs/key-management @bitwarden/team-key-management-dev libs/key-management-ui @bitwarden/team-key-management-dev libs/common/src/key-management @bitwarden/team-key-management-dev +# Node-cryptofunction service +libs/node @bitwarden/team-key-management-dev apps/desktop/desktop_native/core/src/biometric/ @bitwarden/team-key-management-dev apps/desktop/src/services/native-messaging.service.ts @bitwarden/team-key-management-dev diff --git a/.github/DISCUSSION_TEMPLATE/password-manager.yml b/.github/DISCUSSION_TEMPLATE/password-manager.yml index bc3938c1962..1d464ca9504 100644 --- a/.github/DISCUSSION_TEMPLATE/password-manager.yml +++ b/.github/DISCUSSION_TEMPLATE/password-manager.yml @@ -3,7 +3,7 @@ body: - type: markdown attributes: value: | - If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. For feature requests and community discussion, please visit https://community.bitwarden.com/ + If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. - type: dropdown attributes: label: Select Topic Area diff --git a/.github/DISCUSSION_TEMPLATE/secrets-manager.yml b/.github/DISCUSSION_TEMPLATE/secrets-manager.yml index bc3938c1962..1d464ca9504 100644 --- a/.github/DISCUSSION_TEMPLATE/secrets-manager.yml +++ b/.github/DISCUSSION_TEMPLATE/secrets-manager.yml @@ -3,7 +3,7 @@ body: - type: markdown attributes: value: | - If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. For feature requests and community discussion, please visit https://community.bitwarden.com/ + If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. - type: dropdown attributes: label: Select Topic Area diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 30134e33e48..d373013d4af 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -262,6 +262,7 @@ import { PhishingDetectionService } from "../phishing-detection/background/phish import { BrowserApi } from "../platform/browser/browser-api"; import { flagEnabled } from "../platform/flags"; import { IpcBackgroundService } from "../platform/ipc/ipc-background.service"; +import { IpcContentScriptManagerService } from "../platform/ipc/ipc-content-script-manager.service"; import { UpdateBadge } from "../platform/listeners/update-badge"; /* eslint-disable no-restricted-imports */ import { ChromeMessageSender } from "../platform/messaging/chrome-message.sender"; @@ -406,6 +407,7 @@ export default class MainBackground { inlineMenuFieldQualificationService: InlineMenuFieldQualificationService; taskService: TaskService; + ipcContentScriptManagerService: IpcContentScriptManagerService; ipcService: IpcService; onUpdatedRan: boolean; @@ -1333,6 +1335,7 @@ export default class MainBackground { this.logService.error("Failed to check phishing detection feature flag", error), ); + this.ipcContentScriptManagerService = new IpcContentScriptManagerService(this.configService); this.ipcService = new IpcBackgroundService(this.logService); } diff --git a/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts b/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts new file mode 100644 index 00000000000..e5fe95e2018 --- /dev/null +++ b/apps/browser/src/platform/ipc/ipc-content-script-manager.service.ts @@ -0,0 +1,42 @@ +import { mergeMap } from "rxjs"; + +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; + +import { BrowserApi } from "../browser/browser-api"; + +const IPC_CONTENT_SCRIPT_ID = "ipc-content-script"; + +export class IpcContentScriptManagerService { + constructor(configService: ConfigService) { + if (!BrowserApi.isManifestVersion(3)) { + // IPC not supported on MV2 + return; + } + + configService + .getFeatureFlag$(FeatureFlag.IpcChannelFramework) + .pipe( + mergeMap(async (enabled) => { + if (!enabled) { + return; + } + + try { + await BrowserApi.unregisterContentScriptsMv3({ ids: [IPC_CONTENT_SCRIPT_ID] }); + } catch { + // Ignore errors + } + + await BrowserApi.registerContentScriptsMv3([ + { + id: IPC_CONTENT_SCRIPT_ID, + matches: ["https://*/*"], + js: ["content/ipc-content-script.js"], + }, + ]); + }), + ) + .subscribe(); + } +} diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-verify.component.ts b/apps/web/src/app/auth/settings/two-factor/two-factor-verify.component.ts index 98cb7199d2a..a153a9ec56a 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-verify.component.ts +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-verify.component.ts @@ -99,6 +99,7 @@ export class TwoFactorVerifyComponent { case -1 as TwoFactorProviderType: return this.i18nService.t("recoveryCodeTitle"); case TwoFactorProviderType.Duo: + case TwoFactorProviderType.OrganizationDuo: return "Duo"; case TwoFactorProviderType.Email: return this.i18nService.t("emailTitle"); diff --git a/apps/web/src/connectors/duo-redirect.scss b/apps/web/src/connectors/duo-redirect.scss deleted file mode 100644 index a4c7f9b25b7..00000000000 --- a/apps/web/src/connectors/duo-redirect.scss +++ /dev/null @@ -1 +0,0 @@ -@import "../scss/styles.scss"; diff --git a/apps/web/src/connectors/duo-redirect.spec.ts b/apps/web/src/connectors/duo-redirect.spec.ts new file mode 100644 index 00000000000..c0498861ba0 --- /dev/null +++ b/apps/web/src/connectors/duo-redirect.spec.ts @@ -0,0 +1,51 @@ +import { redirectToDuoFrameless } from "./duo-redirect"; + +describe("duo-redirect", () => { + describe("redirectToDuoFrameless", () => { + beforeEach(() => { + Object.defineProperty(window, "location", { + value: { href: "" }, + writable: true, + }); + }); + + it("should redirect to a valid Duo URL", () => { + const validUrl = "https://api-123.duosecurity.com/auth"; + redirectToDuoFrameless(validUrl); + expect(window.location.href).toBe(validUrl); + }); + + it("should redirect to a valid Duo Federal URL", () => { + const validUrl = "https://api-123.duofederal.com/auth"; + redirectToDuoFrameless(validUrl); + expect(window.location.href).toBe(validUrl); + }); + + it("should throw an error for an invalid URL", () => { + const invalidUrl = "https://malicious-site.com"; + expect(() => redirectToDuoFrameless(invalidUrl)).toThrow("Invalid redirect URL"); + }); + + it("should throw an error for an malicious URL with valid redirect embedded", () => { + const invalidUrl = "https://malicious-site.com\\@api-123.duosecurity.com/auth"; + expect(() => redirectToDuoFrameless(invalidUrl)).toThrow("Invalid redirect URL"); + }); + + it("should throw an error for a non-HTTPS URL", () => { + const nonHttpsUrl = "http://api-123.duosecurity.com/auth"; + expect(() => redirectToDuoFrameless(nonHttpsUrl)).toThrow("Invalid redirect URL"); + }); + + it("should throw an error for a URL with an invalid hostname", () => { + const invalidHostnameUrl = "https://api-123.invalid.com"; + expect(() => redirectToDuoFrameless(invalidHostnameUrl)).toThrow("Invalid redirect URL"); + }); + + it("should throw an error for a URL with credentials", () => { + const UrlWithCredentials = "https://api-123.duosecurity.com:password@evil/attack"; + expect(() => redirectToDuoFrameless(UrlWithCredentials)).toThrow( + "Invalid redirect URL: embedded credentials not allowed", + ); + }); + }); +}); diff --git a/apps/web/src/connectors/duo-redirect.ts b/apps/web/src/connectors/duo-redirect.ts index c19e056d306..d1841247962 100644 --- a/apps/web/src/connectors/duo-redirect.ts +++ b/apps/web/src/connectors/duo-redirect.ts @@ -1,14 +1,8 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { getQsParam } from "./common"; import { TranslationService } from "./translation.service"; -// FIXME: Remove when updating file. Eslint update -// eslint-disable-next-line @typescript-eslint/no-require-imports -require("./duo-redirect.scss"); - const mobileDesktopCallback = "bitwarden://duo-callback"; -let localeService: TranslationService = null; +let localeService: TranslationService | null = null; window.addEventListener("load", async () => { const redirectUrl = getQsParam("duoFramelessUrl"); @@ -18,9 +12,18 @@ window.addEventListener("load", async () => { return; } - const client = getQsParam("client"); - const code = getQsParam("code"); - const state = getQsParam("state"); + const client: string | null = getQsParam("client"); + const code: string | null = getQsParam("code"); + const state: string | null = getQsParam("state"); + if (!client) { + throw new Error("client is null"); + } + if (!code) { + throw new Error("code is null"); + } + if (!state) { + throw new Error("state is null"); + } localeService = new TranslationService(navigator.language, "locales"); await localeService.init(); @@ -53,16 +56,28 @@ window.addEventListener("load", async () => { * validate the Duo AuthUrl and redirect to it. * @param redirectUrl the duo auth url */ -function redirectToDuoFrameless(redirectUrl: string) { - const validateUrl = new URL(redirectUrl); - const validDuoUrl = - validateUrl.protocol === "https:" && - (validateUrl.hostname.endsWith(".duosecurity.com") || - validateUrl.hostname.endsWith(".duofederal.com")); - - if (!validDuoUrl) { +export function redirectToDuoFrameless(redirectUrl: string) { + // Regex to match a valid duo redirect URL + /** + * This regex checks for the following: + * The string must start with "https://api-" + * Followed by a subdomain that can contain letters, numbers + * Followed by either "duosecurity.com" or "duofederal.com" + * This ensures that the redirect does not contain any malicious content + * and is a valid Duo URL. + * */ + const duoRedirectUrlRegex = /^https:\/\/api-[a-zA-Z0-9]+\.(duosecurity|duofederal)\.com/; + // Check if the redirect URL matches the regex + if (!duoRedirectUrlRegex.test(redirectUrl)) { throw new Error("Invalid redirect URL"); } + // At this point we know the URL to be valid, but we need to check for embedded credentials + const validateUrl = new URL(redirectUrl); + // URLs should not contain + // Check that no embedded credentials are present + if (validateUrl.username || validateUrl.password) { + throw new Error("Invalid redirect URL: embedded credentials not allowed"); + } window.location.href = decodeURIComponent(redirectUrl); } @@ -72,17 +87,23 @@ function redirectToDuoFrameless(redirectUrl: string) { * so browser, desktop, and mobile are not able to take advantage of the countdown timer or close button. */ function displayHandoffMessage(client: string) { - const content = document.getElementById("content"); + const content: HTMLElement | null = document.getElementById("content"); + if (!content) { + throw new Error("content element not found"); + } content.className = "text-center"; content.innerHTML = ""; const h1 = document.createElement("h1"); - const p = document.createElement("p"); + const p: HTMLElement = document.createElement("p"); + if (!localeService) { + throw new Error("localeService is not initialized"); + } h1.textContent = localeService.t("youSuccessfullyLoggedIn"); p.textContent = client == "web" - ? (p.textContent = localeService.t("thisWindowWillCloseIn5Seconds")) + ? localeService.t("thisWindowWillCloseIn5Seconds") : localeService.t("youMayCloseThisWindow"); h1.className = "font-weight-semibold"; @@ -102,11 +123,20 @@ function displayHandoffMessage(client: string) { }); content.appendChild(button); - // Countdown timer (closes tab upon completion) - let num = Number(p.textContent.match(/\d+/)[0]); + if (p.textContent === null) { + throw new Error("count down container is null"); + } + const counterString: string | null = p.textContent.match(/\d+/)?.[0] || null; + if (!counterString) { + throw new Error("count down time cannot be null"); + } + let num: number = Number(counterString); const interval = setInterval(() => { if (num > 1) { + if (p.textContent === null) { + throw new Error("count down container is null"); + } p.textContent = p.textContent.replace(String(num), String(num - 1)); num--; } else { diff --git a/apps/web/webpack.config.js b/apps/web/webpack.config.js index d172ea95c71..9ccccee21bf 100644 --- a/apps/web/webpack.config.js +++ b/apps/web/webpack.config.js @@ -142,7 +142,7 @@ const plugins = [ new HtmlWebpackPlugin({ template: "./src/connectors/duo-redirect.html", filename: "duo-redirect-connector.html", - chunks: ["connectors/duo-redirect"], + chunks: ["connectors/duo-redirect", "styles"], }), new HtmlWebpackPlugin({ template: "./src/404.html", diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index e445bff6eb1..8d9b362d784 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -56,6 +56,9 @@ export enum FeatureFlag { SecurityTasks = "security-tasks", CipherKeyEncryption = "cipher-key-encryption", PhishingDetection = "phishing-detection", + + /* Platform */ + IpcChannelFramework = "ipc-channel-framework", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -120,6 +123,9 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PrivateKeyRegeneration]: FALSE, [FeatureFlag.UserKeyRotationV2]: FALSE, [FeatureFlag.PM4154_BulkEncryptionService]: FALSE, + + /* Platform */ + [FeatureFlag.IpcChannelFramework]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; diff --git a/libs/common/src/tools/integration/integration-context.spec.ts b/libs/common/src/tools/integration/integration-context.spec.ts index 67a40afb337..33694aefea1 100644 --- a/libs/common/src/tools/integration/integration-context.spec.ts +++ b/libs/common/src/tools/integration/integration-context.spec.ts @@ -189,6 +189,33 @@ describe("IntegrationContext", () => { expect(result).toBe(""); }); + + it("extracts the hostname when extractHostname is true", () => { + const context = new IntegrationContext(EXAMPLE_META, null, i18n); + + const result = context.website( + { website: "https://www.example.com/path" }, + { extractHostname: true }, + ); + + expect(result).toBe("www.example.com"); + }); + + it("falls back to the full URL when Utils.getHost cannot extract the hostname", () => { + const context = new IntegrationContext(EXAMPLE_META, null, i18n); + + const result = context.website({ website: "invalid-url" }, { extractHostname: true }); + + expect(result).toBe("invalid-url"); + }); + + it("truncates the website to maxLength", () => { + const context = new IntegrationContext(EXAMPLE_META, null, i18n); + + const result = context.website({ website: "www.example.com" }, { maxLength: 3 }); + + expect(result).toBe("www"); + }); }); describe("generatedBy", () => { @@ -211,5 +238,15 @@ describe("IntegrationContext", () => { expect(result).toBe("result"); expect(i18n.t).toHaveBeenCalledWith("forwarderGeneratedByWithWebsite", "www.example.com"); }); + + it("truncates generated text to maxLength", () => { + const context = new IntegrationContext(EXAMPLE_META, null, i18n); + i18n.t.mockReturnValue("This is the result text"); + + const result = context.generatedBy({ website: null }, { maxLength: 4 }); + + expect(result).toBe("This"); + expect(i18n.t).toHaveBeenCalledWith("forwarderGeneratedBy", ""); + }); }); }); diff --git a/libs/common/src/tools/integration/integration-context.ts b/libs/common/src/tools/integration/integration-context.ts index 40648df6803..49edafc026b 100644 --- a/libs/common/src/tools/integration/integration-context.ts +++ b/libs/common/src/tools/integration/integration-context.ts @@ -79,24 +79,40 @@ export class IntegrationContext { /** look up the website the integration is working with. * @param request supplies information about the state of the extension site + * @param options optional parameters + * @param options.extractHostname when `true`, tries to extract the hostname from the website URL, returns full URL otherwise + * @param options.maxLength limits the length of the return value * @returns The website or an empty string if a website isn't available * @remarks `website` is usually supplied when generating a credential from the vault */ - website(request: IntegrationRequest) { - return request.website ?? ""; + website( + request: IntegrationRequest, + options?: { extractHostname?: boolean; maxLength?: number }, + ) { + let url = request.website ?? ""; + if (options?.extractHostname) { + url = Utils.getHost(url) ?? url; + } + return url.slice(0, options?.maxLength); } /** look up localized text indicating Bitwarden requested the forwarding address. * @param request supplies information about the state of the extension site + * @param options optional parameters + * @param options.extractHostname when `true`, extracts the hostname from the website URL + * @param options.maxLength limits the length of the return value * @returns localized text describing a generated forwarding address */ - generatedBy(request: IntegrationRequest) { - const website = this.website(request); + generatedBy( + request: IntegrationRequest, + options?: { extractHostname?: boolean; maxLength?: number }, + ) { + const website = this.website(request, { extractHostname: options?.extractHostname ?? false }); const descriptionId = website === "" ? "forwarderGeneratedBy" : "forwarderGeneratedByWithWebsite"; const description = this.i18n.t(descriptionId, website); - return description; + return description.slice(0, options?.maxLength); } } diff --git a/libs/importer/src/importers/base-importer.spec.ts b/libs/importer/src/importers/base-importer.spec.ts index 309bb7ca8c4..4e3cdb355be 100644 --- a/libs/importer/src/importers/base-importer.spec.ts +++ b/libs/importer/src/importers/base-importer.spec.ts @@ -1,6 +1,9 @@ import { CipherType } from "@bitwarden/common/vault/enums"; import { CardView } from "@bitwarden/common/vault/models/view/card.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; + +import { ImportResult } from "../models"; import { BaseImporter } from "./base-importer"; @@ -16,8 +19,169 @@ class FakeBaseImporter extends BaseImporter { parseXml(data: string): Document { return super.parseXml(data); } + + processFolder(result: ImportResult, folderName: string, addRelationship: boolean = true): void { + return super.processFolder(result, folderName, addRelationship); + } } +describe("processFolder method", () => { + let result: ImportResult; + const importer = new FakeBaseImporter(); + + beforeEach(() => { + result = { + folders: [], + folderRelationships: [], + collections: [], + collectionRelationships: [], + ciphers: [], + success: false, + errorMessage: "", + }; + }); + + it("should add a new folder and relationship when folderName is unique", () => { + // arrange + // a folder exists - but it is not the same as the one we are importing + result = { + folders: [{ name: "ABC" } as FolderView], + folderRelationships: [], + collections: [], + collectionRelationships: [], + ciphers: [{ name: "cipher1", id: "cipher1" } as CipherView], + success: false, + errorMessage: "", + }; + importer.processFolder(result, "Folder1"); + + expect(result.folders).toHaveLength(2); + expect(result.folders[0].name).toBe("ABC"); + expect(result.folders[1].name).toBe("Folder1"); + expect(result.folderRelationships).toHaveLength(1); + expect(result.folderRelationships[0]).toEqual([1, 1]); // cipher1 -> Folder1 + }); + + it("should not add duplicate folders and should add relationships", () => { + // setup + // folder called "Folder1" already exists + result = { + folders: [{ name: "Folder1" } as FolderView], + folderRelationships: [], + collections: [], + collectionRelationships: [], + ciphers: [{ name: "cipher1", id: "cipher1" } as CipherView], + success: false, + errorMessage: "", + }; + + // import an existing folder should not add to the result.folders + importer.processFolder(result, "Folder1"); + + expect(result.folders).toHaveLength(1); + expect(result.folders[0].name).toBe("Folder1"); + expect(result.folderRelationships).toHaveLength(1); + expect(result.folderRelationships[0]).toEqual([1, 0]); // cipher1 -> folder1 + }); + + it("should create parent folders for nested folder names but not duplicates", () => { + // arrange + result = { + folders: [ + { name: "Ancestor/Parent/Child" } as FolderView, + { name: "Ancestor" } as FolderView, + ], + folderRelationships: [], + collections: [], + collectionRelationships: [], + ciphers: [{ name: "cipher1", id: "cipher1" } as CipherView], + success: false, + errorMessage: "", + }; + + // act + // importing an existing folder with a relationship should not change the result.folders + // nor should it change the result.folderRelationships + importer.processFolder(result, "Ancestor/Parent/Child/Grandchild/GreatGrandchild"); + + expect(result.folders).toHaveLength(5); + expect(result.folders.map((f) => f.name)).toEqual([ + "Ancestor/Parent/Child", + "Ancestor", + "Ancestor/Parent/Child/Grandchild/GreatGrandchild", + "Ancestor/Parent/Child/Grandchild", + "Ancestor/Parent", + ]); + expect(result.folderRelationships).toHaveLength(1); + expect(result.folderRelationships[0]).toEqual([1, 2]); // cipher1 -> grandchild + }); + + it("should not affect existing relationships", () => { + // arrange + // "Parent" is a folder with no relationship + // "Child" is a folder with 2 ciphers + result = { + folders: [{ name: "Parent" } as FolderView, { name: "Parent/Child" } as FolderView], + folderRelationships: [ + [1, 1], + [2, 1], + ], + collections: [], + collectionRelationships: [], + ciphers: [ + { name: "cipher1", id: "cipher1" } as CipherView, + { name: "cipher2", id: "cipher2" } as CipherView, + { name: "cipher3", id: "cipher3" } as CipherView, + ], + success: false, + errorMessage: "", + }; + + // act + // importing an existing folder with a relationship should not change the result.folders + // nor should it change the result.folderRelationships + importer.processFolder(result, "Parent/Child/Grandchild"); + + expect(result.folders).toHaveLength(3); + expect(result.folders.map((f) => f.name)).toEqual([ + "Parent", + "Parent/Child", + "Parent/Child/Grandchild", + ]); + expect(result.folderRelationships).toHaveLength(3); + expect(result.folderRelationships[0]).toEqual([1, 1]); // cipher1 -> child + expect(result.folderRelationships[1]).toEqual([2, 1]); // cipher2 -> child + expect(result.folderRelationships[2]).toEqual([3, 2]); // cipher3 -> grandchild + }); + + it("should not add relationships if addRelationship is false", () => { + importer.processFolder(result, "Folder1", false); + + expect(result.folders).toHaveLength(1); + expect(result.folders[0].name).toBe("Folder1"); + expect(result.folderRelationships).toHaveLength(0); + }); + + it("should replace backslashes with forward slashes in folder names", () => { + importer.processFolder(result, "Parent\\Child\\Grandchild"); + + expect(result.folders).toHaveLength(3); + expect(result.folders.map((f) => f.name)).toEqual([ + "Parent/Child/Grandchild", + "Parent/Child", + "Parent", + ]); + }); + + it("should handle empty or null folder names gracefully", () => { + importer.processFolder(result, null); + importer.processFolder(result, ""); + + expect(result.folders).toHaveLength(0); + expect(result.folderRelationships).toHaveLength(0); + }); +}); + describe("BaseImporter class", () => { const importer = new FakeBaseImporter(); let cipher: CipherView; diff --git a/libs/importer/src/importers/base-importer.ts b/libs/importer/src/importers/base-importer.ts index 90af5344cfc..0594b6014e8 100644 --- a/libs/importer/src/importers/base-importer.ts +++ b/libs/importer/src/importers/base-importer.ts @@ -366,7 +366,7 @@ export abstract class BaseImporter { let folderIndex = result.folders.length; // Replace backslashes with forward slashes, ensuring we create sub-folders - folderName = folderName.replace("\\", "/"); + folderName = folderName.replace(/\\/g, "/"); let addFolder = true; for (let i = 0; i < result.folders.length; i++) { @@ -387,6 +387,17 @@ export abstract class BaseImporter { if (addRelationship) { result.folderRelationships.push([result.ciphers.length, folderIndex]); } + + // if the folder name is a/b/c/d, we need to create a/b/c and a/b and a + const parts = folderName.split("/"); + for (let i = parts.length - 1; i > 0; i--) { + const parentName = parts.slice(0, i).join("/") as string; + if (result.folders.find((c) => c.name === parentName) == null) { + const folder = new FolderView(); + folder.name = parentName; + result.folders.push(folder); + } + } } protected convertToNoteIfNeeded(cipher: CipherView) { diff --git a/libs/importer/src/importers/bitwarden/bitwarden-csv-importer.ts b/libs/importer/src/importers/bitwarden/bitwarden-csv-importer.ts index fab47b30b1a..abda9a04a8a 100644 --- a/libs/importer/src/importers/bitwarden/bitwarden-csv-importer.ts +++ b/libs/importer/src/importers/bitwarden/bitwarden-csv-importer.ts @@ -1,6 +1,5 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { CollectionView } from "@bitwarden/admin-console/common"; import { FieldType, SecureNoteType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -25,35 +24,11 @@ export class BitwardenCsvImporter extends BaseImporter implements Importer { if (this.organization && !this.isNullOrWhitespace(value.collections)) { const collections = (value.collections as string).split(","); collections.forEach((col) => { - let addCollection = true; - let collectionIndex = result.collections.length; - - for (let i = 0; i < result.collections.length; i++) { - if (result.collections[i].name === col) { - addCollection = false; - collectionIndex = i; - break; - } - } - - if (addCollection) { - const collection = new CollectionView(); - collection.name = col; - result.collections.push(collection); - } - - result.collectionRelationships.push([result.ciphers.length, collectionIndex]); - - // if the collection name is a/b/c/d, we need to create a/b/c and a/b and a - const parts = col.split("/"); - for (let i = parts.length - 1; i > 0; i--) { - const parentCollectionName = parts.slice(0, i).join("/") as string; - if (result.collections.find((c) => c.name === parentCollectionName) == null) { - const parentCollection = new CollectionView(); - parentCollection.name = parentCollectionName; - result.collections.push(parentCollection); - } - } + // here processFolder is used to create collections + // In an Organization folders are converted to collections + // see line just before this function terminates + // where all folders are turned to collections + this.processFolder(result, col); }); } else if (!this.organization) { this.processFolder(result, value.folder); @@ -125,6 +100,10 @@ export class BitwardenCsvImporter extends BaseImporter implements Importer { result.ciphers.push(cipher); }); + if (this.organization) { + this.moveFoldersToCollections(result); + } + result.success = true; return Promise.resolve(result); } diff --git a/libs/importer/src/importers/keeper/keeper-csv-importer.spec.ts b/libs/importer/src/importers/keeper/keeper-csv-importer.spec.ts index 69655eb9177..d7a4d487bcb 100644 --- a/libs/importer/src/importers/keeper/keeper-csv-importer.spec.ts +++ b/libs/importer/src/importers/keeper/keeper-csv-importer.spec.ts @@ -1,6 +1,9 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { testData as TestData } from "../spec-data/keeper-csv/testdata.csv"; +import { + testData as TestData, + testDataMultiCollection, +} from "../spec-data/keeper-csv/testdata.csv"; import { KeeperCsvImporter } from "./keeper-csv-importer"; @@ -121,4 +124,32 @@ describe("Keeper CSV Importer", () => { expect(result.collectionRelationships[1]).toEqual([1, 0]); expect(result.collectionRelationships[2]).toEqual([2, 1]); }); + + it("should create collections tree, with child collections and relationships", async () => { + importer.organizationId = Utils.newGuid(); + const result = await importer.parse(testDataMultiCollection); + expect(result != null).toBe(true); + + const collections = result.collections; + expect(collections).not.toBeNull(); + expect(collections.length).toBe(3); + + // collection with the cipher + const collections1 = collections.shift(); + expect(collections1.name).toBe("Foo/Baz/Bar"); + + //second level collection + const collections2 = collections.shift(); + expect(collections2.name).toBe("Foo/Baz"); + + //third level + const collections3 = collections.shift(); + expect(collections3.name).toBe("Foo"); + + // [Cipher, Folder] + expect(result.collectionRelationships.length).toBe(3); + expect(result.collectionRelationships[0]).toEqual([0, 0]); + expect(result.collectionRelationships[1]).toEqual([1, 1]); + expect(result.collectionRelationships[2]).toEqual([2, 2]); + }); }); diff --git a/libs/importer/src/importers/netwrix/netwrix-passwordsecure-csv-importer.spec.ts b/libs/importer/src/importers/netwrix/netwrix-passwordsecure-csv-importer.spec.ts index ff327daf04d..8736b3df0c8 100644 --- a/libs/importer/src/importers/netwrix/netwrix-passwordsecure-csv-importer.spec.ts +++ b/libs/importer/src/importers/netwrix/netwrix-passwordsecure-csv-importer.spec.ts @@ -1,6 +1,9 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { credentialsData } from "../spec-data/netwrix-csv/login-export.csv"; +import { + credentialsData, + credentialsDataWithFolders, +} from "../spec-data/netwrix-csv/login-export.csv"; import { NetwrixPasswordSecureCsvImporter } from "./netwrix-passwordsecure-csv-importer"; @@ -88,4 +91,18 @@ describe("Netwrix Password Secure CSV Importer", () => { expect(result.collectionRelationships[1]).toEqual([1, 1]); expect(result.collectionRelationships[2]).toEqual([2, 0]); }); + + it("should parse multiple collections", async () => { + importer.organizationId = Utils.newGuid(); + const result = await importer.parse(credentialsDataWithFolders); + + expect(result).not.toBeNull(); + expect(result.success).toBe(true); + expect(result.collections.length).toBe(3); + expect(result.collections[0].name).toBe("folder1/folder2/folder3"); + expect(result.collections[1].name).toBe("folder1/folder2"); + expect(result.collections[2].name).toBe("folder1"); + expect(result.collectionRelationships.length).toBe(1); + expect(result.collectionRelationships[0]).toEqual([0, 0]); + }); }); diff --git a/libs/importer/src/importers/passsordxp/passwordxp-csv-importer.spec.ts b/libs/importer/src/importers/passsordxp/passwordxp-csv-importer.spec.ts index 0decd1e2830..12cfbbe62bb 100644 --- a/libs/importer/src/importers/passsordxp/passwordxp-csv-importer.spec.ts +++ b/libs/importer/src/importers/passsordxp/passwordxp-csv-importer.spec.ts @@ -4,7 +4,10 @@ import { ImportResult } from "../../models/import-result"; import { dutchHeaders } from "../spec-data/passwordxp-csv/dutch-headers"; import { germanHeaders } from "../spec-data/passwordxp-csv/german-headers"; import { noFolder } from "../spec-data/passwordxp-csv/no-folder.csv"; -import { withFolders } from "../spec-data/passwordxp-csv/passwordxp-with-folders.csv"; +import { + withFolders, + withMultipleFolders, +} from "../spec-data/passwordxp-csv/passwordxp-with-folders.csv"; import { withoutFolders } from "../spec-data/passwordxp-csv/passwordxp-without-folders.csv"; import { PasswordXPCsvImporter } from "./passwordxp-csv-importer"; @@ -167,4 +170,22 @@ describe("PasswordXPCsvImporter", () => { expect(collectionRelationship).toEqual([4, 2]); collectionRelationship = result.collectionRelationships.shift(); }); + + it("should convert multi-level folders to collections when importing into an organization", async () => { + importer.organizationId = "someOrg"; + const result: ImportResult = await importer.parse(withMultipleFolders); + expect(result.success).toBe(true); + expect(result.ciphers.length).toBe(5); + + expect(result.collections.length).toBe(3); + expect(result.collections[0].name).toEqual("Test Folder"); + expect(result.collections[1].name).toEqual("Test Folder/Level 2 Folder"); + expect(result.collections[2].name).toEqual("Test Folder/Level 2 Folder/Level 3 Folder"); + + expect(result.collectionRelationships.length).toBe(4); + expect(result.collectionRelationships[0]).toEqual([1, 0]); + expect(result.collectionRelationships[1]).toEqual([2, 1]); + expect(result.collectionRelationships[2]).toEqual([3, 1]); + expect(result.collectionRelationships[3]).toEqual([4, 2]); + }); }); diff --git a/libs/importer/src/importers/roboform-csv-importer.spec.ts b/libs/importer/src/importers/roboform-csv-importer.spec.ts index dd385e10b8d..23604042a02 100644 --- a/libs/importer/src/importers/roboform-csv-importer.spec.ts +++ b/libs/importer/src/importers/roboform-csv-importer.spec.ts @@ -2,7 +2,7 @@ import { CipherType } from "@bitwarden/common/vault/enums"; import { RoboFormCsvImporter } from "./roboform-csv-importer"; import { data as dataNoFolder } from "./spec-data/roboform-csv/empty-folders"; -import { data as dataFolder } from "./spec-data/roboform-csv/with-folders"; +import { data as dataFolder, dataWithFolderHierarchy } from "./spec-data/roboform-csv/with-folders"; describe("Roboform CSV Importer", () => { beforeEach(() => { @@ -39,4 +39,19 @@ describe("Roboform CSV Importer", () => { expect(result.ciphers[4].notes).toBe("This is a safe note"); expect(result.ciphers[4].name).toBe("note - 2023-03-31"); }); + + it("should parse CSV data with folder hierarchy", async () => { + const importer = new RoboFormCsvImporter(); + const result = await importer.parse(dataWithFolderHierarchy); + expect(result != null).toBe(true); + + expect(result.folders.length).toBe(5); + expect(result.ciphers.length).toBe(5); + + expect(result.folders[0].name).toBe("folder1"); + expect(result.folders[1].name).toBe("folder2"); + expect(result.folders[2].name).toBe("folder2/folder3"); + expect(result.folders[3].name).toBe("folder1/folder2/folder3"); + expect(result.folders[4].name).toBe("folder1/folder2"); + }); }); diff --git a/libs/importer/src/importers/spec-data/keeper-csv/testdata.csv.ts b/libs/importer/src/importers/spec-data/keeper-csv/testdata.csv.ts index a40e97ff3fd..cfa51faecc6 100644 --- a/libs/importer/src/importers/spec-data/keeper-csv/testdata.csv.ts +++ b/libs/importer/src/importers/spec-data/keeper-csv/testdata.csv.ts @@ -2,3 +2,9 @@ export const testData = `"Foo","Bar","john.doe@example.com","1234567890abcdef"," "Foo","Bar 1","john.doe1@example.com","234567890abcdef1","https://an.example.com/","","","Account ID","12345","Org ID","54321" "Foo\\Baz","Bar 2","john.doe2@example.com","34567890abcdef12","https://another.example.com/","","","Account ID","23456","TFC:Keeper","otpauth://totp/Amazon:me@company.com?secret=JBSWY3DPEHPK3PXP&issuer=Amazon&algorithm=SHA1&digits=6&period=30" `; + +export const testDataMultiCollection = ` +"Foo\\Baz\\Bar","Bar 2","john.doe2@example.com","34567890abcdef12","https://another.example.com/","","","Account ID","23456","TFC:Keeper","otpauth://totp/Amazon:me@company.com?secret=JBSWY3DPEHPK3PXP&issuer=Amazon&algorithm=SHA1&digits=6&period=30" +"Foo\\Baz","Bar 2","john.doe2@example.com","34567890abcdef12","https://another.example.com/","","","Account ID","23456","TFC:Keeper","otpauth://totp/Amazon:me@company.com?secret=JBSWY3DPEHPK3PXP&issuer=Amazon&algorithm=SHA1&digits=6&period=30" +"Foo","Bar 2","john.doe2@example.com","34567890abcdef12","https://another.example.com/","","","Account ID","23456","TFC:Keeper","otpauth://totp/Amazon:me@company.com?secret=JBSWY3DPEHPK3PXP&issuer=Amazon&algorithm=SHA1&digits=6&period=30" +`; diff --git a/libs/importer/src/importers/spec-data/netwrix-csv/login-export.csv.ts b/libs/importer/src/importers/spec-data/netwrix-csv/login-export.csv.ts index 715dd8e0074..5b0fa0c5cb2 100644 --- a/libs/importer/src/importers/spec-data/netwrix-csv/login-export.csv.ts +++ b/libs/importer/src/importers/spec-data/netwrix-csv/login-export.csv.ts @@ -2,3 +2,6 @@ "folderOrCollection1";"tag1, tag2, tag3";"Test Entry 1";"someUser";"somePassword";"https://www.example.com";"some note for example.com";"someTOTPSeed" "folderOrCollection2";"tag2";"Test Entry 2";"jdoe";"})9+Kg2fz_O#W1§H1-0Zio";"www.123.com";"Description123";"anotherTOTP" "folderOrCollection1";"someTag";"Test Entry 3";"username";"password";"www.internetsite.com";"Information";""`; + +export const credentialsDataWithFolders = `"Organisationseinheit";"DataTags";"Beschreibung";"Benutzername";"Passwort";"Internetseite";"Informationen";"One-Time Passwort" +"folder1\\folder2\\folder3";"tag1, tag2, tag3";"Test Entry 1";"someUser";"somePassword";"https://www.example.com";"some note for example.com";"someTOTPSeed"`; diff --git a/libs/importer/src/importers/spec-data/passwordxp-csv/passwordxp-with-folders.csv.ts b/libs/importer/src/importers/spec-data/passwordxp-csv/passwordxp-with-folders.csv.ts index c7cfe825759..884929cfc8f 100644 --- a/libs/importer/src/importers/spec-data/passwordxp-csv/passwordxp-with-folders.csv.ts +++ b/libs/importer/src/importers/spec-data/passwordxp-csv/passwordxp-with-folders.csv.ts @@ -11,3 +11,17 @@ test;testtest;;http://test;test;27-3-2024 12:36:59;27-3-2024 12:36:59;;; [Cert folder\\Nested folder]; test2;testtest;;http://test;test;27-3-2024 12:36:59;27-3-2024 12:36:59;;;`; + +export const withMultipleFolders = `Title;User name;Account;URL;Password;Modified;Created;Expire on;Description;Modified by +>>> +Title2;Username2;Account2;http://URL2.com;12345678;27-3-2024 08:11:21;27-3-2024 08:11:21;;; + +[Test Folder] +Title Test 1;Username1;Account1;http://URL1.com;Password1;27-3-2024 08:10:52;27-3-2024 08:10:52;;; + +[Test Folder\\Level 2 Folder] +Certificate 1;;;;;27-3-2024 10:22:39;27-3-2024 10:22:39;;; +test;testtest;;http://test;test;27-3-2024 12:36:59;27-3-2024 12:36:59;;; + +[Test Folder\\Level 2 Folder\\Level 3 Folder] +test2;testtest;;http://test;test;27-3-2024 12:36:59;27-3-2024 12:36:59;;;`; diff --git a/libs/importer/src/importers/spec-data/roboform-csv/with-folders.ts b/libs/importer/src/importers/spec-data/roboform-csv/with-folders.ts index e836c6430f0..86757b79c86 100644 --- a/libs/importer/src/importers/spec-data/roboform-csv/with-folders.ts +++ b/libs/importer/src/importers/spec-data/roboform-csv/with-folders.ts @@ -4,3 +4,10 @@ Test,https://www.test.com/,https://www.test.com/,test@gmail.com,:testPassword,te LoginWebsite,https://login.Website.com/,https://login.Website.com/,test@outlook.com,123password,,folder2,"User ID$,,,txt,test@outlook.com","Password$,,,pwd,123password" Website,https://signin.website.com/,https://signin.website.com/,user@bitwarden.com,password123,Website ,folder3,"User ID$,,,txt,user@bitwarden.com","Password$,,,pwd,password123" note - 2023-03-31,,,,,This is a safe note,`; + +export const dataWithFolderHierarchy = `Name,Url,MatchUrl,Login,Pwd,Note,Folder,RfFieldsV2 +Bitwarden,https://bitwarden.com,https://bitwarden.com,user@bitwarden.com,password,,folder1,"User ID$,,,txt,user@bitwarden.com","Password$,,,pwd,password" +Test,https://www.test.com/,https://www.test.com/,test@gmail.com,:testPassword,test,folder1,"User ID$,,,txt,test@gmail.com","Password$,,,pwd,:testPassword" +LoginWebsite,https://login.Website.com/,https://login.Website.com/,test@outlook.com,123password,,folder2,"User ID$,,,txt,test@outlook.com","Password$,,,pwd,123password" +Website,https://signin.website.com/,https://signin.website.com/,user@bitwarden.com,password123,Website ,folder2\\folder3,"User ID$,,,txt,user@bitwarden.com","Password$,,,pwd,password123" +Website,https://signin.website.com/,https://signin.website.com/,user@bitwarden.com,password123,Website ,folder1\\folder2\\folder3,"User ID$,,,txt,user@bitwarden.com","Password$,,,pwd,password123"`; diff --git a/libs/tools/generator/core/src/integration/addy-io.spec.ts b/libs/tools/generator/core/src/integration/addy-io.spec.ts index 9c816330616..40d17e9d888 100644 --- a/libs/tools/generator/core/src/integration/addy-io.spec.ts +++ b/libs/tools/generator/core/src/integration/addy-io.spec.ts @@ -55,6 +55,10 @@ describe("Addy.io forwarder", () => { const result = AddyIo.forwarder.createForwardingEmail.body(null, context); + expect(context.generatedBy).toHaveBeenCalledWith(null, { + extractHostname: true, + maxLength: 200, + }); expect(result).toEqual({ domain: "domain", description: "generated by", diff --git a/libs/tools/generator/core/src/integration/addy-io.ts b/libs/tools/generator/core/src/integration/addy-io.ts index 631c5fdb510..93ffed3392a 100644 --- a/libs/tools/generator/core/src/integration/addy-io.ts +++ b/libs/tools/generator/core/src/integration/addy-io.ts @@ -39,7 +39,7 @@ const createForwardingEmail = Object.freeze({ body(request: IntegrationRequest, context: ForwarderContext) { return { domain: context.emailDomain(), - description: context.generatedBy(request), + description: context.generatedBy(request, { extractHostname: true, maxLength: 200 }), }; }, hasJsonPayload(response: Response) { diff --git a/libs/tools/generator/core/src/integration/firefox-relay.spec.ts b/libs/tools/generator/core/src/integration/firefox-relay.spec.ts index ed487b7f49f..08798b154b3 100644 --- a/libs/tools/generator/core/src/integration/firefox-relay.spec.ts +++ b/libs/tools/generator/core/src/integration/firefox-relay.spec.ts @@ -56,6 +56,11 @@ describe("Firefox Relay forwarder", () => { const result = FirefoxRelay.forwarder.createForwardingEmail.body(null, context); + expect(context.website).toHaveBeenCalledWith(null, { maxLength: 255 }); + expect(context.generatedBy).toHaveBeenCalledWith(null, { + extractHostname: true, + maxLength: 64, + }); expect(result).toEqual({ enabled: true, generated_for: "website", diff --git a/libs/tools/generator/core/src/integration/firefox-relay.ts b/libs/tools/generator/core/src/integration/firefox-relay.ts index 9f40a3631ff..f80de0c95dd 100644 --- a/libs/tools/generator/core/src/integration/firefox-relay.ts +++ b/libs/tools/generator/core/src/integration/firefox-relay.ts @@ -33,8 +33,8 @@ const createForwardingEmail = Object.freeze({ body(request: IntegrationRequest, context: ForwarderContext) { return { enabled: true, - generated_for: context.website(request), - description: context.generatedBy(request), + generated_for: context.website(request, { maxLength: 255 }), + description: context.generatedBy(request, { extractHostname: true, maxLength: 64 }), }; }, hasJsonPayload(response: Response) {