fix(quick-260909-ab3): kollidierende AD-Konten werden angelegt, nur ohne Adresse
- User.email auf optional gestellt (Migration geschrieben, NICHT ausgefuehrt); Eindeutigkeitsindex unangetastet, NULL bleibt in Postgres je verschieden - Neuer Kollisionsentscheider (resolveEmailForWrite) in ldap.service.ts: eine bereits vergebene Adresse wird nie umgehaengt (T-Q3-01) — das zuerst angelegte Konto behaelt sie, jedes weitere Konto entsteht ohne Adresse (gesperrte Nutzerentscheidung 2026-09-09, WINDOWS #15) - Entscheider in upsertMappedUser (Sync) UND importUsersByDn (Handimport) verdrahtet, damit der zweite Anlageweg nicht als Luecke bestehen bleibt - LdapSyncResult um emailConflicts/skippedNoLogin/entryFailures erweitert; rohe ORM-Ausnahmetexte gehen nur noch an logger.error, nie in den Bericht (T-Q3-02) - UserService.create nimmt die Adresse optional entgegen; Tender-Digest und Instant-Alert ueberspringen Empfaenger ohne Adresse (continue) - Fuenf neue Testfaelle vorab gegen den unveraenderten Bestand rot gelaufen (erwartete Ursachen bestaetigt); 651/651 API-Tests gruen, prisma validate und type-check sauber Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FYZcd3SSmo14QTqWx2KKzU
This commit is contained in:
@@ -0,0 +1,18 @@
|
||||
-- WINDOWS #15: der AD-Sync liess Konten, deren Adresse bereits einem anderen
|
||||
-- Konto gehoert (vier Funktionskonten mit geteilter mail-Adresse gemessen),
|
||||
-- wegen der Pflicht auf "User.email" komplett unimportiert liegen -- ohne
|
||||
-- verstaendliche Rueckmeldung.
|
||||
--
|
||||
-- Gesperrte Nutzerentscheidung vom 2026-09-09 (nicht verhandelbar): ein
|
||||
-- Konto mit bereits belegter Adresse wird trotzdem angelegt, nur eben OHNE
|
||||
-- Adresse. Die Anmeldung laeuft ueber den Benutzernamen, nicht ueber die
|
||||
-- Adresse -- das Konto bleibt voll funktionsfaehig, lediglich
|
||||
-- Benachrichtigungen per E-Mail erreichen es nicht. Wer eine Adresse zuerst
|
||||
-- belegt, behaelt sie unveraendert.
|
||||
--
|
||||
-- Der Eindeutigkeitsindex auf "email" wird NICHT angefasst: PostgreSQL
|
||||
-- behandelt NULL-Werte in einer Eindeutigkeitsregel als jeweils verschieden,
|
||||
-- mehrere Konten ohne Adresse sind also weiterhin erlaubt. Bestandszeilen
|
||||
-- bleiben unangetastet -- jedes bereits angelegte Konto behaelt seine
|
||||
-- heutige Adresse.
|
||||
ALTER TABLE "User" ALTER COLUMN "email" DROP NOT NULL;
|
||||
@@ -29,7 +29,7 @@ enum Role {
|
||||
model User {
|
||||
id String @id @default(uuid())
|
||||
username String @unique
|
||||
email String @unique
|
||||
email String? @unique
|
||||
passwordHash String?
|
||||
displayName String?
|
||||
role Role @default(USER)
|
||||
|
||||
@@ -218,6 +218,9 @@ describe('LdapService.syncUsersForTenant — empty base DN no-op', () => {
|
||||
groupsRenamed: 0,
|
||||
groupsDeleted: 0,
|
||||
defaultMarkerMoved: 0,
|
||||
emailConflicts: [],
|
||||
skippedNoLogin: [],
|
||||
entryFailures: [],
|
||||
errors: [],
|
||||
});
|
||||
expect(mockSearch).not.toHaveBeenCalled();
|
||||
@@ -348,6 +351,9 @@ describe('LdapService — individual user search & import (dedup)', () => {
|
||||
prisma = {
|
||||
user: {
|
||||
findFirst: vi.fn().mockResolvedValue(null),
|
||||
// Consulted by importUsersByDn's WINDOWS #15 email-collision decider
|
||||
// whenever an entry carries a mapped `mail` attribute.
|
||||
findUnique: vi.fn().mockResolvedValue(null),
|
||||
findMany: vi.fn().mockResolvedValue([]),
|
||||
update: vi.fn().mockResolvedValue({}),
|
||||
},
|
||||
@@ -473,6 +479,172 @@ describe('LdapService — individual user search & import (dedup)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('LdapService.syncUsersForTenant — E-Mail-Kollision bei der Kontoanlage (WINDOWS #15)', () => {
|
||||
let service: LdapService;
|
||||
let prisma: any;
|
||||
let userService: 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' },
|
||||
{ ldapField: 'mail', tesseraField: 'email' },
|
||||
],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockBind.mockResolvedValue(undefined);
|
||||
mockUnbind.mockResolvedValue(undefined);
|
||||
prisma = {
|
||||
user: {
|
||||
findFirst: vi.fn().mockResolvedValue(null),
|
||||
// The collision decider queries this — NOT mocked in the older
|
||||
// describe blocks above, added here because this is the first
|
||||
// block that exercises it (see PLAN.md Task 2 behavior note).
|
||||
findUnique: vi.fn().mockResolvedValue(null),
|
||||
findMany: vi.fn().mockResolvedValue([]),
|
||||
update: vi.fn().mockResolvedValue({}),
|
||||
},
|
||||
group: { findMany: 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({ id: 'created-user' }) };
|
||||
service = new LdapService(prisma, userService, {} as any);
|
||||
});
|
||||
|
||||
it('legt ein Konto mit belegter Adresse trotzdem an, nur ohne Adresse', async () => {
|
||||
mockSearch.mockResolvedValue({
|
||||
searchEntries: [
|
||||
{
|
||||
dn: 'cn=konto1,dc=example,dc=com',
|
||||
sAMAccountName: 'konto1',
|
||||
mail: 'geteilt@example.com',
|
||||
},
|
||||
{
|
||||
dn: 'cn=konto2,dc=example,dc=com',
|
||||
sAMAccountName: 'konto2',
|
||||
mail: 'geteilt@example.com',
|
||||
},
|
||||
],
|
||||
});
|
||||
// The first entry's address is free (findUnique -> null). By the time
|
||||
// the second entry is processed, the address already belongs to the
|
||||
// first-created account.
|
||||
prisma.user.findUnique
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce({ id: 'konto1-id', email: 'geteilt@example.com' });
|
||||
|
||||
const result = await service.syncUsersForTenant(cfg as any, 't1');
|
||||
|
||||
expect(userService.create).toHaveBeenCalledTimes(2);
|
||||
expect(userService.create).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ username: 'konto1', email: 'geteilt@example.com' }),
|
||||
);
|
||||
const secondCallArgs = userService.create.mock.calls[1][0];
|
||||
expect(secondCallArgs.email).toBeUndefined();
|
||||
expect(result.created).toBe(2);
|
||||
});
|
||||
|
||||
it('meldet die Kollision strukturiert und nicht als Fehler', async () => {
|
||||
mockSearch.mockResolvedValue({
|
||||
searchEntries: [
|
||||
{
|
||||
dn: 'cn=konto1,dc=example,dc=com',
|
||||
sAMAccountName: 'konto1',
|
||||
mail: 'geteilt@example.com',
|
||||
},
|
||||
{
|
||||
dn: 'cn=konto2,dc=example,dc=com',
|
||||
sAMAccountName: 'konto2',
|
||||
mail: 'geteilt@example.com',
|
||||
},
|
||||
],
|
||||
});
|
||||
prisma.user.findUnique
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce({ id: 'konto1-id', email: 'geteilt@example.com' });
|
||||
|
||||
const result = await service.syncUsersForTenant(cfg as any, 't1');
|
||||
|
||||
expect(result.emailConflicts).toEqual([
|
||||
{ account: 'konto2', email: 'geteilt@example.com' },
|
||||
]);
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('nimmt einem bestehenden Konto seine Adresse nicht weg', async () => {
|
||||
mockSearch.mockResolvedValue({
|
||||
searchEntries: [
|
||||
{
|
||||
dn: 'cn=konto3,dc=example,dc=com',
|
||||
sAMAccountName: 'konto3',
|
||||
mail: 'fremd@example.com',
|
||||
},
|
||||
],
|
||||
});
|
||||
// konto3 already exists locally (matched by ldapDn); the address AD now
|
||||
// reports for it belongs to a THIRD, unrelated user.
|
||||
prisma.user.findFirst.mockResolvedValue({ id: 'konto3-id', email: 'alt@example.com' });
|
||||
prisma.user.findUnique.mockResolvedValue({ id: 'dritter-user-id', email: 'fremd@example.com' });
|
||||
|
||||
const result = await service.syncUsersForTenant(cfg as any, 't1');
|
||||
|
||||
expect(prisma.user.update).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: { id: 'konto3-id' },
|
||||
data: expect.not.objectContaining({ email: expect.anything() }),
|
||||
}),
|
||||
);
|
||||
expect(result.emailConflicts).toEqual([
|
||||
{ account: 'konto3', email: 'fremd@example.com' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('meldet Eintraege ohne Anmeldenamen getrennt und nicht als Fehler', async () => {
|
||||
mockSearch.mockResolvedValue({
|
||||
searchEntries: [{ dn: 'cn=kontakt1,dc=example,dc=com' }],
|
||||
});
|
||||
|
||||
const result = await service.syncUsersForTenant(cfg as any, 't1');
|
||||
|
||||
expect(result.skippedNoLogin).toEqual(['cn=kontakt1,dc=example,dc=com']);
|
||||
expect(result.errors).toEqual([]);
|
||||
});
|
||||
|
||||
it('reicht keinen rohen Datenbanktext an den Bericht durch', async () => {
|
||||
mockSearch.mockResolvedValue({
|
||||
searchEntries: [
|
||||
{
|
||||
dn: 'cn=konto5,dc=example,dc=com',
|
||||
sAMAccountName: 'konto5',
|
||||
mail: 'konto5@example.com',
|
||||
},
|
||||
],
|
||||
});
|
||||
const ormMessage =
|
||||
'Invalid `prisma.user.create()` invocation: Unique constraint failed on the fields: (`email`)';
|
||||
userService.create.mockRejectedValueOnce(new Error(ormMessage));
|
||||
|
||||
const result = await service.syncUsersForTenant(cfg as any, 't1');
|
||||
|
||||
expect(result.entryFailures).toEqual(['cn=konto5,dc=example,dc=com']);
|
||||
expect(result.errors).not.toContain(ormMessage);
|
||||
expect(JSON.stringify(result)).not.toContain(ormMessage);
|
||||
});
|
||||
});
|
||||
|
||||
describe('LdapService.testConnection — TLS verification opt-out (ldaps)', () => {
|
||||
let service: LdapService;
|
||||
|
||||
|
||||
@@ -30,9 +30,31 @@ export interface LdapSyncResult {
|
||||
groupsRenamed: number;
|
||||
groupsDeleted: number;
|
||||
defaultMarkerMoved: number;
|
||||
// WINDOWS #15, gesperrte Nutzerentscheidung 2026-09-09: ein AD-Konto,
|
||||
// dessen mail-Adresse bereits einem ANDEREN Konto gehoert, wird trotzdem
|
||||
// angelegt/aktualisiert -- nur eben OHNE diese Adresse. Wer die Adresse
|
||||
// zuerst hatte, behaelt sie unveraendert (T-Q3-01). Diese Liste macht die
|
||||
// Kollision im Bericht sichtbar statt sie still zu verwerfen.
|
||||
emailConflicts: LdapEmailConflict[];
|
||||
// Verzeichniseintraege ohne sAMAccountName (Kontakte, Verteiler,
|
||||
// Ressourcen) -- ein normaler, erwarteter Vorgang, kein Fehler. Traegt die
|
||||
// Kennung (dn) des uebersprungenen Eintrags.
|
||||
skippedNoLogin: string[];
|
||||
// Ein unerwarteter Fehler bei genau diesem Eintrag. Nur die Kennung (dn)
|
||||
// steht hier -- der technische Wortlaut (z. B. eine rohe Prisma/ORM-
|
||||
// Ausnahme) geht AUSSCHLIESSLICH ueber this.logger.error ins
|
||||
// Serverprotokoll und erreicht diesen Bericht nie (T-Q3-02).
|
||||
entryFailures: string[];
|
||||
errors: string[];
|
||||
}
|
||||
|
||||
/** Ein Konto, das wegen einer bereits vergebenen Adresse ohne diese Adresse
|
||||
* angelegt oder aktualisiert wurde (WINDOWS #15). */
|
||||
export interface LdapEmailConflict {
|
||||
account: string;
|
||||
email: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* LDAP configuration shape as stored in the database.
|
||||
*/
|
||||
@@ -378,6 +400,32 @@ export class LdapService {
|
||||
return { username: mappedData['username']?.toLowerCase(), mappedData };
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether `desiredEmail` may be written to the account identified by
|
||||
* `ownRecordId` (null when the account does not exist yet, i.e. a create).
|
||||
* Queries the unique `email` column via findUnique (deliberately NOT
|
||||
* findFirst — the column is unique, and this keeps the call unambiguously
|
||||
* distinguishable in tests from the identity findFirst lookups in
|
||||
* upsertMappedUser/importUsersByDn) and returns the address only when
|
||||
* nobody holds it yet, or the holder IS this same account. Otherwise the
|
||||
* address is withheld and reported as a collision instead — an address is
|
||||
* NEVER handed from one account to another (T-Q3-01): a directory entry
|
||||
* could otherwise take over a real person's address and receive their
|
||||
* password-reset mail.
|
||||
*/
|
||||
private async resolveEmailForWrite(
|
||||
desiredEmail: string,
|
||||
ownRecordId: string | null,
|
||||
): Promise<{ email?: string; collides: boolean }> {
|
||||
const holder = await this.prisma.user.findUnique({
|
||||
where: { email: desiredEmail },
|
||||
});
|
||||
if (!holder || holder.id === ownRecordId) {
|
||||
return { email: desiredEmail, collides: false };
|
||||
}
|
||||
return { collides: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Find-or-upsert one LDAP user by (ldapDn, then username) within a tenant.
|
||||
*
|
||||
@@ -385,13 +433,22 @@ export class LdapService {
|
||||
* identity resolution: a user imported one way is NEVER duplicated by the
|
||||
* other. A manually-imported user (ldapDn set) is matched by ldapDn on a
|
||||
* later department/group sync and updated in place, not re-created.
|
||||
*
|
||||
* WINDOWS #15: an account whose mapped `mail` address is already held by a
|
||||
* DIFFERENT account is still created/updated, just without that address
|
||||
* (locked user decision, 2026-09-09) — see resolveEmailForWrite() above.
|
||||
* The `${username}@ldap.local` fallback for entries with NO mail attribute
|
||||
* at all is untouched: it is not part of this defect.
|
||||
*/
|
||||
private async upsertMappedUser(
|
||||
dn: string,
|
||||
username: string,
|
||||
mappedData: Record<string, string>,
|
||||
tenantId: string,
|
||||
): Promise<'created' | 'updated'> {
|
||||
): Promise<{
|
||||
status: 'created' | 'updated';
|
||||
emailConflict?: LdapEmailConflict;
|
||||
}> {
|
||||
const existingByDn = await this.prisma.user.findFirst({
|
||||
where: { ldapDn: dn, tenantId },
|
||||
});
|
||||
@@ -401,30 +458,60 @@ export class LdapService {
|
||||
const existing = existingByDn || existingByUsername;
|
||||
|
||||
if (existing) {
|
||||
let emailToWrite: string | undefined;
|
||||
let emailConflict: LdapEmailConflict | undefined;
|
||||
if (mappedData['email']) {
|
||||
const decision = await this.resolveEmailForWrite(
|
||||
mappedData['email'],
|
||||
existing.id,
|
||||
);
|
||||
if (decision.collides) {
|
||||
emailConflict = { account: username, email: mappedData['email'] };
|
||||
} else {
|
||||
emailToWrite = decision.email;
|
||||
}
|
||||
}
|
||||
|
||||
await this.prisma.user.update({
|
||||
where: { id: existing.id },
|
||||
data: {
|
||||
...(mappedData['displayName'] && {
|
||||
displayName: mappedData['displayName'],
|
||||
}),
|
||||
...(mappedData['email'] && { email: mappedData['email'] }),
|
||||
...(emailToWrite && { email: emailToWrite }),
|
||||
...(mappedData['username'] && { username }),
|
||||
ldapDn: dn,
|
||||
isActive: true,
|
||||
},
|
||||
});
|
||||
return 'updated';
|
||||
return { status: 'updated', emailConflict };
|
||||
}
|
||||
|
||||
let createEmail: string | undefined =
|
||||
mappedData['email'] || `${username}@ldap.local`;
|
||||
let emailConflict: LdapEmailConflict | undefined;
|
||||
if (mappedData['email']) {
|
||||
const decision = await this.resolveEmailForWrite(
|
||||
mappedData['email'],
|
||||
null,
|
||||
);
|
||||
if (decision.collides) {
|
||||
createEmail = undefined;
|
||||
emailConflict = { account: username, email: mappedData['email'] };
|
||||
} else {
|
||||
createEmail = decision.email;
|
||||
}
|
||||
}
|
||||
|
||||
await this.userService.create({
|
||||
username,
|
||||
email: mappedData['email'] || `${username}@ldap.local`,
|
||||
...(createEmail && { email: createEmail }),
|
||||
displayName: mappedData['displayName'],
|
||||
role: 'USER',
|
||||
tenantId,
|
||||
ldapDn: dn,
|
||||
});
|
||||
return 'created';
|
||||
return { status: 'created', emailConflict };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -599,9 +686,24 @@ export class LdapService {
|
||||
continue;
|
||||
}
|
||||
|
||||
// WINDOWS #15: same collision decider as upsertMappedUser — an
|
||||
// address already held by a DIFFERENT account is withheld, never
|
||||
// reassigned (T-Q3-01). No new LdapUserImportResult field: the
|
||||
// account is still created and counted, this manual-import path's
|
||||
// display stays as-is.
|
||||
let createEmail: string | undefined =
|
||||
mappedData['email'] || `${username}@ldap.local`;
|
||||
if (mappedData['email']) {
|
||||
const decision = await this.resolveEmailForWrite(
|
||||
mappedData['email'],
|
||||
null,
|
||||
);
|
||||
createEmail = decision.collides ? undefined : decision.email;
|
||||
}
|
||||
|
||||
await this.userService.create({
|
||||
username,
|
||||
email: mappedData['email'] || `${username}@ldap.local`,
|
||||
...(createEmail && { email: createEmail }),
|
||||
displayName: mappedData['displayName'],
|
||||
role: 'USER',
|
||||
tenantId,
|
||||
@@ -775,6 +877,9 @@ export class LdapService {
|
||||
groupsRenamed: 0,
|
||||
groupsDeleted: 0,
|
||||
defaultMarkerMoved: 0,
|
||||
emailConflicts: [],
|
||||
skippedNoLogin: [],
|
||||
entryFailures: [],
|
||||
errors: [],
|
||||
};
|
||||
|
||||
@@ -858,15 +963,15 @@ export class LdapService {
|
||||
syncedDns.push(dn);
|
||||
|
||||
if (!username) {
|
||||
result.errors.push(
|
||||
`Entry ${dn}: no username mapped (check sAMAccountName mapping)`,
|
||||
);
|
||||
// Normal for contacts/resources/distribution entries without a
|
||||
// sAMAccountName — a neutral hint, not an error (WINDOWS #15).
|
||||
result.skippedNoLogin.push(dn);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Find-or-upsert by (ldapDn, then username) via the shared helper so
|
||||
// sync and manual import dedupe identically (never a duplicate row).
|
||||
const status = await this.upsertMappedUser(
|
||||
const { status, emailConflict } = await this.upsertMappedUser(
|
||||
dn,
|
||||
username,
|
||||
mappedData,
|
||||
@@ -877,12 +982,20 @@ export class LdapService {
|
||||
} else {
|
||||
result.updated++;
|
||||
}
|
||||
if (emailConflict) {
|
||||
result.emailConflicts.push(emailConflict);
|
||||
}
|
||||
} catch (entryError: unknown) {
|
||||
const msg =
|
||||
entryError instanceof Error
|
||||
? entryError.message
|
||||
: 'Unknown error processing entry';
|
||||
result.errors.push(`Entry ${entry.dn}: ${msg}`);
|
||||
// T-Q3-02: the raw exception text (ORM table/column/call names)
|
||||
// goes to the server log only — never to the report an
|
||||
// administrator reads. Only the entry's identifier (dn) is
|
||||
// reported.
|
||||
this.logger.error(`LDAP sync entry ${entry.dn} failed: ${msg}`);
|
||||
result.entryFailures.push(entry.dn);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -134,7 +134,10 @@ export class TenderDigestScheduler implements OnModuleInit {
|
||||
if (!matches.length) continue;
|
||||
|
||||
const user = await this.prisma.user.findUnique({ where: { id: userId } });
|
||||
if (!user) continue;
|
||||
// Kein Konto, oder ein Konto ohne Adresse (WINDOWS #15, kollidierte
|
||||
// AD-Adresse) -- die Zugehoerigkeit funktioniert, nur der
|
||||
// Mailversand wird uebersprungen (zugesagtes Verhalten).
|
||||
if (!user || !user.email) continue;
|
||||
|
||||
const sections = groupMatchesByProfile(matches);
|
||||
const sent = await this.mail.sendDigest({ email: user.email }, user.tenantId, sections);
|
||||
|
||||
@@ -125,7 +125,10 @@ export class TenderMatchingService {
|
||||
if (!fresh.length) continue; // nothing new for this profile this tick
|
||||
|
||||
const user = await this.prisma.user.findUnique({ where: { id: profile.userId } });
|
||||
if (!user) continue;
|
||||
// Kein Konto, oder ein Konto ohne Adresse (WINDOWS #15, kollidierte
|
||||
// AD-Adresse) -- die Zugehoerigkeit funktioniert, nur der
|
||||
// Mailversand wird uebersprungen (zugesagtes Verhalten).
|
||||
if (!user || !user.email) continue;
|
||||
|
||||
const sent = await this.mail.sendInstant(
|
||||
{ email: user.email },
|
||||
|
||||
@@ -48,7 +48,7 @@ export class UserService {
|
||||
*/
|
||||
async create(data: {
|
||||
username: string;
|
||||
email: string;
|
||||
email?: string;
|
||||
password?: string;
|
||||
displayName?: string;
|
||||
role?: 'SUPER_ADMIN' | 'ADMIN' | 'USER';
|
||||
|
||||
Reference in New Issue
Block a user