Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-14360] Import Batching #703

Merged
merged 25 commits into from
Mar 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions jslib/common/src/abstractions/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { ApiTokenRequest } from "../models/request/identityToken/apiTokenRequest
import { PasswordTokenRequest } from "../models/request/identityToken/passwordTokenRequest";
import { SsoTokenRequest } from "../models/request/identityToken/ssoTokenRequest";
import { OrganizationImportRequest } from "../models/request/organizationImportRequest";
import { IdentityCaptchaResponse } from '../models/response/identityCaptchaResponse';
import { IdentityTokenResponse } from '../models/response/identityTokenResponse';
import { IdentityTwoFactorResponse } from '../models/response/identityTwoFactorResponse';
import { IdentityCaptchaResponse } from "../models/response/identityCaptchaResponse";
import { IdentityTokenResponse } from "../models/response/identityTokenResponse";
import { IdentityTwoFactorResponse } from "../models/response/identityTwoFactorResponse";

export abstract class ApiService {
postIdentityToken: (
Expand Down
6 changes: 6 additions & 0 deletions src/abstractions/directory-factory.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { DirectoryType } from "@/src/enums/directoryType";
import { IDirectoryService } from "@/src/services/directory.service";

export abstract class DirectoryFactoryService {

Check warning on line 4 in src/abstractions/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/abstractions/directory-factory.service.ts#L4

Added line #L4 was not covered by tests
abstract createService(type: DirectoryType): IDirectoryService;
}
13 changes: 13 additions & 0 deletions src/abstractions/request-builder.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";

import { GroupEntry } from "@/src/models/groupEntry";
import { UserEntry } from "@/src/models/userEntry";

export abstract class RequestBuilder {

Check warning on line 6 in src/abstractions/request-builder.service.ts

View check run for this annotation

Codecov / codecov/patch

src/abstractions/request-builder.service.ts#L6

Added line #L6 was not covered by tests
buildRequest: (
groups: GroupEntry[],
users: UserEntry[],
removeDisabled: boolean,
overwriteExisting: boolean,
) => OrganizationImportRequest[];
}
22 changes: 21 additions & 1 deletion src/app/services/services.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
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 { BatchRequestBuilder } from "@/src/services/batch-request-builder";
import { DefaultDirectoryFactoryService } from "@/src/services/directory-factory.service";
import { SingleRequestBuilder } from "@/src/services/single-request-builder";

Check warning on line 31 in src/app/services/services.module.ts

View check run for this annotation

Codecov / codecov/patch

src/app/services/services.module.ts#L28-L31

Added lines #L28 - L31 were not covered by tests

import { AuthService as AuthServiceAbstraction } from "../../abstractions/auth.service";
import { StateService as StateServiceAbstraction } from "../../abstractions/state.service";
import { Account } from "../../models/account";
Expand Down Expand Up @@ -168,13 +173,15 @@
provide: SyncService,
useClass: SyncService,
deps: [
LogServiceAbstraction,
CryptoFunctionServiceAbstraction,
ApiServiceAbstraction,
MessagingServiceAbstraction,
I18nServiceAbstraction,
EnvironmentServiceAbstraction,
StateServiceAbstraction,
BatchRequestBuilder,
SingleRequestBuilder,
DirectoryFactoryService,
],
}),
safeProvider(AuthGuardService),
Expand Down Expand Up @@ -215,6 +222,19 @@
StateMigrationServiceAbstraction,
],
}),
safeProvider({
provide: SingleRequestBuilder,
deps: [],
}),
safeProvider({
provide: BatchRequestBuilder,
deps: [],
}),
safeProvider({
provide: DirectoryFactoryService,
useClass: DefaultDirectoryFactoryService,
deps: [LogServiceAbstraction, I18nServiceAbstraction, StateServiceAbstraction],
}),
] satisfies SafeProvider[],
})
export class ServicesModule {}
20 changes: 19 additions & 1 deletion src/bwdc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
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 { BatchRequestBuilder } from "./services/batch-request-builder";
import { DefaultDirectoryFactoryService } from "./services/directory-factory.service";

Check warning on line 25 in src/bwdc.ts

View check run for this annotation

Codecov / codecov/patch

src/bwdc.ts#L24-L25

Added lines #L24 - L25 were not covered by tests
import { I18nService } from "./services/i18n.service";
import { KeytarSecureStorageService } from "./services/keytarSecureStorage.service";
import { LowdbStorageService } from "./services/lowdbStorage.service";
import { SingleRequestBuilder } from "./services/single-request-builder";

Check warning on line 29 in src/bwdc.ts

View check run for this annotation

Codecov / codecov/patch

src/bwdc.ts#L29

Added line #L29 was not covered by tests
import { StateService } from "./services/state.service";
import { StateMigrationService } from "./services/stateMigration.service";
import { SyncService } from "./services/sync.service";
Expand Down Expand Up @@ -51,6 +55,9 @@
syncService: SyncService;
stateService: StateService;
stateMigrationService: StateMigrationService;
directoryFactoryService: DirectoryFactoryService;
batchRequestBuilder: BatchRequestBuilder;
singleRequestBuilder: SingleRequestBuilder;

constructor() {
const applicationName = "Bitwarden Directory Connector";
Expand Down Expand Up @@ -146,14 +153,25 @@
this.stateService,
);

this.syncService = new SyncService(
this.directoryFactoryService = new DefaultDirectoryFactoryService(

Check warning on line 156 in src/bwdc.ts

View check run for this annotation

Codecov / codecov/patch

src/bwdc.ts#L156

Added line #L156 was not covered by tests
this.logService,
this.i18nService,
this.stateService,
);

this.batchRequestBuilder = new BatchRequestBuilder();
this.singleRequestBuilder = new SingleRequestBuilder();

Check warning on line 163 in src/bwdc.ts

View check run for this annotation

Codecov / codecov/patch

src/bwdc.ts#L162-L163

Added lines #L162 - L163 were not covered by tests

this.syncService = new SyncService(

Check warning on line 165 in src/bwdc.ts

View check run for this annotation

Codecov / codecov/patch

src/bwdc.ts#L165

Added line #L165 was not covered by tests
this.cryptoFunctionService,
this.apiService,
this.messagingService,
this.i18nService,
this.environmentService,
this.stateService,
this.batchRequestBuilder,
this.singleRequestBuilder,
this.directoryFactoryService,
);

this.program = new Program(this);
Expand Down
70 changes: 70 additions & 0 deletions src/services/batch-request-builder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { OrganizationImportRequest } from "@/jslib/common/src/models/request/organizationImportRequest";

import { GroupEntry } from "@/src/models/groupEntry";
import { UserEntry } from "@/src/models/userEntry";

import { RequestBuilder } from "../abstractions/request-builder.service";

import { batchSize } from "./sync.service";

/**
* This class is responsible for batching large sync requests (>2k users) into multiple smaller
* requests to the /import 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 BatchRequestBuilder implements RequestBuilder {
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),
};
});

// Partition users
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),
};
});

// Partition groups
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;
}
}
47 changes: 47 additions & 0 deletions src/services/batch-requests-builder.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { GroupEntry } from "@/src/models/groupEntry";
import { UserEntry } from "@/src/models/userEntry";

import { BatchRequestBuilder } from "./batch-request-builder";
import { SingleRequestBuilder } from "./single-request-builder";

describe("BatchRequestBuilder", () => {
let batchRequestBuilder: BatchRequestBuilder;
let singleRequestBuilder: SingleRequestBuilder;

function userSimulator(userCount: number) {
return Array(userCount).fill(new UserEntry());
}

function groupSimulator(groupCount: number) {
return Array(groupCount).fill(new GroupEntry());
}

beforeEach(async () => {
batchRequestBuilder = new BatchRequestBuilder();
singleRequestBuilder = new SingleRequestBuilder();
});

it("BatchRequestBuilder batches requests for > 2000 users", () => {
const mockGroups = groupSimulator(11000);
const mockUsers = userSimulator(11000);

const requests = batchRequestBuilder.buildRequest(mockGroups, mockUsers, true, true);

expect(requests.length).toEqual(12);
});

it("SingleRequestBuilder returns single request for 200 users", () => {
const mockGroups = groupSimulator(200);
const mockUsers = userSimulator(200);

const requests = singleRequestBuilder.buildRequest(mockGroups, mockUsers, true, true);

expect(requests.length).toEqual(1);
});

it("BatchRequestBuilder retuns an empty array when there are no users or groups", () => {
const requests = batchRequestBuilder.buildRequest([], [], true, true);

expect(requests).toEqual([]);
});
});
37 changes: 37 additions & 0 deletions src/services/directory-factory.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
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";

Check warning on line 6 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L6

Added line #L6 was not covered by tests

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";

Check warning on line 12 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L8-L12

Added lines #L8 - L12 were not covered by tests

export class DefaultDirectoryFactoryService implements DirectoryFactoryService {

Check warning on line 14 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L14

Added line #L14 was not covered by tests
constructor(
private logService: LogService,
private i18nService: I18nService,
private stateService: StateService,

Check warning on line 18 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L16-L18

Added lines #L16 - L18 were not covered by tests
) {}

createService(directoryType: DirectoryType) {
switch (directoryType) {
case DirectoryType.GSuite:
return new GSuiteDirectoryService(this.logService, this.i18nService, this.stateService);

Check warning on line 24 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L24

Added line #L24 was not covered by tests
case DirectoryType.AzureActiveDirectory:
return new AzureDirectoryService(this.logService, this.i18nService, this.stateService);

Check warning on line 26 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L26

Added line #L26 was not covered by tests
case DirectoryType.Ldap:
return new LdapDirectoryService(this.logService, this.i18nService, this.stateService);

Check warning on line 28 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L28

Added line #L28 was not covered by tests
case DirectoryType.Okta:
return new OktaDirectoryService(this.logService, this.i18nService, this.stateService);

Check warning on line 30 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L30

Added line #L30 was not covered by tests
case DirectoryType.OneLogin:
return new OneLoginDirectoryService(this.logService, this.i18nService, this.stateService);

Check warning on line 32 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L32

Added line #L32 was not covered by tests
default:
throw new Error("Invalid Directory Type");

Check warning on line 34 in src/services/directory-factory.service.ts

View check run for this annotation

Codecov / codecov/patch

src/services/directory-factory.service.ts#L34

Added line #L34 was not covered by tests
}
}
}
54 changes: 1 addition & 53 deletions src/services/ldap-directory.service.integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import { groupFixtures } from "../../openldap/group-fixtures";
import { userFixtures } from "../../openldap/user-fixtures";
import { DirectoryType } from "../enums/directoryType";
import { LdapConfiguration } from "../models/ldapConfiguration";
import { SyncConfiguration } from "../models/syncConfiguration";
import { getLdapConfiguration, getSyncConfiguration } from "../utils/test-fixtures";

Check warning on line 8 in src/services/ldap-directory.service.integration.spec.ts

View check run for this annotation

Codecov / codecov/patch

src/services/ldap-directory.service.integration.spec.ts#L8

Added line #L8 was not covered by tests

import { LdapDirectoryService } from "./ldap-directory.service";
import { StateService } from "./state.service";
Expand Down Expand Up @@ -154,54 +153,3 @@
});
});
});

/**
* @returns a basic ldap configuration without TLS/SSL enabled. Can be overridden by passing in a partial configuration.
*/
const getLdapConfiguration = (config?: Partial<LdapConfiguration>): LdapConfiguration => ({
ssl: false,
startTls: false,
tlsCaPath: null,
sslAllowUnauthorized: false,
sslCertPath: null,
sslKeyPath: null,
sslCaPath: null,
hostname: "localhost",
port: 1389,
domain: null,
rootPath: "dc=bitwarden,dc=com",
currentUser: false,
username: "cn=admin,dc=bitwarden,dc=com",
password: "admin",
ad: false,
pagedSearch: false,
...(config ?? {}),
});

/**
* @returns a basic sync configuration. Can be overridden by passing in a partial configuration.
*/
const getSyncConfiguration = (config?: Partial<SyncConfiguration>): SyncConfiguration => ({
users: false,
groups: false,
interval: 5,
userFilter: null,
groupFilter: null,
removeDisabled: false,
overwriteExisting: false,
largeImport: false,
// Ldap properties
groupObjectClass: "posixGroup",
userObjectClass: "person",
groupPath: null,
userPath: null,
groupNameAttribute: "cn",
userEmailAttribute: "mail",
memberAttribute: "memberUid",
useEmailPrefixSuffix: false,
emailPrefixAttribute: "sAMAccountName",
emailSuffix: null,
creationDateAttribute: "whenCreated",
revisionDateAttribute: "whenChanged",
...(config ?? {}),
});
Loading