feat(260728-lih): make LDAP sync strictly selective (empty selection = no-op)
- collectSearchEntries() returns [] on empty/undefined groupFilterDns instead of scanning the whole baseDn subtree - syncUsersForTenant() early-returns an empty successful result before any LDAP search or the deactivation loop when groupFilterDns is empty, so an empty selection can never mass-deactivate existing LDAP users - Updated exclude-list tests to use a non-empty groupFilterDns; added a dedicated no-op test proving empty selection performs zero search/ create/update/deactivate operations Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -34,7 +34,10 @@ describe('LdapService.syncUsersForTenant — per-user exclude list', () => {
|
||||
serverUrl: 'ldap://example',
|
||||
baseDn: 'dc=example,dc=com',
|
||||
searchFilter: '(objectClass=person)',
|
||||
groupFilterDns: [] as string[],
|
||||
// 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.
|
||||
groupFilterDns: ['ou=people,dc=example,dc=com'] as string[],
|
||||
userExcludeList: [] as string[],
|
||||
fieldMappings: [
|
||||
{ ldapField: 'sAMAccountName', tesseraField: 'username' },
|
||||
@@ -115,6 +118,55 @@ describe('LdapService.syncUsersForTenant — per-user exclude list', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('LdapService.syncUsersForTenant — empty selection no-op', () => {
|
||||
let service: LdapService;
|
||||
let prisma: any;
|
||||
let userService: any;
|
||||
|
||||
const emptySelectionConfig = {
|
||||
id: 'cfg1',
|
||||
tenantId: 't1',
|
||||
serverUrl: 'ldap://example',
|
||||
baseDn: 'dc=example,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([{ id: 'u-existing', ldapDn: 'cn=existing' }]),
|
||||
update: vi.fn().mockResolvedValue({}),
|
||||
},
|
||||
ldapConfig: { update: vi.fn().mockResolvedValue({}) },
|
||||
};
|
||||
userService = { create: vi.fn().mockResolvedValue({}) };
|
||||
service = new LdapService(prisma, userService);
|
||||
});
|
||||
|
||||
it('creates nobody and deactivates nobody when groupFilterDns is empty', async () => {
|
||||
const result = await service.syncUsersForTenant(
|
||||
emptySelectionConfig as any,
|
||||
't1',
|
||||
);
|
||||
|
||||
expect(result).toEqual({ created: 0, updated: 0, deactivated: 0, errors: [] });
|
||||
expect(mockSearch).not.toHaveBeenCalled();
|
||||
expect(mockBind).not.toHaveBeenCalled();
|
||||
expect(userService.create).not.toHaveBeenCalled();
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
expect(prisma.ldapConfig.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('LdapService — individual user search & import (dedup)', () => {
|
||||
let service: LdapService;
|
||||
let prisma: any;
|
||||
|
||||
@@ -534,6 +534,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").
|
||||
// lastSyncAt is intentionally left untouched on this no-op path.
|
||||
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
|
||||
return result;
|
||||
}
|
||||
|
||||
const client = new Client(
|
||||
this.buildClientOptions(config.serverUrl, config.tlsRejectUnauthorized),
|
||||
);
|
||||
@@ -673,8 +684,11 @@ export class LdapService {
|
||||
* Run the directory search for syncUsersForTenant, applying the selective
|
||||
* group/OU import filter (groupFilterDns) when one is configured.
|
||||
*
|
||||
* - Empty groupFilterDns: single search of baseDn with sanitizedFilter
|
||||
* (identical to pre-filter behavior, backward compatible).
|
||||
* - 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.
|
||||
@@ -686,12 +700,7 @@ export class LdapService {
|
||||
attributes: string[],
|
||||
) {
|
||||
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
|
||||
const { searchEntries } = await client.search(config.baseDn, {
|
||||
filter: sanitizedFilter,
|
||||
attributes,
|
||||
scope: 'sub',
|
||||
});
|
||||
return searchEntries;
|
||||
return [];
|
||||
}
|
||||
|
||||
const ouBases = config.groupFilterDns.filter((dn) => /^ou=/i.test(dn));
|
||||
|
||||
Reference in New Issue
Block a user