schalli/tessera-ctl
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 14 Wiki Activity

feat(260729-d3k): multi-base LDAP sync scope + re-keyed no-op guard

Browse Source 
- parseBaseDns() splits the newline-separated baseDn field into a list
- syncUsersForTenant no-op guard re-keyed on empty parsed base-DN list
  (was empty groupFilterDns) — the sole condition that skips search +
  the deactivation loop, preventing mass-deactivation on an
  unconfigured config
- collectSearchEntries/listGroups/searchUsers loop every base DN and
  merge/dedupe results by entry dn
- empty groupFilterDns is no longer a no-op: it now performs a normal
  multi-base search with no memberOf restriction
- groupFilterDns ou= entries stay additional search bases; group DNs
  become an optional memberOf constraint applied to every base search
- spec: replaced empty-groupFilterDns no-op test with empty-base-DN
  no-op test, added multi-base merge/dedup test

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Schalli
2026-07-29 09:36:55 +02:00
parent 5cdd48d864
commit 5cbd530a87
2 changed files with 199 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/api/src/ldap/ldap.service.spec.ts
+97 -8
View File
@@ -34,9 +34,10 @@ describe('LdapService.syncUsersForTenant — per-user exclude list', () => {
serverUrl: 'ldap://example',
baseDn: 'dc=example,dc=com',
searchFilter: '(objectClass=person)',
// Non-empty selection: an empty groupFilterDns is now a no-op (see the
// dedicated "empty selection no-op" describe block below), so these
// exclude-list tests need a real selection to exercise the search path.
// The Base-DN is the sync scope (an empty parsed base-DN list is the
// sole no-op path — see the dedicated "empty base DN no-op" describe
// block below). groupFilterDns here is an optional extra restriction;
// set to exercise the memberOf-restricted search path.
groupFilterDns: ['ou=people,dc=example,dc=com'] as string[],
userExcludeList: [] as string[],
fieldMappings: [
@@ -118,16 +119,16 @@ describe('LdapService.syncUsersForTenant — per-user exclude list', () => {
});
});
describe('LdapService.syncUsersForTenant — empty selection no-op', () => {
describe('LdapService.syncUsersForTenant — empty base DN no-op', () => {
let service: LdapService;
let prisma: any;
let userService: any;
const emptySelectionConfig = {
const emptyBaseDnConfig = {
id: 'cfg1',
tenantId: 't1',
serverUrl: 'ldap://example',
baseDn: 'dc=example,dc=com',
baseDn: '',
searchFilter: '(objectClass=person)',
groupFilterDns: [] as string[],
userExcludeList: [] as string[],
@@ -152,9 +153,9 @@ describe('LdapService.syncUsersForTenant — empty selection no-op', () => {
service = new LdapService(prisma, userService);
});
it('creates nobody and deactivates nobody when groupFilterDns is empty', async () => {
it('creates nobody and deactivates nobody when the base DN is empty (whitespace-only)', async () => {
const result = await service.syncUsersForTenant(
emptySelectionConfig as any,
{ ...emptyBaseDnConfig, baseDn: ' \n \n' } as any,
't1',
);
@@ -165,6 +166,94 @@ describe('LdapService.syncUsersForTenant — empty selection no-op', () => {
expect(prisma.user.update).not.toHaveBeenCalled();
expect(prisma.ldapConfig.update).not.toHaveBeenCalled();
});
it('is NOT a no-op when baseDn is set but groupFilterDns is empty (normal multi-base search)', async () => {
mockSearch.mockResolvedValue({
searchEntries: [{ dn: 'cn=alice', sAMAccountName: 'alice' }],
});
const result = await service.syncUsersForTenant(
{ ...emptyBaseDnConfig, baseDn: 'dc=example,dc=com' } as any,
't1',
);
expect(mockBind).toHaveBeenCalled();
expect(mockSearch).toHaveBeenCalled();
expect(result.created).toBe(1);
expect(userService.create).toHaveBeenCalledTimes(1);
});
});
describe('LdapService.syncUsersForTenant — multi base DN scope', () => {
let service: LdapService;
let prisma: any;
let userService: any;
const multiBaseConfig = {
id: 'cfg1',
tenantId: 't1',
serverUrl: 'ldap://example',
baseDn: 'dc=a,dc=com\ndc=b,dc=com',
searchFilter: '(objectClass=person)',
groupFilterDns: [] as string[],
userExcludeList: [] as string[],
fieldMappings: [
{ ldapField: 'sAMAccountName', tesseraField: 'username' },
],
};
beforeEach(() => {
vi.clearAllMocks();
mockBind.mockResolvedValue(undefined);
mockUnbind.mockResolvedValue(undefined);
prisma = {
user: {
findFirst: vi.fn().mockResolvedValue(null),
findMany: vi.fn().mockResolvedValue([]),
update: vi.fn().mockResolvedValue({}),
},
ldapConfig: { update: vi.fn().mockResolvedValue({}) },
};
userService = { create: vi.fn().mockResolvedValue({}) };
service = new LdapService(prisma, userService);
});
it('searches every configured base DN and merges/dedupes results by dn', async () => {
mockSearch
.mockResolvedValueOnce({
searchEntries: [
{ dn: 'cn=shared,dc=a,dc=com', sAMAccountName: 'shared' },
{ dn: 'cn=alice,dc=a,dc=com', sAMAccountName: 'alice' },
],
})
.mockResolvedValueOnce({
searchEntries: [
{ dn: 'cn=shared,dc=a,dc=com', sAMAccountName: 'shared' },
{ dn: 'cn=bob,dc=b,dc=com', sAMAccountName: 'bob' },
],
});
const result = await service.syncUsersForTenant(
multiBaseConfig as any,
't1',
);
expect(mockSearch).toHaveBeenCalledTimes(2);
expect(mockSearch).toHaveBeenNthCalledWith(
1,
'dc=a,dc=com',
expect.objectContaining({ filter: '(objectClass=person)' }),
);
expect(mockSearch).toHaveBeenNthCalledWith(
2,
'dc=b,dc=com',
expect.objectContaining({ filter: '(objectClass=person)' }),
);
// 3 distinct dns (shared, alice, bob) — the duplicate "shared" dn from
// the second base is deduped, not double-created.
expect(result.created).toBe(3);
expect(userService.create).toHaveBeenCalledTimes(3);
});
});
describe('LdapService — individual user search & import (dedup)', () => {
apps/api/src/ldap/ldap.service.ts
+93 -42
View File
@@ -188,9 +188,27 @@ export class LdapService {
}
/**
* Discover groups and organizational units under the configured base DN.
* Split a `\n`-separated Base-DN admin field into a trimmed, non-empty DN
* list. The baseDn column stays a single String (no schema change) — this
* is the sole place that turns it into the list every search path loops
* over. Blank/whitespace-only lines are dropped; undefined/empty input
* yields [].
*/
private parseBaseDns(baseDn?: string | null): string[] {
if (!baseDn) {
return [];
}
return baseDn
.split(/\r?\n/)
.map((line) => line.trim())
.filter((line) => line.length > 0);
}
/**
* Discover groups and organizational units under EVERY configured base DN.
* Used by the admin UI to build a selective import filter (groupFilterDns).
* Read-only directory query using the service-account bind.
* Read-only directory query using the service-account bind. Results from
* all base DNs are merged and deduped by entry dn.
*/
async listGroups(config: {
serverUrl: string;
@@ -206,13 +224,21 @@ export class LdapService {
try {
await this.bind(client, config.bindDn, config.bindPassword);
const { searchEntries } = await client.search(config.baseDn, {
const baseDns = this.parseBaseDns(config.baseDn);
const entriesByDn = new Map<string, Entry>();
for (const baseDn of baseDns) {
const { searchEntries } = await client.search(baseDn, {
filter: '(|(objectClass=group)(objectClass=organizationalUnit))',
attributes: ['cn', 'ou', 'dn'],
scope: 'sub',
});
for (const entry of searchEntries) {
entriesByDn.set(entry.dn, entry);
}
}
return searchEntries.map((entry) => {
return Array.from(entriesByDn.values()).map((entry) => {
const dn = entry.dn;
const isOu = /^ou=/i.test(dn);
const rawName = isOu ? entry['ou'] : entry['cn'];
@@ -350,14 +376,21 @@ export class LdapService {
const q = LdapService.escapeLdapFilterValue(trimmed);
const filter = `(&(objectClass=person)(|(cn=*${q}*)(sAMAccountName=*${q}*)(displayName=*${q}*)(mail=*${q}*)))`;
const { searchEntries } = await client.search(config.baseDn, {
const baseDns = this.parseBaseDns(config.baseDn);
const entriesByDn = new Map<string, Entry>();
for (const baseDn of baseDns) {
const { searchEntries } = await client.search(baseDn, {
filter,
attributes: ['cn', 'displayName', 'sAMAccountName', 'mail', 'dn'],
scope: 'sub',
sizeLimit: 50,
});
for (const entry of searchEntries) {
entriesByDn.set(entry.dn, entry);
}
}
const entries = searchEntries.map((entry) => ({
const entries = Array.from(entriesByDn.values()).map((entry) => ({
dn: entry.dn,
username: first(entry['sAMAccountName']),
displayName: first(entry['displayName']) || first(entry['cn']),
@@ -534,14 +567,17 @@ export class LdapService {
errors: [],
};
// DELIBERATE back-compat break + critical safety guard: an empty/undefined
// groupFilterDns now means "sync nothing", not "import everyone under
// baseDn". Returning here BEFORE any LDAP search and BEFORE the
// deactivation loop below is what prevents an empty selection from
// mass-deactivating every existing LDAP user (there would be no synced
// DNs to compare against, so every local LDAP user would look "removed").
// CRITICAL SAFETY GUARD: the Base-DN(s) are now the sync scope. Returning
// here BEFORE any LDAP bind/search and BEFORE the deactivation loop below
// is what prevents an unconfigured/blank config from mass-deactivating
// every existing LDAP user (there would be no synced DNs to compare
// against, so every local LDAP user would look "removed"). This is the
// SOLE no-op condition: an empty groupFilterDns no longer short-circuits
// here — with >=1 base DN configured, a normal multi-base search runs
// (see collectSearchEntries), it just applies no memberOf restriction.
// lastSyncAt is intentionally left untouched on this no-op path.
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
const baseDns = this.parseBaseDns(config.baseDn);
if (baseDns.length === 0) {
return result;
}
@@ -681,17 +717,22 @@ export class LdapService {
}
/**
* Run the directory search for syncUsersForTenant, applying the selective
* group/OU import filter (groupFilterDns) when one is configured.
* Run the directory search for syncUsersForTenant across EVERY configured
* Base DN (the primary sync scope — see syncUsersForTenant's early-return
* guard, which is the sole no-op path and is keyed on the parsed base-DN
* list, not on groupFilterDns). groupFilterDns is an OPTIONAL extra
* restriction:
*
* - Empty groupFilterDns: DELIBERATE no-op — returns [] without ever
* searching the directory. A selective sync now means "nothing selected
* = nothing synced", not "import everyone under baseDn" (back-compat
* break, see syncUsersForTenant's early-return guard which normally
* short-circuits before this method is even reached).
* - Non-empty groupFilterDns: split into OU DNs (used as extra search
* bases) and group DNs (matched via memberOf on the base search).
* Results are merged and deduped by entry dn.
* - Empty groupFilterDns: each base DN is searched with the plain
* sanitizedFilter — no memberOf restriction, every user under the base
* DN(s) is synced.
* - Non-empty groupFilterDns: split into OU DNs (searched as ADDITIONAL
* extra bases with the plain filter) and group DNs (turned into a
* memberOf OR-clause ANDed with sanitizedFilter and applied to every
* base DN search).
*
* All results across every base DN (and any extra OU bases) are merged
* and deduped by entry dn.
*/
private async collectSearchEntries(
client: Client,
@@ -699,18 +740,33 @@ export class LdapService {
sanitizedFilter: string,
attributes: string[],
) {
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
return [];
}
const ouBases = config.groupFilterDns.filter((dn) => /^ou=/i.test(dn));
const groupDns = config.groupFilterDns.filter((dn) => !/^ou=/i.test(dn));
const baseDns = this.parseBaseDns(config.baseDn);
const ouBases = (config.groupFilterDns ?? []).filter((dn) =>
/^ou=/i.test(dn),
);
const groupDns = (config.groupFilterDns ?? []).filter(
(dn) => !/^ou=/i.test(dn),
);
const entriesByDn = new Map<string, Entry>();
for (const ouBase of ouBases) {
const { searchEntries } = await client.search(ouBase, {
filter: sanitizedFilter,
// Build the base-search filter: plain sanitizedFilter, optionally ANDed
// with a memberOf OR-clause when group DNs are configured.
let baseFilter = sanitizedFilter;
if (groupDns.length > 0) {
const memberOfClauses = groupDns
.map(
(dn) => `(memberOf=${LdapService.escapeLdapFilterValue(dn)})`,
)
.join('');
baseFilter = `(&${sanitizedFilter}(|${memberOfClauses}))`;
}
// Search every configured base DN with the (possibly memberOf-restricted)
// base filter.
for (const baseDn of baseDns) {
const { searchEntries } = await client.search(baseDn, {
filter: baseFilter,
attributes,
scope: 'sub',
});
@@ -719,16 +775,11 @@ export class LdapService {
}
}
if (groupDns.length > 0) {
const memberOfClauses = groupDns
.map(
(dn) => `(memberOf=${LdapService.escapeLdapFilterValue(dn)})`,
)
.join('');
const combinedFilter = `(&${sanitizedFilter}(|${memberOfClauses}))`;
const { searchEntries } = await client.search(config.baseDn, {
filter: combinedFilter,
// ou= group-filter entries are ADDITIONAL search bases, searched with
// the plain sanitizedFilter (no memberOf restriction).
for (const ouBase of ouBases) {
const { searchEntries } = await client.search(ouBase, {
filter: sanitizedFilter,
attributes,
scope: 'sub',
});