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',
|
serverUrl: 'ldap://example',
|
||||||
baseDn: 'dc=example,dc=com',
|
baseDn: 'dc=example,dc=com',
|
||||||
searchFilter: '(objectClass=person)',
|
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[],
|
userExcludeList: [] as string[],
|
||||||
fieldMappings: [
|
fieldMappings: [
|
||||||
{ ldapField: 'sAMAccountName', tesseraField: 'username' },
|
{ 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)', () => {
|
describe('LdapService — individual user search & import (dedup)', () => {
|
||||||
let service: LdapService;
|
let service: LdapService;
|
||||||
let prisma: any;
|
let prisma: any;
|
||||||
|
|||||||
@@ -534,6 +534,17 @@ export class LdapService {
|
|||||||
errors: [],
|
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(
|
const client = new Client(
|
||||||
this.buildClientOptions(config.serverUrl, config.tlsRejectUnauthorized),
|
this.buildClientOptions(config.serverUrl, config.tlsRejectUnauthorized),
|
||||||
);
|
);
|
||||||
@@ -673,8 +684,11 @@ export class LdapService {
|
|||||||
* Run the directory search for syncUsersForTenant, applying the selective
|
* Run the directory search for syncUsersForTenant, applying the selective
|
||||||
* group/OU import filter (groupFilterDns) when one is configured.
|
* group/OU import filter (groupFilterDns) when one is configured.
|
||||||
*
|
*
|
||||||
* - Empty groupFilterDns: single search of baseDn with sanitizedFilter
|
* - Empty groupFilterDns: DELIBERATE no-op — returns [] without ever
|
||||||
* (identical to pre-filter behavior, backward compatible).
|
* 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
|
* - Non-empty groupFilterDns: split into OU DNs (used as extra search
|
||||||
* bases) and group DNs (matched via memberOf on the base search).
|
* bases) and group DNs (matched via memberOf on the base search).
|
||||||
* Results are merged and deduped by entry dn.
|
* Results are merged and deduped by entry dn.
|
||||||
@@ -686,12 +700,7 @@ export class LdapService {
|
|||||||
attributes: string[],
|
attributes: string[],
|
||||||
) {
|
) {
|
||||||
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
|
if (!config.groupFilterDns || config.groupFilterDns.length === 0) {
|
||||||
const { searchEntries } = await client.search(config.baseDn, {
|
return [];
|
||||||
filter: sanitizedFilter,
|
|
||||||
attributes,
|
|
||||||
scope: 'sub',
|
|
||||||
});
|
|
||||||
return searchEntries;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const ouBases = config.groupFilterDns.filter((dn) => /^ou=/i.test(dn));
|
const ouBases = config.groupFilterDns.filter((dn) => /^ou=/i.test(dn));
|
||||||
|
|||||||
Reference in New Issue
Block a user