diff --git a/apps/api/src/ldap/ldap.service.spec.ts b/apps/api/src/ldap/ldap.service.spec.ts index 253d9d3..98aad17 100644 --- a/apps/api/src/ldap/ldap.service.spec.ts +++ b/apps/api/src/ldap/ldap.service.spec.ts @@ -550,6 +550,46 @@ describe('LdapService.syncUsersForTenant — AD-bound group membership sync (D-1 fieldMappings: [{ ldapField: 'sAMAccountName', tesseraField: 'username' }], }; + // Plan 16-03 step 5a now runs BEFORE this block's own step-5b search in + // every real syncUsersForTenant() call. This describe block only + // exercises step 5b (membership sync), so 5a must resolve as a + // deterministic no-op by default: every fixture group's own DN-derived + // GUID resolves to an identity hit (same cn/dn as already stored) — no + // rename, no delete, nothing added to result.errors. Exposed at + // describe-scope (not just inside beforeEach) so a test that needs to + // ALSO fail one group's step-5b search (see the "records a search + // failure" test below) can still delegate the 5a shapes to this helper + // instead of re-deriving them. + const guidForDn = (dn: string): Buffer => { + const hex = Buffer.from(dn).toString('hex').padEnd(32, '0').slice(0, 32); + return Buffer.from(hex, 'hex'); + }; + + const resolve5aNoOp = (baseDn: string, filter: string) => { + if (filter === '(objectClass=group)') { + // 5a legacy-binding backfill probe: the probed base DN IS the + // group's own stored ldapDn (scope 'base'). + const g = groups.find((x) => x.ldapDn === baseDn); + return { + searchEntries: g + ? [{ dn: g.ldapDn, cn: g.name, objectGUID: guidForDn(g.ldapDn as string) }] + : [], + }; + } + if (filter.startsWith('(objectGUID=')) { + // 5a existence sweep: match whichever fixture group's DN-derived + // GUID produced this escaped filter. + const g = groups.find( + (x) => + x.ldapDn && + filter === + `(objectGUID=${LdapService.escapeLdapFilterBuffer(guidForDn(x.ldapDn))})`, + ); + return { searchEntries: g ? [{ dn: g.ldapDn, cn: g.name }] : [] }; + } + return null; + }; + beforeEach(() => { vi.clearAllMocks(); mockBind.mockResolvedValue(undefined); @@ -564,10 +604,15 @@ describe('LdapService.syncUsersForTenant — AD-bound group membership sync (D-1 ]; groupSearchEntries = []; - mockSearch.mockImplementation((_baseDn: string, opts: any) => { - if (typeof opts.filter === 'string' && opts.filter.includes('memberOf=')) { + mockSearch.mockImplementation((baseDn: string, opts: any) => { + const filter = typeof opts.filter === 'string' ? opts.filter : ''; + if (filter.includes('memberOf=')) { return Promise.resolve({ searchEntries: groupSearchEntries }); } + const fivea = resolve5aNoOp(baseDn, filter); + if (fivea) { + return Promise.resolve(fivea); + } // Plain user-sync search: no entries in this describe block. return Promise.resolve({ searchEntries: [] }); }); @@ -599,6 +644,16 @@ describe('LdapService.syncUsersForTenant — AD-bound group membership sync (D-1 ), ), ), + // 5a's legacy-binding backfill write (ldapObjectGuid only) — never + // a rename/delete write in this block since the mockSearch above + // always resolves an identity hit. + update: vi.fn((args: any) => { + const g = groups.find((x) => x.id === args.where.id); + if (g) { + Object.assign(g, args.data); + } + return Promise.resolve(g); + }), }, groupMembership: { createMany: vi.fn((args: any) => { @@ -848,15 +903,23 @@ describe('LdapService.syncUsersForTenant — AD-bound group membership sync (D-1 { id: 'm0', groupId: 'g-broken', userId: 'u-bob', source: 'MANUAL' }, ]; - mockSearch.mockImplementation((_baseDn: string, opts: any) => { - if (typeof opts.filter === 'string' && opts.filter.includes('CN=Broken')) { + mockSearch.mockImplementation((baseDn: string, opts: any) => { + const filter = typeof opts.filter === 'string' ? opts.filter : ''; + // Only step 5b's memberOf-filtered search for THIS group fails — 5a + // (reconciliation) resolves as a no-op for both groups via the + // shared helper, so this test isolates the 5b failure it targets. + if (filter.includes('memberOf=') && filter.includes('CN=Broken')) { return Promise.reject(new Error('directory unavailable')); } - if (typeof opts.filter === 'string' && opts.filter.includes('memberOf=')) { + if (filter.includes('memberOf=')) { return Promise.resolve({ searchEntries: [{ dn: 'cn=alice,dc=example,dc=com', sAMAccountName: 'alice' }], }); } + const fivea = resolve5aNoOp(baseDn, filter); + if (fivea) { + return Promise.resolve(fivea); + } return Promise.resolve({ searchEntries: [] }); }); @@ -879,6 +942,148 @@ describe('LdapService.syncUsersForTenant — AD-bound group membership sync (D-1 }); }); +describe('LdapService.syncUsersForTenant — Schrittreihenfolge Gruppen vor Mitgliedschaften', () => { + let service: LdapService; + let prisma: any; + let userService: any; + let groupsService: any; + + const cfg = { + 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' }], + }; + + const guidBuffer = Buffer.from('0123456789abcdef'.repeat(2), 'hex'); + const guidHex = guidBuffer.toString('hex'); + + beforeEach(() => { + vi.clearAllMocks(); + mockBind.mockResolvedValue(undefined); + mockUnbind.mockResolvedValue(undefined); + mockSearch.mockResolvedValue({ searchEntries: [] }); + prisma = { + user: { + findFirst: vi.fn().mockResolvedValue(null), + findMany: vi.fn().mockResolvedValue([]), + update: vi.fn().mockResolvedValue({}), + }, + group: { + findMany: vi.fn().mockResolvedValue([]), + update: vi.fn().mockResolvedValue({}), + }, + groupMembership: { + createMany: vi.fn().mockResolvedValue({ count: 0 }), + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + }, + ldapConfig: { update: vi.fn().mockResolvedValue({}) }, + }; + userService = { create: vi.fn().mockResolvedValue({}) }; + groupsService = { + reassignDefaultBeforeDelete: vi.fn().mockResolvedValue(false), + ensureDefaultGroup: vi.fn().mockResolvedValue(null), + }; + service = new LdapService(prisma, userService, groupsService); + }); + + it('calls syncBoundGroupsForTenant (5a) before syncGroupMembershipsForTenant (5b) — observed via call-order spies, not just asserted', async () => { + const callOrder: string[] = []; + const originalGroups = (service as any).syncBoundGroupsForTenant.bind(service); + const originalMemberships = (service as any).syncGroupMembershipsForTenant.bind(service); + vi.spyOn(service as any, 'syncBoundGroupsForTenant').mockImplementation( + async (...args: any[]) => { + callOrder.push('syncBoundGroupsForTenant'); + return originalGroups(...args); + }, + ); + vi.spyOn(service as any, 'syncGroupMembershipsForTenant').mockImplementation( + async (...args: any[]) => { + callOrder.push('syncGroupMembershipsForTenant'); + return originalMemberships(...args); + }, + ); + + await service.syncUsersForTenant(cfg as any, 't1'); + + expect(callOrder).toEqual([ + 'syncBoundGroupsForTenant', + 'syncGroupMembershipsForTenant', + ]); + }); + + it('sits behind the same Base-DN no-op guard as the rest of the sync — an empty base DN triggers neither step', async () => { + const callOrder: string[] = []; + vi.spyOn(service as any, 'syncBoundGroupsForTenant').mockImplementation( + async () => { + callOrder.push('syncBoundGroupsForTenant'); + }, + ); + vi.spyOn(service as any, 'syncGroupMembershipsForTenant').mockImplementation( + async () => { + callOrder.push('syncGroupMembershipsForTenant'); + }, + ); + + await service.syncUsersForTenant({ ...cfg, baseDn: ' \n ' } as any, 't1'); + + expect(callOrder).toEqual([]); + expect(mockSearch).not.toHaveBeenCalled(); + }); + + it('regression: a rename detected in 5a is written back before 5b builds its memberOf filter — no memberOf search ever uses the stale pre-rename DN', async () => { + const groups = [ + { + id: 'g1', + tenantId: 't1', + name: 'Sales', + ldapDn: 'cn=Sales,dc=example,dc=com', + ldapObjectGuid: guidHex, + isDefault: false, + }, + ]; + prisma.group.findMany = vi.fn(() => Promise.resolve(groups)); + prisma.group.update = vi.fn((args: any) => { + const g = groups.find((x) => x.id === args.where.id); + if (g) { + Object.assign(g, args.data); + } + return Promise.resolve(g); + }); + + mockSearch.mockImplementation((_baseDn: string, opts: any) => { + const filter = typeof opts.filter === 'string' ? opts.filter : ''; + if (filter.startsWith('(objectGUID=')) { + // 5a's existence sweep: AD reports the group renamed — new cn/dn. + return Promise.resolve({ + searchEntries: [ + { dn: 'cn=Vertrieb,dc=example,dc=com', cn: 'Vertrieb' }, + ], + }); + } + // 5b's memberOf search (and the plain user-sync search) — no hits, + // only the filter STRING passed matters for this test. + return Promise.resolve({ searchEntries: [] }); + }); + + await service.syncUsersForTenant(cfg as any, 't1'); + + // The rename was written back (5a ran and updated the in-memory row). + expect(groups[0].ldapDn).toBe('cn=Vertrieb,dc=example,dc=com'); + + const memberOfCall = mockSearch.mock.calls.find(([, opts]: any) => + (opts.filter as string).includes('memberOf='), + ); + expect(memberOfCall).toBeDefined(); + expect(memberOfCall![1].filter).toContain('cn=Vertrieb,dc=example,dc=com'); + expect(memberOfCall![1].filter).not.toContain('cn=Sales,dc=example,dc=com'); + }); +}); + describe('LdapService.syncBoundGroupsForTenant — Rekonziliation gegen das Verzeichnis (SC-3/SC-4/SC-5, D-05/D-06)', () => { let service: LdapService; let prisma: any; diff --git a/apps/api/src/ldap/ldap.service.ts b/apps/api/src/ldap/ldap.service.ts index ea14e80..3e08a0e 100644 --- a/apps/api/src/ldap/ldap.service.ts +++ b/apps/api/src/ldap/ldap.service.ts @@ -906,9 +906,24 @@ export class LdapService { } } + // 5a. Plan 16-03 (SC-3/SC-4/SC-5, D-05/D-06/D-07): reconcile every + // AD-bound Group's name/ldapDn/existence BEFORE the membership + // reconciliation below. This ordering is the central correctness + // condition of Phase 16, not a style choice (RESEARCH.md Pitfall 1): + // step 5b reads Group.ldapDn from the DB to build its memberOf + // filter. If a rename detected in THIS SAME run hadn't already been + // written back by the time 5b runs, the memberOf filter would still + // use the stale pre-rename DN, AD would return zero hits for it, and + // every LDAP membership of the renamed group would be misreported as + // removed — a rename would look like a membership wipeout that never + // happened in the directory. + await this.syncBoundGroupsForTenant(client, config, tenantId, result); + // 5b. D-21: reconcile GroupMembership rows for every AD-bound Group in // this same run — no separate sync job, no second button. Runs behind // the Base-DN no-op guard above, exactly like the rest of this method. + // Relies on 5a above having already written back any rename in this + // run (see 5a's comment). await this.syncGroupMembershipsForTenant( client, config,