[PM-14360] Import Batching

jslib/angular/src/services/jslib-services.module.ts

@@ -5,7 +5,6 @@ import { AppIdService as AppIdServiceAbstraction } from "@/jslib/common/src/abst
 import { BroadcasterService as BroadcasterServiceAbstraction } from "@/jslib/common/src/abstractions/broadcaster.service";
 import { CryptoService as CryptoServiceAbstraction } from "@/jslib/common/src/abstractions/crypto.service";
 import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@/jslib/common/src/abstractions/cryptoFunction.service";
-import { DirectoryFactoryAbstraction } from "@/jslib/common/src/abstractions/directory-factory.service";
 import { EnvironmentService as EnvironmentServiceAbstraction } from "@/jslib/common/src/abstractions/environment.service";
 import { I18nService as I18nServiceAbstraction } from "@/jslib/common/src/abstractions/i18n.service";
 import { LogService } from "@/jslib/common/src/abstractions/log.service";
@@ -20,16 +19,18 @@ import { Account } from "@/jslib/common/src/models/domain/account";
 import { GlobalState } from "@/jslib/common/src/models/domain/globalState";
 import { ApiService } from "@/jslib/common/src/services/api.service";
 import { AppIdService } from "@/jslib/common/src/services/appId.service";
-import { BatchRequestBuilder } from "@/jslib/common/src/services/batch-requests.service";
 import { ConsoleLogService } from "@/jslib/common/src/services/consoleLog.service";
 import { CryptoService } from "@/jslib/common/src/services/crypto.service";
 import { EnvironmentService } from "@/jslib/common/src/services/environment.service";
-import { SingleRequestBuilder } from "@/jslib/common/src/services/single-request.service";
 import { StateService } from "@/jslib/common/src/services/state.service";
 import { StateMigrationService } from "@/jslib/common/src/services/stateMigration.service";
 import { TokenService } from "@/jslib/common/src/services/token.service";
 
-import { DirectoryFactoryService } from "@/src/services/directory-factory.service";
+import { DirectoryFactoryService } from "@/src/abstractions/directory-factory.service";
+import { StateService as StateServiceExtended } from "@/src/abstractions/state.service";
+import { DefaultBatchRequestBuilder } from "@/src/services/default-batch-request-builder";
+import { DefaultSingleRequestBuilder } from "@/src/services/default-single-request-builder";
+import { DefaultDirectoryFactoryService } from "@/src/services/directory-factory.service";
 
 import {
   SafeInjectionToken,
@@ -144,19 +145,17 @@ import { ValidationService } from "./validation.service";
       deps: [StorageServiceAbstraction, SECURE_STORAGE],
     }),
     safeProvider({
-      provide: BatchRequestBuilder,
-      useClass: BatchRequestBuilder,
-      useAngularDecorators: true,
+      provide: DefaultBatchRequestBuilder,
+      deps: []
     }),
     safeProvider({
-      provide: SingleRequestBuilder,
-      useClass: SingleRequestBuilder,
-      useAngularDecorators: true,
+      provide: DefaultSingleRequestBuilder,
+      deps: []
     }),
     safeProvider({
-      provide: DirectoryFactoryAbstraction,
-      useClass: DirectoryFactoryService,
-      useAngularDecorators: true,
+      provide: DirectoryFactoryService,
+      useClass: DefaultDirectoryFactoryService,
+      deps: [LogService, I18nServiceAbstraction, StateServiceExtended],
     }),
   ] satisfies SafeProvider[],
 })

src/abstractions/directory-factory.service.ts

@@ -0,0 +1,6 @@
+import { DirectoryType } from "@/src/enums/directoryType";
+import { IDirectoryService } from "@/src/services/directory.service";
+
+export abstract class DirectoryFactoryService {
+  abstract createService(type: DirectoryType): IDirectoryService;
+}

jslib/common/src/abstractions/request-builder.service.ts → src/abstractions/request-builder.service.ts

@@ -1,9 +1,9 @@
+import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";
+
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
-import { OrganizationImportRequest } from "../models/request/organizationImportRequest";
-
 export abstract class RequestBuilderAbstratction {
   buildRequest: (
     groups: GroupEntry[],
     users: UserEntry[],

src/app/services/services.module.ts

@@ -6,7 +6,6 @@
 import { BroadcasterService as BroadcasterServiceAbstraction } from "@/jslib/common/src/abstractions/broadcaster.service";
 import { CryptoService as CryptoServiceAbstraction } from "@/jslib/common/src/abstractions/crypto.service";
 import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@/jslib/common/src/abstractions/cryptoFunction.service";
-import { DirectoryFactoryAbstraction } from "@/jslib/common/src/abstractions/directory-factory.service";
 import { EnvironmentService as EnvironmentServiceAbstraction } from "@/jslib/common/src/abstractions/environment.service";
 import { I18nService as I18nServiceAbstraction } from "@/jslib/common/src/abstractions/i18n.service";
 import { LogService as LogServiceAbstraction } from "@/jslib/common/src/abstractions/log.service";
@@ -17,9 +16,7 @@
 import { TokenService as TokenServiceAbstraction } from "@/jslib/common/src/abstractions/token.service";
 import { StateFactory } from "@/jslib/common/src/factories/stateFactory";
 import { GlobalState } from "@/jslib/common/src/models/domain/globalState";
-import { BatchRequestBuilder as BatchRequestAbstraction } from "@/jslib/common/src/services/batch-requests.service";
 import { ContainerService } from "@/jslib/common/src/services/container.service";
-import { SingleRequestBuilder as SingleRequestAbstraction } from "@/jslib/common/src/services/single-request.service";
 import { ElectronLogService } from "@/jslib/electron/src/services/electronLog.service";
 import { ElectronPlatformUtilsService } from "@/jslib/electron/src/services/electronPlatformUtils.service";
 import { ElectronRendererMessagingService } from "@/jslib/electron/src/services/electronRendererMessaging.service";
@@ -28,6 +25,10 @@
 import { NodeApiService } from "@/jslib/node/src/services/nodeApi.service";
 import { NodeCryptoFunctionService } from "@/jslib/node/src/services/nodeCryptoFunction.service";
 
+import { DirectoryFactoryService } from "@/src/abstractions/directory-factory.service";
+import { DefaultBatchRequestBuilder } from "@/src/services/default-batch-request-builder";
+import { DefaultSingleRequestBuilder } from "@/src/services/default-single-request-builder";
+
 import { AuthService as AuthServiceAbstraction } from "../../abstractions/auth.service";
 import { StateService as StateServiceAbstraction } from "../../abstractions/state.service";
 import { Account } from "../../models/account";
@@ -171,16 +172,15 @@
       provide: SyncService,
       useClass: SyncService,
       deps: [
-        LogServiceAbstraction,
         CryptoFunctionServiceAbstraction,
         ApiServiceAbstraction,
         MessagingServiceAbstraction,
         I18nServiceAbstraction,
         EnvironmentServiceAbstraction,
         StateServiceAbstraction,
-        BatchRequestAbstraction,
-        SingleRequestAbstraction,
-        DirectoryFactoryAbstraction,
+        DefaultBatchRequestBuilder,
+        DefaultSingleRequestBuilder,
+        DirectoryFactoryService,
       ],
     }),
     safeProvider(AuthGuardService),

src/bwdc.ts

@@ -7,22 +7,23 @@
 import { StateFactory } from "@/jslib/common/src/factories/stateFactory";
 import { GlobalState } from "@/jslib/common/src/models/domain/globalState";
 import { AppIdService } from "@/jslib/common/src/services/appId.service";
-import { BatchRequestBuilder } from "@/jslib/common/src/services/batch-requests.service";
 import { ContainerService } from "@/jslib/common/src/services/container.service";
 import { CryptoService } from "@/jslib/common/src/services/crypto.service";
 import { EnvironmentService } from "@/jslib/common/src/services/environment.service";
 import { NoopMessagingService } from "@/jslib/common/src/services/noopMessaging.service";
-import { SingleRequestBuilder } from "@/jslib/common/src/services/single-request.service";
 import { TokenService } from "@/jslib/common/src/services/token.service";
 import { CliPlatformUtilsService } from "@/jslib/node/src/cli/services/cliPlatformUtils.service";
 import { ConsoleLogService } from "@/jslib/node/src/cli/services/consoleLog.service";
 import { NodeApiService } from "@/jslib/node/src/services/nodeApi.service";
 import { NodeCryptoFunctionService } from "@/jslib/node/src/services/nodeCryptoFunction.service";
 
+import { DirectoryFactoryService } from "./abstractions/directory-factory.service";
 import { Account } from "./models/account";
 import { Program } from "./program";
 import { AuthService } from "./services/auth.service";
-import { DirectoryFactoryService } from "./services/directory-factory.service";
+import { DefaultBatchRequestBuilder } from "./services/default-batch-request-builder";
+import { DefaultSingleRequestBuilder } from "./services/default-single-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";
@@ -55,8 +56,8 @@
   stateService: StateService;
   stateMigrationService: StateMigrationService;
   directoryFactoryService: DirectoryFactoryService;
-  batchRequestBuilder: BatchRequestBuilder;
-  singleRequestBuilder: SingleRequestBuilder;
+  batchRequestBuilder: DefaultBatchRequestBuilder;
+  singleRequestBuilder: DefaultSingleRequestBuilder;
 
   constructor() {
     const applicationName = "Bitwarden Directory Connector";
@@ -152,8 +153,16 @@
       this.stateService,
     );
 
-    this.syncService = new SyncService(
+    this.directoryFactoryService = new DefaultDirectoryFactoryService(
       this.logService,
+      this.i18nService,
+      this.stateService,
+    );
+
+    this.batchRequestBuilder = new DefaultBatchRequestBuilder();
+    this.singleRequestBuilder = new DefaultSingleRequestBuilder();
+
+    this.syncService = new SyncService(
       this.cryptoFunctionService,
       this.apiService,
       this.messagingService,

jslib/common/src/services/batch-requests.service.ts → src/services/default-batch-request-builder.ts

@@ -1,24 +1,29 @@
 import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";
 
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
 import { RequestBuilderAbstratction } from "../abstractions/request-builder.service";
 
-export class BatchRequestBuilder implements RequestBuilderAbstratction {
-  batchSize = 2000;
+import { batchSize } from "./sync.service";
 
+/**
+ * This class is responsible for batching large sync requests (>2k users) into multiple smaller
+ * requests to the /import REST endpoint. This is done to ensure we are under the default
+ * maximum packet size for NGINX web servers to avoid the request potentially timing out
+ * */
+export class DefaultBatchRequestBuilder implements RequestBuilderAbstratction {
   buildRequest(
     groups: GroupEntry[],
     users: UserEntry[],
     removeDisabled: boolean,
     overwriteExisting: boolean,
   ): OrganizationImportRequest[] {
     const requests: OrganizationImportRequest[] = [];
 
     if (users.length > 0) {
       const usersRequest = users.map((u) => {
         return {
           email: u.email,
           externalId: u.externalId,
           deleted: u.deleted || (removeDisabled && u.disabled),
@@ -26,21 +31,21 @@
       });
 
       // Partition users
-      for (let i = 0; i < usersRequest.length; i += this.batchSize) {
-        const u = usersRequest.slice(i, i + this.batchSize);
+      for (let i = 0; i < usersRequest.length; i += batchSize) {
+        const u = usersRequest.slice(i, i + batchSize);
         const req = new OrganizationImportRequest({
           groups: [],
           users: u,
           largeImport: true,
           overwriteExisting,
         });
         requests.push(req);
       }
     }
 
     if (groups.length > 0) {
       const groupRequest = groups.map((g) => {
         return {
           name: g.name,
           externalId: g.externalId,
           memberExternalIds: Array.from(g.userMemberExternalIds),
@@ -48,18 +53,18 @@
       });
 
       // Partition groups
-      for (let i = 0; i < groupRequest.length; i += this.batchSize) {
-        const g = groupRequest.slice(i, i + this.batchSize);
+      for (let i = 0; i < groupRequest.length; i += batchSize) {
+        const g = groupRequest.slice(i, i + batchSize);
         const req = new OrganizationImportRequest({
           groups: g,
           users: [],
           largeImport: true,
           overwriteExisting,
         });
         requests.push(req);
       }
     }
 
     return requests;
   }
 }

jslib/common/src/services/batch-requests.service.spec.ts → src/services/default-batch-requests-builder.spec.ts

@@ -1,32 +1,24 @@
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
-import { BatchRequestBuilder } from "./batch-requests.service";
-import { SingleRequestBuilder } from "./single-request.service";
+import { DefaultBatchRequestBuilder } from "./default-batch-request-builder";
+import { DefaultSingleRequestBuilder } from "./default-single-request-builder";
 
 describe("BatchingService", () => {
-  let batchRequestBuilder: BatchRequestBuilder;
-  let singleRequestBuilder: SingleRequestBuilder;
+  let batchRequestBuilder: DefaultBatchRequestBuilder;
+  let singleRequestBuilder: DefaultSingleRequestBuilder;
 
   function userSimulator(userCount: number) {
-    const simulatedArray: UserEntry[] = [];
-    for (let i = 0; i <= userCount; i++) {
-      simulatedArray.push(new UserEntry());
-    }
-    return simulatedArray;
+    return Array(userCount).fill(new UserEntry());
   }
 
   function groupSimulator(groupCount: number) {
-    const simulatedArray: GroupEntry[] = [];
-    for (let i = 0; i <= groupCount; i++) {
-      simulatedArray.push(new GroupEntry());
-    }
-    return simulatedArray;
+    return Array(groupCount).fill(new GroupEntry());
   }
 
   beforeEach(async () => {
-    batchRequestBuilder = new BatchRequestBuilder();
-    singleRequestBuilder = new SingleRequestBuilder();
+    batchRequestBuilder = new DefaultBatchRequestBuilder();
+    singleRequestBuilder = new DefaultSingleRequestBuilder();
   });
 
   it("BatchRequestBuilder batches requests for > 2000 users", () => {

jslib/common/src/services/single-request.service.ts → src/services/default-single-request-builder.ts

@@ -1,28 +1,34 @@
 import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";
 
 import { GroupEntry } from "@/src/models/groupEntry";
 import { UserEntry } from "@/src/models/userEntry";
 
 import { RequestBuilderAbstratction } from "../abstractions/request-builder.service";
 
-export class SingleRequestBuilder implements RequestBuilderAbstratction {
+/**
+ * This class is responsible for building small (<2k users) syncs as a single
+ * request to the /import endpoint. This is done to be backwards compatible with
+ * existing functionality for sync requests that are sufficiently small enough to not
+ * exceed default maximum packet size limits on NGINX web servers.
+ * */
+export class DefaultSingleRequestBuilder implements RequestBuilderAbstratction {
   buildRequest(
     groups: GroupEntry[],
     users: UserEntry[],
     removeDisabled: boolean,
     overwriteExisting: boolean,
   ): OrganizationImportRequest[] {
     return [
       new OrganizationImportRequest({
         groups: (groups ?? []).map((g) => {
           return {
             name: g.name,
             externalId: g.externalId,
             memberExternalIds: Array.from(g.userMemberExternalIds),
           };
         }),
         users: (users ?? []).map((u) => {
           return {
             email: u.email,
             externalId: u.externalId,
             deleted: u.deleted || (removeDisabled && u.disabled),

src/services/directory-factory.service.ts

@@ -1,36 +1,37 @@
-import { DirectoryFactoryAbstraction } from "@/jslib/common/src/abstractions/directory-factory.service";
 import { I18nService } from "@/jslib/common/src/abstractions/i18n.service";
 import { LogService } from "@/jslib/common/src/abstractions/log.service";
 
+import { DirectoryFactoryService } from "../abstractions/directory-factory.service";
 import { StateService } from "../abstractions/state.service";
 import { DirectoryType } from "../enums/directoryType";
 
 import { AzureDirectoryService } from "./azure-directory.service";
 import { GSuiteDirectoryService } from "./gsuite-directory.service";
 import { LdapDirectoryService } from "./ldap-directory.service";
 import { OktaDirectoryService } from "./okta-directory.service";
 import { OneLoginDirectoryService } from "./onelogin-directory.service";
 
-export class DirectoryFactoryService implements DirectoryFactoryAbstraction {
-  createService(
-    directoryType: DirectoryType,
-    logService: LogService,
-    i18nService: I18nService,
-    stateService: StateService,
-  ) {
+export class DefaultDirectoryFactoryService implements DirectoryFactoryService {
+  constructor(
+    private logService: LogService,
+    private i18nService: I18nService,
+    private stateService: StateService,
+  ) {}
+
+  createService(directoryType: DirectoryType) {
     switch (directoryType) {
       case DirectoryType.GSuite:
-        return new GSuiteDirectoryService(logService, i18nService, stateService);
+        return new GSuiteDirectoryService(this.logService, this.i18nService, this.stateService);
       case DirectoryType.AzureActiveDirectory:
-        return new AzureDirectoryService(logService, i18nService, stateService);
+        return new AzureDirectoryService(this.logService, this.i18nService, this.stateService);
       case DirectoryType.Ldap:
-        return new LdapDirectoryService(logService, i18nService, stateService);
+        return new LdapDirectoryService(this.logService, this.i18nService, this.stateService);
       case DirectoryType.Okta:
-        return new OktaDirectoryService(logService, i18nService, stateService);
+        return new OktaDirectoryService(this.logService, this.i18nService, this.stateService);
       case DirectoryType.OneLogin:
-        return new OneLoginDirectoryService(logService, i18nService, stateService);
+        return new OneLoginDirectoryService(this.logService, this.i18nService, this.stateService);
       default:
-        return null;
+        throw new Error("Invalid Directory Type");
     }
   }
 }