[PM-31004]: Fix Stackoverflow from Circular Group References (#991)

* Fix circular groups

* Simplify tests

.github/workflows/build.yml

@@ -62,25 +62,31 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
-
-      - name: Set up system dependencies
+      - name: Update NPM
         run: |
-          sudo apt-get update
-          sudo apt-get -y install libdbus-1-dev libsecret-1-dev pkg-config
+          npm install -g node-gyp
+          node-gyp install "$(node -v)"
+
+      - name: Keytar
+        run: |
+          keytarVersion=$(cat package.json | jq -r '.dependencies.keytar')
+          keytarTar="keytar-v$keytarVersion-napi-v3-linux-x64.tar"
+
+          keytarTarGz="$keytarTar.gz"
+          keytarUrl="https://github.com/atom/node-keytar/releases/download/v$keytarVersion/$keytarTarGz"
+
+          mkdir -p ./keytar/linux
+          wget "$keytarUrl" -O "./keytar/linux/$keytarTarGz"
+          tar -xvf "./keytar/linux/$keytarTarGz" -C ./keytar/linux
 
       - name: Install
         run: npm install
 
-      - name: Build native module
-        run: npm run build:native:release
-
       - name: Package CLI
         run: npm run dist:cli:lin
 
       - name: Zip
-        run: zip -j "dist-cli/bwdc-linux-$_PACKAGE_VERSION.zip" "dist-cli/linux/bwdc" "node_modules/dc-native/dc_native.linux-x64-gnu.node"
+        run: zip -j "dist-cli/bwdc-linux-$_PACKAGE_VERSION.zip" "dist-cli/linux/bwdc" "keytar/linux/build/Release/keytar.node"
 
       - name: Version Test
         run: |
@@ -134,20 +140,31 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
+      - name: Update NPM
+        run: |
+          npm install -g node-gyp
+          node-gyp install "$(node -v)"
+
+      - name: Keytar
+        run: |
+          keytarVersion=$(cat package.json | jq -r '.dependencies.keytar')
+          keytarTar="keytar-v$keytarVersion-napi-v3-darwin-x64.tar"
+
+          keytarTarGz="$keytarTar.gz"
+          keytarUrl="https://github.com/atom/node-keytar/releases/download/v$keytarVersion/$keytarTarGz"
+
+          mkdir -p ./keytar/macos
+          wget "$keytarUrl" -O "./keytar/macos/$keytarTarGz"
+          tar -xvf "./keytar/macos/$keytarTarGz" -C ./keytar/macos
 
       - name: Install
         run: npm install
 
-      - name: Build native module
-        run: npm run build:native:release
-
       - name: Package CLI
         run: npm run dist:cli:mac
 
       - name: Zip
-        run: zip -j "dist-cli/bwdc-macos-$_PACKAGE_VERSION.zip" "dist-cli/macos/bwdc" "node_modules/dc-native/dc_native.darwin-x64.node"
+        run: zip -j "dist-cli/bwdc-macos-$_PACKAGE_VERSION.zip" "dist-cli/macos/bwdc" "keytar/macos/build/Release/keytar.node"
 
       - name: Version Test
         run: |
@@ -198,23 +215,36 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
-        with:
-          targets: x86_64-pc-windows-msvc
+      - name: Update NPM
+        run: |
+          npm install -g node-gyp
+          node-gyp install $(node -v)
+
+      - name: Keytar
+        shell: pwsh
+        run: |
+          $keytarVersion = (Get-Content -Raw -Path ./package.json | ConvertFrom-Json).dependencies.keytar
+          $keytarTar = "keytar-v${keytarVersion}-napi-v3-{0}-x64.tar"
+          $keytarTarGz = "${keytarTar}.gz"
+          $keytarUrl = "https://github.com/atom/node-keytar/releases/download/v${keytarVersion}/${keytarTarGz}"
+
+          New-Item -ItemType directory -Path ./keytar/windows | Out-Null
+
+          Invoke-RestMethod -Uri $($keytarUrl -f "win32") -OutFile "./keytar/windows/$($keytarTarGz -f "win32")"
+
+          7z e "./keytar/windows/$($keytarTarGz -f "win32")" -o"./keytar/windows"
+
+          7z e "./keytar/windows/$($keytarTar -f "win32")" -o"./keytar/windows"
 
       - name: Install
         run: npm install
 
-      - name: Build native module
-        run: npm run build:native:release
-
       - name: Package CLI
         run: npm run dist:cli:win
 
       - name: Zip
         shell: cmd
-        run: 7z a .\dist-cli\bwdc-windows-%_PACKAGE_VERSION%.zip .\dist-cli\windows\bwdc.exe .\node_modules\dc-native\dc_native.win32-x64-msvc.node
+        run: 7z a .\dist-cli\bwdc-windows-%_PACKAGE_VERSION%.zip .\dist-cli\windows\bwdc.exe .\keytar\windows\keytar.node
 
       - name: Version Test
         shell: pwsh
@@ -260,10 +290,10 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
-        with:
-          targets: x86_64-pc-windows-msvc
+      - name: Update NPM
+        run: |
+          npm install -g node-gyp
+          node-gyp install $(node -v)
 
       - name: Print environment
         run: |
@@ -360,13 +390,15 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
+      - name: Update NPM
+        run: |
+          npm install -g node-gyp
+          node-gyp install "$(node -v)"
 
       - name: Set up environment
         run: |
           sudo apt-get update
-          sudo apt-get -y install pkg-config libxss-dev libsecret-1-dev libdbus-1-dev
+          sudo apt-get -y install pkg-config libxss-dev libsecret-1-dev
           sudo apt-get -y install rpm
 
       - name: NPM Install
@@ -418,8 +450,10 @@ jobs:
           cache-dependency-path: '**/package-lock.json'
           node-version: ${{ env._NODE_VERSION }}
 
-      - name: Set up Rust
-        uses: dtolnay/rust-toolchain@stable
+      - name: Update NPM
+        run: |
+          npm install -g node-gyp
+          node-gyp install "$(node -v)"
 
       - name: Print environment
         run: |

.gitignore

@@ -32,10 +32,6 @@ build
 build-cli
 .angular/cache
 
-# Rust build artifacts
-native/target
-native/*.node
-
 # Testing
 coverage*
 junit.xml*

.nvmrc

@@ -1 +1 @@
-v22
+v20

electron-builder.json

@@ -11,7 +11,6 @@
     "app": "build"
   },
   "afterSign": "scripts/notarize.js",
-  "asarUnpack": ["node_modules/dc-native/*.node"],
   "mac": {
     "artifactName": "Bitwarden-Connector-${version}-mac.${ext}",
     "category": "public.app-category.productivity",

eslint.config.mjs

@@ -23,7 +23,6 @@ export default [
       "eslint.config.mjs",
       "scripts/**/*.js",
       "**/node_modules/**",
-      "native/**",
     ],
   },
 

jslib/common/src/enums/stateVersion.ts

@@ -3,6 +3,5 @@ export enum StateVersion {
   Two = 2, // Move to a typed State object
   Three = 3, // Fix migration of users' premium status
   Four = 4, // Fix 'Never Lock' option by removing stale data
-  Five = 5, // Migrate Windows keychain credentials from keytar (UTF-8) to desktop_core (UTF-16)
-  Latest = Five,
+  Latest = Four,
 }

jslib/electron/src/services/electronRendererSecureStorage.service.ts

@@ -5,7 +5,7 @@ import { StorageOptions } from "@/jslib/common/src/models/domain/storageOptions"
 
 export class ElectronRendererSecureStorageService implements StorageService {
   async get<T>(key: string, options?: StorageOptions): Promise<T> {
-    const val = ipcRenderer.sendSync("nativeSecureStorage", {
+    const val = ipcRenderer.sendSync("keytar", {
       action: "getPassword",
       key: key,
       keySuffix: options?.keySuffix ?? "",
@@ -14,7 +14,7 @@ export class ElectronRendererSecureStorageService implements StorageService {
   }
 
   async has(key: string, options?: StorageOptions): Promise<boolean> {
-    const val = ipcRenderer.sendSync("nativeSecureStorage", {
+    const val = ipcRenderer.sendSync("keytar", {
       action: "hasPassword",
       key: key,
       keySuffix: options?.keySuffix ?? "",
@@ -23,7 +23,7 @@ export class ElectronRendererSecureStorageService implements StorageService {
   }
 
   async save(key: string, obj: any, options?: StorageOptions): Promise<any> {
-    ipcRenderer.sendSync("nativeSecureStorage", {
+    ipcRenderer.sendSync("keytar", {
       action: "setPassword",
       key: key,
       keySuffix: options?.keySuffix ?? "",
@@ -33,7 +33,7 @@ export class ElectronRendererSecureStorageService implements StorageService {
   }
 
   async remove(key: string, options?: StorageOptions): Promise<any> {
-    ipcRenderer.sendSync("nativeSecureStorage", {
+    ipcRenderer.sendSync("keytar", {
       action: "deletePassword",
       key: key,
       keySuffix: options?.keySuffix ?? "",

native/Cargo.toml

@@ -1,27 +0,0 @@
-[package]
-name = "dc_native"
-version = "0.1.0"
-edition = "2021"
-description = "Native keychain bindings for Bitwarden Directory Connector"
-license = "GPL-3.0"
-
-[lib]
-crate-type = ["cdylib"]
-name = "dc_native"
-
-[dependencies]
-anyhow = "=1.0.100"
-desktop_core = { git = "https://github.com/bitwarden/clients", rev = "00cf24972d944638bbd1adc00a0ae3eeabb6eb9a", package = "desktop_core" }
-napi = { version = "=3.3.0", features = ["async"] }
-napi-derive = "=3.2.5"
-
-[target.'cfg(windows)'.dependencies]
-scopeguard = "=1.2.0"
-widestring = "=1.2.0"
-windows = { version = "=0.61.1", features = [
-    "Win32_Foundation",
-    "Win32_Security_Credentials",
-] }
-
-[build-dependencies]
-napi-build = "=2.2.3"

native/build.rs

@@ -1,5 +0,0 @@
-extern crate napi_build;
-
-fn main() {
-    napi_build::setup();
-}

native/index.d.ts

@@ -1,34 +0,0 @@
-export declare namespace passwords {
-  /** The error message returned when a password is not found during retrieval or deletion. */
-  export const PASSWORD_NOT_FOUND: string;
-
-  /**
-   * Fetch the stored password from the keychain.
-   * Throws an Error with message PASSWORD_NOT_FOUND if the password does not exist.
-   */
-  export function getPassword(service: string, account: string): Promise<string>;
-
-  /**
-   * Save the password to the keychain. Adds an entry if none exists, otherwise updates it.
-   */
-  export function setPassword(service: string, account: string, password: string): Promise<void>;
-
-  /**
-   * Delete the stored password from the keychain.
-   * Throws an Error with message PASSWORD_NOT_FOUND if the password does not exist.
-   */
-  export function deletePassword(service: string, account: string): Promise<void>;
-
-  /**
-   * Check if OS secure storage is available.
-   */
-  export function isAvailable(): Promise<boolean>;
-
-  /**
-   * Migrate a credential previously stored by keytar (UTF-8 blob on Windows) to the UTF-16
-   * format used by desktop_core. No-ops on non-Windows platforms.
-   *
-   * Returns true if a migration was performed, false otherwise.
-   */
-  export function migrateKeytarPassword(service: string, account: string): Promise<boolean>;
-}

native/index.js

@@ -1,67 +0,0 @@
-const { existsSync } = require("fs");
-const { join } = require("path");
-
-const { platform, arch } = process;
-
-let nativeBinding = null;
-let loadError = null;
-
-function loadFirstAvailable(localFiles) {
-  for (const localFile of localFiles) {
-    const filePath = join(__dirname, localFile);
-    if (existsSync(filePath)) {
-      return require(filePath);
-    }
-  }
-  throw new Error(`Could not find dc-native binary. Run 'npm run build:native' to compile it.`);
-}
-
-switch (platform) {
-  case "win32":
-    switch (arch) {
-      case "x64":
-        nativeBinding = loadFirstAvailable(["dc_native.win32-x64-msvc.node"]);
-        break;
-      case "arm64":
-        nativeBinding = loadFirstAvailable(["dc_native.win32-arm64-msvc.node"]);
-        break;
-      default:
-        throw new Error(`Unsupported architecture on Windows: ${arch}`);
-    }
-    break;
-  case "darwin":
-    switch (arch) {
-      case "x64":
-        nativeBinding = loadFirstAvailable(["dc_native.darwin-x64.node"]);
-        break;
-      case "arm64":
-        nativeBinding = loadFirstAvailable(["dc_native.darwin-arm64.node"]);
-        break;
-      default:
-        throw new Error(`Unsupported architecture on macOS: ${arch}`);
-    }
-    break;
-  case "linux":
-    switch (arch) {
-      case "x64":
-        nativeBinding = loadFirstAvailable(["dc_native.linux-x64-gnu.node"]);
-        break;
-      case "arm64":
-        nativeBinding = loadFirstAvailable(["dc_native.linux-arm64-gnu.node"]);
-        break;
-      default:
-        throw new Error(`Unsupported architecture on Linux: ${arch}`);
-    }
-    break;
-  default:
-    throw new Error(`Unsupported platform: ${platform}, architecture: ${arch}`);
-}
-
-if (!nativeBinding) {
-  if (loadError) {
-    throw loadError;
-  }
-  throw new Error(`Failed to load dc-native binding`);
-}
-
-module.exports = nativeBinding;

native/package.json

@@ -1,15 +0,0 @@
-{
-  "name": "dc-native",
-  "version": "1.0.0",
-  "description": "Native keychain bindings for Bitwarden Directory Connector",
-  "main": "index.js",
-  "types": "index.d.ts",
-  "license": "GPL-3.0",
-  "scripts": {
-    "build": "napi build --platform",
-    "build:release": "napi build --platform --release"
-  },
-  "devDependencies": {
-    "@napi-rs/cli": "^3.0.0"
-  }
-}

native/src/lib.rs

@@ -1,70 +0,0 @@
-#[macro_use]
-extern crate napi_derive;
-
-#[napi]
-pub mod passwords {
-    /// The error message returned when a password is not found during retrieval or deletion.
-    #[napi]
-    pub const PASSWORD_NOT_FOUND: &str = desktop_core::password::PASSWORD_NOT_FOUND;
-
-    /// Fetch the stored password from the keychain.
-    /// Throws an Error with message PASSWORD_NOT_FOUND if the password does not exist.
-    #[napi]
-    pub async fn get_password(service: String, account: String) -> napi::Result<String> {
-        desktop_core::password::get_password(&service, &account)
-            .await
-            .map_err(|e| napi::Error::from_reason(e.to_string()))
-    }
-
-    /// Save the password to the keychain. Adds an entry if none exists, otherwise updates it.
-    #[napi]
-    pub async fn set_password(
-        service: String,
-        account: String,
-        password: String,
-    ) -> napi::Result<()> {
-        desktop_core::password::set_password(&service, &account, &password)
-            .await
-            .map_err(|e| napi::Error::from_reason(e.to_string()))
-    }
-
-    /// Delete the stored password from the keychain.
-    /// Throws an Error with message PASSWORD_NOT_FOUND if the password does not exist.
-    #[napi]
-    pub async fn delete_password(service: String, account: String) -> napi::Result<()> {
-        desktop_core::password::delete_password(&service, &account)
-            .await
-            .map_err(|e| napi::Error::from_reason(e.to_string()))
-    }
-
-    /// Check if OS secure storage is available.
-    #[napi]
-    pub async fn is_available() -> napi::Result<bool> {
-        desktop_core::password::is_available()
-            .await
-            .map_err(|e| napi::Error::from_reason(e.to_string()))
-    }
-
-    /// Migrate a credential that was stored by keytar (UTF-8 blob) to the new UTF-16 format
-    /// used by desktop_core on Windows. No-ops on non-Windows platforms.
-    ///
-    /// Returns true if a migration was performed, false if the credential was already in the
-    /// correct format or does not exist.
-    #[napi]
-    pub async fn migrate_keytar_password(service: String, account: String) -> napi::Result<bool> {
-        #[cfg(windows)]
-        {
-            crate::migration::migrate_keytar_password(&service, &account)
-                .await
-                .map_err(|e| napi::Error::from_reason(e.to_string()))
-        }
-        #[cfg(not(windows))]
-        {
-            let _ = (service, account);
-            Ok(false)
-        }
-    }
-}
-
-#[cfg(windows)]
-mod migration;

native/src/migration.rs

@@ -1,67 +0,0 @@
-/// Windows-only: migrates credentials stored by keytar (UTF-8 blob via CredWriteA) to the
-/// UTF-16 format expected by desktop_core (CredWriteW).
-///
-/// Keytar used CredWriteA on Windows, which stored the credential blob as raw UTF-8 bytes.
-/// desktop_core uses CredWriteW with a UTF-16 encoded blob. Reading old keytar credentials
-/// through desktop_core's get_password produces garbled output because the UTF-8 bytes are
-/// reinterpreted as UTF-16.
-///
-/// This function detects the old format by checking whether the raw blob bytes are valid UTF-8
-/// without null bytes (UTF-16 LE encoding of ASCII always contains null bytes). If so, it
-/// re-saves the credential using desktop_core's set_password (UTF-16 encoding).
-use anyhow::{anyhow, Result};
-use widestring::U16CString;
-use windows::{
-    core::PCWSTR,
-    Win32::Security::Credentials::{CredFree, CredReadW, CRED_TYPE_GENERIC},
-};
-
-pub async fn migrate_keytar_password(service: &str, account: &str) -> Result<bool> {
-    let target = format!("{}/{}", service, account);
-    let target_wide = U16CString::from_str(&target)?;
-
-    let mut credential = std::ptr::null_mut();
-    let result = unsafe {
-        CredReadW(
-            PCWSTR(target_wide.as_ptr()),
-            CRED_TYPE_GENERIC,
-            None,
-            &mut credential,
-        )
-    };
-
-    scopeguard::defer! {{
-        unsafe { CredFree(credential as *mut _) };
-    }};
-
-    if result.is_err() {
-        // Credential does not exist; nothing to migrate.
-        return Ok(false);
-    }
-
-    let blob_bytes: Vec<u8> = unsafe {
-        let blob_ptr = (*credential).CredentialBlob;
-        let blob_size = (*credential).CredentialBlobSize as usize;
-        if blob_ptr.is_null() || blob_size == 0 {
-            return Ok(false);
-        }
-        std::slice::from_raw_parts(blob_ptr, blob_size).to_vec()
-    };
-
-    // UTF-16 LE encoding of ASCII always contains null bytes (e.g. 'A' → 0x41 0x00).
-    // Keytar stored raw UTF-8 bytes which will never contain null bytes for valid JSON.
-    // If the blob is valid UTF-8 and contains no null bytes, it was written by keytar.
-    let blob_is_utf8 = std::str::from_utf8(&blob_bytes)
-        .map(|s| !s.contains('\0'))
-        .unwrap_or(false);
-
-    if !blob_is_utf8 {
-        // Already UTF-16 or unrecognised format; no migration needed.
-        return Ok(false);
-    }
-
-    let utf8_value = String::from_utf8(blob_bytes).map_err(|e| anyhow!(e))?;
-    desktop_core::password::set_password(service, account, &utf8_value).await?;
-
-    Ok(true)
-}

package.json

@@ -26,16 +26,15 @@
     "symlink:win": "rm -rf ./jslib && cmd /c mklink /J .\\jslib ..\\jslib",
     "symlink:mac": "npm run symlink:lin",
     "symlink:lin": "rm -rf ./jslib && ln -s ../jslib ./jslib",
-    "build:native": "cd native && npm install && npm run build",
-    "build:native:release": "cd native && npm install && npm run build:release",
-    "rebuild": "npm run build:native:release",
+    "rebuild": "electron-rebuild",
+    "reset": "rimraf --glob ./node_modules/keytar/* && npm install",
     "lint": "eslint . && prettier --check .",
     "lint:fix": "eslint . --fix",
     "build": "concurrently -n Main,Rend -c yellow,cyan \"npm run build:main\" \"npm run build:renderer\"",
     "build:main": "webpack --config webpack.main.cjs",
     "build:renderer": "webpack --config webpack.renderer.cjs",
     "build:renderer:watch": "webpack --config webpack.renderer.cjs --watch",
-    "build:dist": "npm run rebuild && npm run build",
+    "build:dist": "npm run reset && npm run rebuild && npm run build",
     "build:cli": "webpack --config webpack.cli.cjs",
     "build:cli:watch": "webpack --config webpack.cli.cjs --watch",
     "build:cli:prod": "cross-env NODE_ENV=production webpack --config webpack.cli.cjs",
@@ -79,6 +78,7 @@
     "@angular/build": "21.1.2",
     "@angular/compiler-cli": "21.1.1",
     "@electron/notarize": "2.5.0",
+    "@electron/rebuild": "4.0.1",
     "@microsoft/microsoft-graph-types": "2.43.1",
     "@ngtools/webpack": "21.1.2",
     "@types/inquirer": "8.2.10",
@@ -162,7 +162,7 @@
     "googleapis": "149.0.0",
     "https-proxy-agent": "7.0.6",
     "inquirer": "8.2.6",
-    "dc-native": "file:./native",
+    "keytar": "7.9.0",
     "ldapts": "8.1.3",
     "lowdb": "1.0.0",
     "ngx-toastr": "20.0.4",
@@ -175,7 +175,7 @@
     "zone.js": "0.16.0"
   },
   "engines": {
-    "node": "~22",
+    "node": "~20",
     "npm": "~10"
   },
   "lint-staged": {

src/bwdc.ts

@@ -24,8 +24,8 @@ import { AuthService } from "./services/auth.service";
 import { BatchRequestBuilder } from "./services/batch-request-builder";
 import { DefaultDirectoryFactoryService } from "./services/directory-factory.service";
 import { I18nService } from "./services/i18n.service";
+import { KeytarSecureStorageService } from "./services/keytarSecureStorage.service";
 import { LowdbStorageService } from "./services/lowdbStorage.service";
-import { NativeSecureStorageService } from "./services/nativeSecureStorage.service";
 import { SingleRequestBuilder } from "./services/single-request-builder";
 import { StateService } from "./services/state.service";
 import { StateMigrationService } from "./services/stateMigration.service";
@@ -100,7 +100,7 @@ export class Main {
     );
     this.secureStorageService = plaintextSecrets
       ? this.storageService
-      : new NativeSecureStorageService(applicationName);
+      : new KeytarSecureStorageService(applicationName);
 
     this.stateMigrationService = new StateMigrationService(
       this.storageService,

src/main/credential-storage-listener.ts

@@ -1,11 +1,11 @@
-import { passwords } from "dc-native";
 import { ipcMain } from "electron";
+import { deletePassword, getPassword, setPassword } from "keytar";
 
 export class DCCredentialStorageListener {
   constructor(private serviceName: string) {}
 
   init() {
-    ipcMain.on("nativeSecureStorage", async (event: any, message: any) => {
+    ipcMain.on("keytar", async (event: any, message: any) => {
       try {
         let serviceName = this.serviceName;
         message.keySuffix = "_" + (message.keySuffix ?? "");
@@ -16,14 +16,14 @@ export class DCCredentialStorageListener {
         let val: string | boolean = null;
         if (message.action && message.key) {
           if (message.action === "getPassword") {
-            val = await passwords.getPassword(serviceName, message.key);
+            val = await getPassword(serviceName, message.key);
           } else if (message.action === "hasPassword") {
-            const result = await passwords.getPassword(serviceName, message.key);
+            const result = await getPassword(serviceName, message.key);
             val = result != null;
           } else if (message.action === "setPassword" && message.value) {
-            await passwords.setPassword(serviceName, message.key, message.value);
+            await setPassword(serviceName, message.key, message.value);
           } else if (message.action === "deletePassword") {
-            await passwords.deletePassword(serviceName, message.key);
+            await deletePassword(serviceName, message.key);
           }
         }
         event.returnValue = val;

src/services/keytarSecureStorage.service.ts

@@ -0,0 +1,31 @@
+import { deletePassword, getPassword, setPassword } from "keytar";
+
+import { StorageService } from "@/jslib/common/src/abstractions/storage.service";
+
+export class KeytarSecureStorageService implements StorageService {
+  constructor(private serviceName: string) {}
+
+  get<T>(key: string): Promise<T> {
+    return getPassword(this.serviceName, key).then((val) => {
+      return JSON.parse(val) as T;
+    });
+  }
+
+  async has(key: string): Promise<boolean> {
+    return (await this.get(key)) != null;
+  }
+
+  save(key: string, obj: any): Promise<any> {
+    // keytar throws if you try to save a falsy value: https://github.com/atom/node-keytar/issues/86
+    // handle this by removing the key instead
+    if (!obj) {
+      return this.remove(key);
+    }
+
+    return setPassword(this.serviceName, key, JSON.stringify(obj));
+  }
+
+  remove(key: string): Promise<any> {
+    return deletePassword(this.serviceName, key);
+  }
+}

src/services/nativeSecureStorage.service.ts

@@ -1,28 +0,0 @@
-import { passwords } from "dc-native";
-
-import { StorageService } from "@/jslib/common/src/abstractions/storage.service";
-
-export class NativeSecureStorageService implements StorageService {
-  constructor(private serviceName: string) {}
-
-  get<T>(key: string): Promise<T> {
-    return passwords.getPassword(this.serviceName, key).then((val) => {
-      return JSON.parse(val) as T;
-    });
-  }
-
-  async has(key: string): Promise<boolean> {
-    return (await this.get(key)) != null;
-  }
-
-  save(key: string, obj: any): Promise<any> {
-    if (!obj) {
-      return this.remove(key);
-    }
-    return passwords.setPassword(this.serviceName, key, JSON.stringify(obj));
-  }
-
-  remove(key: string): Promise<any> {
-    return passwords.deletePassword(this.serviceName, key);
-  }
-}

src/services/stateMigration.service.ts

@@ -1,5 +1,3 @@
-import { passwords } from "dc-native";
-
 import { StateVersion } from "@/jslib/common/src/enums/stateVersion";
 import { StateMigrationService as BaseStateMigrationService } from "@/jslib/common/src/services/stateMigration.service";
 
@@ -63,13 +61,6 @@ export class StateMigrationService extends BaseStateMigrationService {
           break;
         case StateVersion.Two:
           await this.migrateStateFrom2To3();
-          break;
-        case StateVersion.Three:
-          await this.migrateStateFrom3To4();
-          break;
-        case StateVersion.Four:
-          await this.migrateStateFrom4To5();
-          break;
       }
       currentStateVersion += 1;
     }
@@ -177,50 +168,6 @@ export class StateMigrationService extends BaseStateMigrationService {
       }
     }
   }
-  /**
-   * Migrates Windows credential store entries previously written by keytar (UTF-8 blob) to
-   * the UTF-16 format expected by desktop_core. No-ops on non-Windows platforms.
-   *
-   * This migration is needed because keytar used CredWriteA (storing blobs as raw UTF-8 bytes)
-   * while desktop_core uses CredWriteW (storing blobs as UTF-16). Reading old keytar credentials
-   * through desktop_core produces garbled output without this migration.
-   */
-  protected async migrateStateFrom3To4(useSecureStorageForSecrets = true): Promise<void> {
-    if (useSecureStorageForSecrets && process.platform === "win32") {
-      const serviceName = "Bitwarden Directory Connector";
-      const authenticatedUserIds = await this.get<string[]>(StateKeys.authenticatedAccounts);
-
-      if (authenticatedUserIds?.length) {
-        const credentialKeys = [
-          SecureStorageKeys.ldap,
-          SecureStorageKeys.gsuite,
-          SecureStorageKeys.azure,
-          SecureStorageKeys.entra,
-          SecureStorageKeys.okta,
-          SecureStorageKeys.oneLogin,
-        ];
-
-        await Promise.all(
-          authenticatedUserIds.flatMap((userId) =>
-            credentialKeys.map((key) =>
-              passwords.migrateKeytarPassword(serviceName, `${userId}_${key}`),
-            ),
-          ),
-        );
-      }
-    }
-
-    const globals = await this.getGlobals();
-    globals.stateVersion = StateVersion.Four;
-    await this.set(StateKeys.global, globals);
-  }
-
-  protected async migrateStateFrom4To5(): Promise<void> {
-    const globals = await this.getGlobals();
-    globals.stateVersion = StateVersion.Five;
-    await this.set(StateKeys.global, globals);
-  }
-
   protected async migrateStateFrom2To3(useSecureStorageForSecrets = true): Promise<void> {
     if (useSecureStorageForSecrets) {
       const authenticatedUserIds = await this.get<string[]>(StateKeys.authenticatedAccounts);

src/services/sync.service.spec.ts

@@ -6,6 +6,8 @@ import { MessagingService } from "@/jslib/common/src/abstractions/messaging.serv
 import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";
 import { ApiService } from "@/jslib/common/src/services/api.service";
 
+import { GroupEntry } from "@/src/models/groupEntry";
+
 import { getSyncConfiguration } from "../../utils/openldap/config-fixtures";
 import { DirectoryFactoryService } from "../abstractions/directory-factory.service";
 import { DirectoryType } from "../enums/directoryType";
@@ -134,4 +136,134 @@ describe("SyncService", () => {
 
     expect(apiService.postPublicImportDirectory).not.toHaveBeenCalled();
   });
+
+  describe("nested and circular group handling", () => {
+    function createGroup(
+      name: string,
+      userExternalIds: string[] = [],
+      groupMemberReferenceIds: string[] = [],
+    ) {
+      return GroupEntry.fromJSON({
+        name,
+        referenceId: name,
+        externalId: name,
+        userMemberExternalIds: userExternalIds,
+        groupMemberReferenceIds: groupMemberReferenceIds,
+        users: [],
+      });
+    }
+
+    function setupSyncWithGroups(groups: GroupEntry[]) {
+      const mockDirectoryService = mock<LdapDirectoryService>();
+      mockDirectoryService.getEntries.mockResolvedValue([groups, []]);
+      directoryFactory.createService.mockReturnValue(mockDirectoryService);
+
+      stateService.getSync.mockResolvedValue(getSyncConfiguration({ groups: true, users: true }));
+      cryptoFunctionService.hash.mockResolvedValue(new ArrayBuffer(1));
+      stateService.getLastSyncHash.mockResolvedValue("unique hash");
+      singleRequestBuilder.buildRequest.mockReturnValue([
+        { members: [], groups: [], overwriteExisting: true, largeImport: false },
+      ]);
+    }
+
+    it("should handle simple circular reference (A ↔ B) without stack overflow", async () => {
+      const groupA = createGroup("GroupA", ["userA"], ["GroupB"]);
+      const groupB = createGroup("GroupB", ["userB"], ["GroupA"]);
+      setupSyncWithGroups([groupA, groupB]);
+
+      const [groups] = await syncService.sync(true, true);
+
+      const [a, b] = groups;
+      expect(a.userMemberExternalIds).toEqual(new Set(["userA", "userB"]));
+      expect(b.userMemberExternalIds).toEqual(new Set(["userA", "userB"]));
+    });
+
+    it("should handle longer circular chain (A → B → C → A) without stack overflow", async () => {
+      const groupA = createGroup("GroupA", ["userA"], ["GroupB"]);
+      const groupB = createGroup("GroupB", ["userB"], ["GroupC"]);
+      const groupC = createGroup("GroupC", ["userC"], ["GroupA"]);
+      setupSyncWithGroups([groupA, groupB, groupC]);
+
+      const [groups] = await syncService.sync(true, true);
+
+      const allUsers = new Set(["userA", "userB", "userC"]);
+      for (const group of groups) {
+        expect(group.userMemberExternalIds).toEqual(allUsers);
+      }
+    });
+
+    it("should handle diamond structure (A → [B, C] → D)", async () => {
+      const groupA = createGroup("GroupA", ["userA"], ["GroupB", "GroupC"]);
+      const groupB = createGroup("GroupB", ["userB"], ["GroupD"]);
+      const groupC = createGroup("GroupC", ["userC"], ["GroupD"]);
+      const groupD = createGroup("GroupD", ["userD"], []);
+      setupSyncWithGroups([groupA, groupB, groupC, groupD]);
+
+      const [groups] = await syncService.sync(true, true);
+
+      const [a, b, c, d] = groups;
+      expect(a.userMemberExternalIds).toEqual(new Set(["userA", "userB", "userC", "userD"]));
+      expect(b.userMemberExternalIds).toEqual(new Set(["userB", "userD"]));
+      expect(c.userMemberExternalIds).toEqual(new Set(["userC", "userD"]));
+      expect(d.userMemberExternalIds).toEqual(new Set(["userD"]));
+    });
+
+    it("should handle deep nesting with circular reference at leaf", async () => {
+      // Structure: A → B → C → D → B (cycle back to B)
+      const groupA = createGroup("GroupA", ["userA"], ["GroupB"]);
+      const groupB = createGroup("GroupB", ["userB"], ["GroupC"]);
+      const groupC = createGroup("GroupC", ["userC"], ["GroupD"]);
+      const groupD = createGroup("GroupD", ["userD"], ["GroupB"]);
+      setupSyncWithGroups([groupA, groupB, groupC, groupD]);
+
+      const [groups] = await syncService.sync(true, true);
+
+      const [a, b, c, d] = groups;
+      const cycleUsers = new Set(["userB", "userC", "userD"]);
+      expect(a.userMemberExternalIds).toEqual(new Set(["userA", ...cycleUsers]));
+      expect(b.userMemberExternalIds).toEqual(cycleUsers);
+      expect(c.userMemberExternalIds).toEqual(cycleUsers);
+      expect(d.userMemberExternalIds).toEqual(cycleUsers);
+    });
+
+    it("should handle complex structure with multiple cycles and shared members", async () => {
+      // Structure:
+      // A → [B, C]
+      // B → [D, E]
+      // C → [E, F]
+      // D → A (cycle)
+      // E → C (cycle)
+      // F → (leaf)
+      const groupA = createGroup("GroupA", ["userA"], ["GroupB", "GroupC"]);
+      const groupB = createGroup("GroupB", ["userB"], ["GroupD", "GroupE"]);
+      const groupC = createGroup("GroupC", ["userC"], ["GroupE", "GroupF"]);
+      const groupD = createGroup("GroupD", ["userD"], ["GroupA"]);
+      const groupE = createGroup("GroupE", ["userE"], ["GroupC"]);
+      const groupF = createGroup("GroupF", ["userF"], []);
+      setupSyncWithGroups([groupA, groupB, groupC, groupD, groupE, groupF]);
+
+      const [groups] = await syncService.sync(true, true);
+
+      const allUsers = new Set(["userA", "userB", "userC", "userD", "userE", "userF"]);
+      const a = groups.find((g) => g.name === "GroupA");
+      const b = groups.find((g) => g.name === "GroupB");
+      const c = groups.find((g) => g.name === "GroupC");
+      const d = groups.find((g) => g.name === "GroupD");
+      const e = groups.find((g) => g.name === "GroupE");
+      const f = groups.find((g) => g.name === "GroupF");
+
+      // A can reach all groups, so it gets all users
+      expect(a.userMemberExternalIds).toEqual(allUsers);
+      // B reaches D, E, and through cycles reaches everything
+      expect(b.userMemberExternalIds).toEqual(allUsers);
+      // C reaches E (which cycles back to C) and F
+      expect(c.userMemberExternalIds).toEqual(new Set(["userC", "userE", "userF"]));
+      // D cycles to A, which reaches everything
+      expect(d.userMemberExternalIds).toEqual(allUsers);
+      // E cycles to C, picking up C's descendants
+      expect(e.userMemberExternalIds).toEqual(new Set(["userC", "userE", "userF"]));
+      // F is a leaf
+      expect(f.userMemberExternalIds).toEqual(new Set(["userF"]));
+    });
+  });
 });

src/services/sync.service.ts

@@ -196,14 +196,27 @@ export class SyncService {
     return users == null ? null : users.filter((u) => u.email?.length <= 256);
   }
 
-  private flattenUsersToGroups(levelGroups: GroupEntry[], allGroups: GroupEntry[]): Set<string> {
+  private flattenUsersToGroups(
+    levelGroups: GroupEntry[],
+    allGroups: GroupEntry[],
+    visitedGroups?: Set<string>,
+  ): Set<string> {
     let allUsers = new Set<string>();
     if (allGroups == null) {
       return allUsers;
     }
+
     for (const group of levelGroups) {
+      const visited = visitedGroups ?? new Set<string>();
+
+      if (visited.has(group.referenceId)) {
+        continue;
+      }
+
+      visited.add(group.referenceId);
+
       const childGroups = allGroups.filter((g) => group.groupMemberReferenceIds.has(g.referenceId));
-      const childUsers = this.flattenUsersToGroups(childGroups, allGroups);
+      const childUsers = this.flattenUsersToGroups(childGroups, allGroups, visited);
       childUsers.forEach((id) => group.userMemberExternalIds.add(id));
       allUsers = new Set([...allUsers, ...group.userMemberExternalIds]);
     }

utils/openldap/example-ldifs/directory-circular-groups.ldif

@@ -0,0 +1,308 @@
+version: 1
+
+dn: dc=bitwarden,dc=com
+dc: bitwarden
+objectClass: dcObject
+objectClass: organization
+o: Bitwarden
+
+# Organizational Units
+dn: ou=Human Resources,dc=bitwarden,dc=com
+changetype: add
+ou: Human Resources
+objectClass: top
+objectClass: organizationalUnit
+
+dn: ou=Engineering,dc=bitwarden,dc=com
+changetype: add
+ou: Engineering
+objectClass: top
+objectClass: organizationalUnit
+
+dn: ou=Marketing,dc=bitwarden,dc=com
+changetype: add
+ou: Marketing
+objectClass: top
+objectClass: organizationalUnit
+
+# Users - Human Resources
+dn: cn=Roland Dyke,ou=Human Resources,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Roland Dyke
+sn: Dyke
+description: This is Roland Dyke's description
+facsimileTelephoneNumber: +1 804 674-5794
+l: San Francisco
+ou: Human Resources
+postalAddress: Human Resources$San Francisco
+telephoneNumber: +1 804 831-5121
+title: Supreme Human Resources Writer
+userPassword: Password1
+uid: DykeR
+givenName: Roland
+mail: DykeR@220af87272f04218bb8dd81d50fb19f5.bitwarden.com
+carLicense: 4CMGOJ
+departmentNumber: 2838
+employeeType: Contract
+homePhone: +1 804 936-4965
+initials: R. D.
+mobile: +1 804 592-3734
+pager: +1 804 285-2962
+roomNumber: 9890
+
+dn: cn=Teirtza Kara,ou=Human Resources,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Teirtza Kara
+sn: Kara
+description: This is Teirtza Kara's description
+facsimileTelephoneNumber: +1 206 759-2040
+l: San Francisco
+ou: Human Resources
+postalAddress: Human Resources$San Francisco
+telephoneNumber: +1 206 562-1407
+title: Junior Human Resources President
+userPassword: Password1
+uid: KaraT
+givenName: Teirtza
+mail: KaraT@c2afe8b3509f4a20b2b784841685bd74.bitwarden.com
+carLicense: O9GAN2
+departmentNumber: 3880
+employeeType: Employee
+homePhone: +1 206 154-4842
+initials: T. K.
+mobile: +1 206 860-1835
+pager: +1 206 684-1438
+roomNumber: 9079
+
+# Users - Engineering
+dn: cn=Alice Chen,ou=Engineering,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Alice Chen
+sn: Chen
+description: Senior DevOps Engineer
+l: Seattle
+ou: Engineering
+telephoneNumber: +1 206 555-0101
+title: Senior DevOps Engineer
+userPassword: Password1
+uid: ChenA
+givenName: Alice
+mail: ChenA@bitwarden.com
+employeeType: Employee
+
+dn: cn=Bob Martinez,ou=Engineering,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Bob Martinez
+sn: Martinez
+description: Platform Engineer
+l: Austin
+ou: Engineering
+telephoneNumber: +1 512 555-0102
+title: Platform Engineer
+userPassword: Password1
+uid: MartinezB
+givenName: Bob
+mail: MartinezB@bitwarden.com
+employeeType: Employee
+
+dn: cn=Carol Williams,ou=Engineering,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Carol Williams
+sn: Williams
+description: QA Lead
+l: Denver
+ou: Engineering
+telephoneNumber: +1 303 555-0103
+title: QA Lead
+userPassword: Password1
+uid: WilliamsC
+givenName: Carol
+mail: WilliamsC@bitwarden.com
+employeeType: Employee
+
+dn: cn=David Kim,ou=Engineering,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: David Kim
+sn: Kim
+description: QA Engineer
+l: Portland
+ou: Engineering
+telephoneNumber: +1 503 555-0104
+title: QA Engineer
+userPassword: Password1
+uid: KimD
+givenName: David
+mail: KimD@bitwarden.com
+employeeType: Contractor
+
+# Users - Marketing
+dn: cn=Eva Johnson,ou=Marketing,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Eva Johnson
+sn: Johnson
+description: Marketing Director
+l: New York
+ou: Marketing
+telephoneNumber: +1 212 555-0105
+title: Marketing Director
+userPassword: Password1
+uid: JohnsonE
+givenName: Eva
+mail: JohnsonE@bitwarden.com
+employeeType: Employee
+
+dn: cn=Frank Lee,ou=Marketing,dc=bitwarden,dc=com
+changetype: add
+objectClass: top
+objectClass: person
+objectClass: organizationalPerson
+objectClass: inetOrgPerson
+cn: Frank Lee
+sn: Lee
+description: Content Strategist
+l: Chicago
+ou: Marketing
+telephoneNumber: +1 312 555-0106
+title: Content Strategist
+userPassword: Password1
+uid: LeeF
+givenName: Frank
+mail: LeeF@bitwarden.com
+employeeType: Employee
+
+# ============================================================
+# GROUP HIERARCHY
+# ============================================================
+# Structure (arrows show "contains" relationship):
+#
+#   AllStaff
+#     ├── Engineering ◄────────────────┐ (CYCLE from Platform)
+#     │     ├── DevOps                 │
+#     │     │     └── Platform ────────┘
+#     │     └── QA
+#     ├── Marketing
+#     └── HR
+#
+#   Contractors ─── DevOps (diamond: second path to Platform)
+#
+#   TestNestA ◄──► TestNestB (simple bidirectional cycle)
+#
+# ============================================================
+
+# Leaf group - Platform team (CYCLES BACK to Engineering)
+dn: cn=Platform,dc=bitwarden,dc=com
+changetype: add
+cn: Platform
+member: cn=Bob Martinez,ou=Engineering,dc=bitwarden,dc=com
+member: cn=Engineering,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# DevOps group - contains Platform subgroup
+dn: cn=DevOps,dc=bitwarden,dc=com
+changetype: add
+cn: DevOps
+member: cn=Alice Chen,ou=Engineering,dc=bitwarden,dc=com
+member: cn=Platform,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# QA group
+dn: cn=QA,dc=bitwarden,dc=com
+changetype: add
+cn: QA
+member: cn=Carol Williams,ou=Engineering,dc=bitwarden,dc=com
+member: cn=David Kim,ou=Engineering,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# Engineering group - contains DevOps and QA subgroups
+dn: cn=Engineering,dc=bitwarden,dc=com
+changetype: add
+cn: Engineering
+member: cn=DevOps,dc=bitwarden,dc=com
+member: cn=QA,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# Marketing group
+dn: cn=Marketing,dc=bitwarden,dc=com
+changetype: add
+cn: Marketing
+member: cn=Eva Johnson,ou=Marketing,dc=bitwarden,dc=com
+member: cn=Frank Lee,ou=Marketing,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# HR group
+dn: cn=HR,dc=bitwarden,dc=com
+changetype: add
+cn: HR
+member: cn=Roland Dyke,ou=Human Resources,dc=bitwarden,dc=com
+member: cn=Teirtza Kara,ou=Human Resources,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# AllStaff - top-level group containing all departments
+dn: cn=AllStaff,dc=bitwarden,dc=com
+changetype: add
+cn: AllStaff
+member: cn=Engineering,dc=bitwarden,dc=com
+member: cn=Marketing,dc=bitwarden,dc=com
+member: cn=HR,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# Contractors group - creates diamond pattern (second path to Platform via DevOps)
+dn: cn=Contractors,dc=bitwarden,dc=com
+changetype: add
+cn: Contractors
+member: cn=DevOps,dc=bitwarden,dc=com
+member: cn=David Kim,ou=Engineering,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+# Simple bidirectional cycle test groups (preserved from original)
+dn: cn=TestNestA,dc=bitwarden,dc=com
+changetype: add
+cn: TestNestA
+member: cn=TestNestB,dc=bitwarden,dc=com
+member: cn=Roland Dyke,ou=Human Resources,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top
+
+dn: cn=TestNestB,dc=bitwarden,dc=com
+changetype: add
+cn: TestNestB
+member: cn=TestNestA,dc=bitwarden,dc=com
+member: cn=Teirtza Kara,ou=Human Resources,dc=bitwarden,dc=com
+objectclass: groupOfNames
+objectclass: top

webpack.main.cjs

@@ -58,7 +58,7 @@ const main = {
   ],
   externals: {
     "electron-reload": "commonjs2 electron-reload",
-    "dc-native": "commonjs2 dc-native",
+    keytar: "commonjs2 keytar",
   },
 };
 

webpack.renderer.cjs

@@ -55,9 +55,6 @@ const renderer = {
   node: {
     __dirname: false,
   },
-  externals: {
-    "dc-native": "commonjs2 dc-native",
-  },
   entry: {
     "app/main": "./src/app/main.ts",
   },