feat(16-02): server-side name lock for imported groups + internalName (D-03/D-04/D-07)
- GroupsService.update() rejects `name` with BadRequestException when the loaded group carries a set ldapObjectGuid (imported groups) — a real backend invariant, not a UI-only disable - internalName is settable/clearable on any group; empty/whitespace-only values normalize to null instead of an empty display name - listForTenant() now projects internalName alongside name - UpdateGroupDto drops ldapDn (D-07: no more codepath binds a local group to AD via this route) and gains internalName?: string | null - 9 new test cases in groups.service.spec.ts (name lock, internalName set/clear/idempotent/local-group/unicode, listForTenant projection); stale ldapDn update() test removed (behavior intentionally deleted)
This commit is contained in:
@@ -1,12 +1,20 @@
|
||||
import { IsBoolean, IsOptional, IsString } from 'class-validator';
|
||||
|
||||
/**
|
||||
* DTO für das Aktualisieren einer Gruppe: Umbenennen, Standardmarkierung
|
||||
* setzen/entfernen (D-13), AD-Bindung setzen/lösen (ldapDn: null löst).
|
||||
* DTO für das Aktualisieren einer Gruppe: Umbenennen (nur bei lokalen
|
||||
* Gruppen — eine importierte Gruppe mit gesetztem ldapObjectGuid lehnt
|
||||
* einen `name`-Wert serverseitig mit BadRequestException ab, D-03),
|
||||
* Standardmarkierung setzen/entfernen (D-13), interner Anzeigename
|
||||
* setzen/löschen (D-04).
|
||||
*
|
||||
* @IsOptional() lässt sowohl undefined als auch null unvalidiert durch —
|
||||
* das ist die Voraussetzung dafür, dass ldapDn: null (Lösen der AD-Bindung)
|
||||
* gültig bleibt, obwohl @IsString() sonst null ablehnen würde.
|
||||
* das ist die Voraussetzung dafür, dass internalName: null (Löschen des
|
||||
* internen Namens) gültig bleibt, obwohl @IsString() sonst null ablehnen
|
||||
* würde.
|
||||
*
|
||||
* Kein AD-Bindungsfeld mehr (D-07): über dieses DTO kann eine lokal
|
||||
* angelegte Gruppe nicht mehr nachträglich an eine AD-Gruppe gebunden und
|
||||
* eine bestehende Bindung nicht mehr gelöst werden.
|
||||
*/
|
||||
export class UpdateGroupDto {
|
||||
@IsOptional()
|
||||
@@ -19,5 +27,5 @@ export class UpdateGroupDto {
|
||||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
ldapDn?: string | null;
|
||||
internalName?: string | null;
|
||||
}
|
||||
|
||||
@@ -108,6 +108,8 @@ function makeFakePrisma() {
|
||||
const record = {
|
||||
id: `g-${groupCounter}`,
|
||||
ldapDn: null,
|
||||
internalName: null,
|
||||
ldapObjectGuid: null,
|
||||
isDefault: false,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
@@ -373,18 +375,121 @@ describe('GroupsService', () => {
|
||||
expect(result.isDefault).toBe(false);
|
||||
});
|
||||
|
||||
it('update() mit ldapDn bindet an eine AD-Gruppe; ldapDn:null löst die Bindung', async () => {
|
||||
// D-07: update() bietet seit diesem Plan absichtlich keinen ldapDn-Parameter
|
||||
// mehr — eine lokal angelegte Gruppe kann nicht mehr nachträglich an eine
|
||||
// AD-Gruppe gebunden werden, und eine bestehende Bindung kann nicht mehr
|
||||
// über diesen Weg gelöst werden. Der frühere Test dafür entfällt ersatzlos.
|
||||
|
||||
// --- update() — Namenssperre und interner Name (D-03/D-04) ---------------
|
||||
|
||||
describe('GroupsService.update — Namenssperre und interner Name (D-03/D-04)', () => {
|
||||
it('lehnt name fuer eine importierte Gruppe (gesetzter ldapObjectGuid) mit BadRequestException ab', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
|
||||
const group = await service.create('t1', { name: 'A' });
|
||||
const bound = await service.update('t1', group.id, {
|
||||
ldapDn: 'CN=A,OU=Groups,DC=ctl,DC=local',
|
||||
const group = await service.create('t1', { name: 'Vertrieb' });
|
||||
await (prisma as any).group.update({
|
||||
where: { id: group.id },
|
||||
data: { ldapObjectGuid: 'abc123' },
|
||||
});
|
||||
expect(bound.ldapDn).toBe('CN=A,OU=Groups,DC=ctl,DC=local');
|
||||
|
||||
const unbound = await service.update('t1', group.id, { ldapDn: null });
|
||||
expect(unbound.ldapDn).toBeNull();
|
||||
await expect(
|
||||
service.update('t1', group.id, { name: 'Umbenannt' }),
|
||||
).rejects.toBeInstanceOf(BadRequestException);
|
||||
const list = await service.listForTenant('t1');
|
||||
expect(list.find((g) => g.id === group.id)!.name).toBe('Vertrieb');
|
||||
});
|
||||
|
||||
it('erlaubt name fuer eine Gruppe ohne gesetzten ldapObjectGuid — unveraendertes Verhalten', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'Alt' });
|
||||
|
||||
const result = await service.update('t1', group.id, { name: 'Neu' });
|
||||
|
||||
expect(result.name).toBe('Neu');
|
||||
});
|
||||
|
||||
it('setzt internalName auf einer importierten Gruppe, name bleibt unangetastet', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'AD-Name' });
|
||||
await (prisma as any).group.update({
|
||||
where: { id: group.id },
|
||||
data: { ldapObjectGuid: 'abc123' },
|
||||
});
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
expect(result.internalName).toBe('Vertrieb');
|
||||
expect(result.name).toBe('AD-Name');
|
||||
});
|
||||
|
||||
it('normalisiert internalName aus reinen Leerzeichen auf null statt auf einen leeren Anzeigenamen', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'A' });
|
||||
await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: ' ' });
|
||||
|
||||
expect(result.internalName).toBeNull();
|
||||
});
|
||||
|
||||
it('setzt internalName:null direkt auf null', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'A' });
|
||||
await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: null });
|
||||
|
||||
expect(result.internalName).toBeNull();
|
||||
});
|
||||
|
||||
it('ein zweites PATCH mit identischem internalName aendert nichts und wirft nicht', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'A' });
|
||||
await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
expect(result.internalName).toBe('Vertrieb');
|
||||
});
|
||||
|
||||
it('erlaubt internalName auf einer lokalen Gruppe (keine Sperre auf diesem Feld)', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'Lokal' });
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: 'Anzeigename' });
|
||||
|
||||
expect(result.internalName).toBe('Anzeigename');
|
||||
});
|
||||
|
||||
it('speichert und liefert Unicode-Namen unveraendert, ueber den getrimmten Inhalt entschieden statt ueber Truthiness', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'A' });
|
||||
|
||||
const result = await service.update('t1', group.id, { internalName: ' Vértriéb 部門 ' });
|
||||
|
||||
expect(result.internalName).toBe('Vértriéb 部門');
|
||||
});
|
||||
|
||||
it('listForTenant() liefert internalName zusaetzlich zu name', async () => {
|
||||
const prisma = makeFakePrisma();
|
||||
const service = new GroupsService(prisma as any);
|
||||
const group = await service.create('t1', { name: 'AD-Name' });
|
||||
await service.update('t1', group.id, { internalName: 'Vertrieb' });
|
||||
|
||||
const list = await service.listForTenant('t1');
|
||||
|
||||
expect(list.find((g) => g.id === group.id)).toMatchObject({
|
||||
name: 'AD-Name',
|
||||
internalName: 'Vertrieb',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// --- getImpact ---------------------------------------------------------
|
||||
|
||||
@@ -45,6 +45,7 @@ export class GroupsService {
|
||||
id: g.id,
|
||||
tenantId: g.tenantId,
|
||||
name: g.name,
|
||||
internalName: g.internalName,
|
||||
ldapDn: g.ldapDn,
|
||||
isDefault: g.isDefault,
|
||||
createdAt: g.createdAt,
|
||||
@@ -95,7 +96,7 @@ export class GroupsService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Aktualisiert Name, Standardmarkierung und/oder AD-Bindung.
|
||||
* Aktualisiert Name, Standardmarkierung und/oder internen Anzeigenamen.
|
||||
*
|
||||
* isDefault:true läuft in einer Transaktion: zuerst updateMany auf alle
|
||||
* Gruppen des Mandanten mit isDefault:false, dann update der Zielgruppe
|
||||
@@ -103,30 +104,52 @@ export class GroupsService {
|
||||
* aus 15-01 ist das Sicherheitsnetz gegen parallele Aufrufe, die
|
||||
* Transaktion ist der normale Pfad. isDefault:false schaltet die
|
||||
* Markierung nur an dieser einen Gruppe ab, ohne sie irgendwo anders zu
|
||||
* setzen. ldapDn:null löst eine AD-Bindung.
|
||||
* setzen.
|
||||
*
|
||||
* Namenssperre (D-03/D-07): trägt die geladene Gruppe einen gesetzten
|
||||
* ldapObjectGuid, ist sie aus dem Verzeichnis importiert — ein `name`
|
||||
* im Request wird dann mit BadRequestException abgelehnt, BEVOR die
|
||||
* Leerstring-Prüfung läuft. Das ist eine Backend-Invariante, kein UI-
|
||||
* Feld-Disable: ein direkter API-Aufruf kommt an ihr nicht vorbei.
|
||||
*
|
||||
* internalName (D-04) läuft unabhängig von dieser Sperre — jede Gruppe,
|
||||
* importiert oder lokal, darf ihn setzen. undefined lässt die Spalte
|
||||
* unangetastet, null oder ein leerer/nur-Leerzeichen-String normalisiert
|
||||
* auf null (nie ein leerer Anzeigename), ein nicht-leerer String wird
|
||||
* getrimmt gespeichert.
|
||||
*/
|
||||
async update(
|
||||
tenantId: string,
|
||||
id: string,
|
||||
data: { name?: string; isDefault?: boolean; ldapDn?: string | null },
|
||||
data: { name?: string; isDefault?: boolean; internalName?: string | null },
|
||||
) {
|
||||
await this.findOwned(tenantId, id);
|
||||
const existing = await this.findOwned(tenantId, id);
|
||||
|
||||
const updateData: {
|
||||
name?: string;
|
||||
ldapDn?: string | null;
|
||||
internalName?: string | null;
|
||||
isDefault?: boolean;
|
||||
} = {};
|
||||
|
||||
if (data.name !== undefined) {
|
||||
if (existing.ldapObjectGuid) {
|
||||
throw new BadRequestException(
|
||||
'Der Name einer aus dem Verzeichnis übernommenen Gruppe wird dort gepflegt und kann hier nicht geändert werden',
|
||||
);
|
||||
}
|
||||
const trimmed = data.name.trim();
|
||||
if (!trimmed) {
|
||||
throw new BadRequestException('Gruppenname darf nicht leer sein');
|
||||
}
|
||||
updateData.name = trimmed;
|
||||
}
|
||||
if (data.ldapDn !== undefined) {
|
||||
updateData.ldapDn = data.ldapDn;
|
||||
if (data.internalName !== undefined) {
|
||||
if (data.internalName === null) {
|
||||
updateData.internalName = null;
|
||||
} else {
|
||||
const trimmed = data.internalName.trim();
|
||||
updateData.internalName = trimmed === '' ? null : trimmed;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user