diff --git a/openldap/group-fixtures.ts b/openldap/group-fixtures.ts index 518ec6d5..a6298840 100644 --- a/openldap/group-fixtures.ts +++ b/openldap/group-fixtures.ts @@ -35,6 +35,29 @@ const data: Jsonify[] = [ externalId: "cn=Cleaners,ou=Janitorial,dc=bitwarden,dc=com", name: "Cleaners", }, + { + userMemberExternalIds: [ + "cn=Painterson Miki,ou=Product Development,dc=bitwarden,dc=com", + "cn=Virgina Pichocki,ou=Product Development,dc=bitwarden,dc=com", + "cn=Steffen Carsten,ou=Product Development,dc=bitwarden,dc=com", + ], + groupMemberReferenceIds: [], + users: [], + referenceId: "cn=DevOps Team,dc=bitwarden,dc=com", + externalId: "cn=DevOps Team,dc=bitwarden,dc=com", + name: "DevOps Team", + }, + { + userMemberExternalIds: [ + "cn=Angus Merizzi,ou=Management,dc=bitwarden,dc=com", + "cn=Grissel Currer,ou=Management,dc=bitwarden,dc=com", + ], + groupMemberReferenceIds: [], + users: [], + referenceId: "cn=Security Team,dc=bitwarden,dc=com", + externalId: "cn=Security Team,dc=bitwarden,dc=com", + name: "Security Team", + }, ]; export const groupFixtures = data.map((g) => GroupEntry.fromJSON(g)); diff --git a/openldap/ldifs/directory-20.ldif b/openldap/ldifs/directory-20.ldif index 62be217a..d0f93ec3 100644 --- a/openldap/ldifs/directory-20.ldif +++ b/openldap/ldifs/directory-20.ldif @@ -688,4 +688,27 @@ mobile: +1 804 319-5569 pager: +1 804 815-3661 roomNumber: 9273 manager: cn=Inga Schnirer,ou=Product Testing,dc=bitwarden, dc=com -secretary: cn=Keven Gilleland,ou=Administrative,dc=bitwarden, dc=com +secretary: cn=Keven Gilleland,ou=Administrative,dc=bitwarden, dc=com + +# DevOps Team and Security Team identify their members by the member uid attribute, +# instead of the member Dn attribute. +# These test that group membership by uid works correctly. + +dn: cn=DevOps Team,dc=bitwarden,dc=com +changetype: add +cn: DevOps Team +gidnumber: 800 +memberuid: mikip +memberuid: pichockv +memberuid: carstens +objectclass: posixGroup +objectclass: top + +dn: cn=Security Team,dc=bitwarden,dc=com +changetype: add +cn: Security Team +gidnumber: 900 +memberuid: merizzia +memberuid: currerg +objectclass: posixGroup +objectclass: top \ No newline at end of file diff --git a/src/services/ldap-directory.service.ts b/src/services/ldap-directory.service.ts index b511997e..b08f0a08 100644 --- a/src/services/ldap-directory.service.ts +++ b/src/services/ldap-directory.service.ts @@ -118,7 +118,7 @@ export class LdapDirectoryService implements IDirectoryService { [delControl], ); return regularUsers.concat(deletedUsers); - } catch (e) { + } catch { this.logService.warning("Cannot query deleted users."); return regularUsers; } @@ -192,14 +192,21 @@ export class LdapDirectoryService implements IDirectoryService { this.syncConfig.userFilter, ); const userPath = this.makeSearchPath(this.syncConfig.userPath); - const userIdMap = new Map(); + const userDnMap = new Map(); + const userUidMap = new Map(); await this.search(userPath, userFilter, (se: any) => { - userIdMap.set(this.getReferenceId(se), this.getExternalId(se, this.getReferenceId(se))); + const dn = this.getReferenceId(se); + const uid = this.getAttr(se, "uid"); + const externalId = this.getExternalId(se, dn); + userDnMap.set(dn, externalId); + if (uid != null) { + userUidMap.set(uid.toLowerCase(), externalId); + } return se; }); for (const se of groupSearchEntries) { - const group = this.buildGroup(se, userIdMap); + const group = this.buildGroup(se, userDnMap, userUidMap); if (group != null) { entries.push(group); } @@ -208,7 +215,20 @@ export class LdapDirectoryService implements IDirectoryService { return entries; } - private buildGroup(searchEntry: any, userMap: Map) { + /** + * Builds a GroupEntry from LDAP search results, including membership. + * Supports user membership by DN or UID and nested group membership by DN. + * + * @param searchEntry - The LDAP search entry containing group data + * @param userDnMap - Map of user DNs to their external IDs + * @param userUidMap - Map of user UIDs to their external IDs + * @returns A populated GroupEntry object, or null if the group lacks required properties + */ + private buildGroup( + searchEntry: any, + userDnMap: Map, + userUidMap: Map, + ) { const group = new GroupEntry(); group.referenceId = this.getReferenceId(searchEntry); if (group.referenceId == null) { @@ -228,11 +248,34 @@ export class LdapDirectoryService implements IDirectoryService { const members = this.getAttrVals(searchEntry, this.syncConfig.memberAttribute); if (members != null) { - for (const memDn of members) { - if (userMap.has(memDn) && !group.userMemberExternalIds.has(userMap.get(memDn))) { - group.userMemberExternalIds.add(userMap.get(memDn)); - } else if (!group.groupMemberReferenceIds.has(memDn)) { - group.groupMemberReferenceIds.add(memDn); + // Parses a group member attribute and identifies it as a member DN, member Uid, or a group Dn + const getMemberAttributeType = (member: string): "memberDn" | "memberUid" | "groupDn" => { + const isDnLike = member.includes("=") && member.includes(","); + if (isDnLike) { + return userDnMap.has(member) ? "memberDn" : "groupDn"; + } + return "memberUid"; + }; + + for (const member of members) { + switch (getMemberAttributeType(member)) { + case "memberDn": { + const externalId = userDnMap.get(member); + if (externalId != null) { + group.userMemberExternalIds.add(externalId); + } + break; + } + case "memberUid": { + const externalId = userUidMap.get(member.toLowerCase()); + if (externalId != null) { + group.userMemberExternalIds.add(externalId); + } + break; + } + case "groupDn": + group.groupMemberReferenceIds.add(member); + break; } } } diff --git a/src/services/sync.service.integration.spec.ts b/src/services/sync.service.integration.spec.ts index 39cd6ac2..2d56f82c 100644 --- a/src/services/sync.service.integration.spec.ts +++ b/src/services/sync.service.integration.spec.ts @@ -123,7 +123,10 @@ describe("SyncService", () => { expect(apiService.postPublicImportDirectory).toHaveBeenCalledWith( expect.objectContaining({ overwriteExisting: false }), ); - expect(apiService.postPublicImportDirectory).toHaveBeenCalledTimes(6); + + // The expected number of calls may change if more data is added to the ldif + // Make sure it equals (number of users / 4) + (number of groups / 4) + expect(apiService.postPublicImportDirectory).toHaveBeenCalledTimes(7); // @ts-expect-error Reset batch size to original state. constants.batchSize = originalBatchSize;