SPSH-1291: LdapPersonEntries have objectClass posixAccount (#740)

* LdapPersonEntries have objectClass posixAccount and required attributes * add error-logging if LDAP-createLehrer fails * add logging when modifying LDAPEntry (email-address) fails --------- Co-authored-by: Kristoff Kiefer <[email protected]>
dBildungsplattform · Nov 7, 2024 · 6bb3a76 · 6bb3a76
1 parent c6a73b7
commit 6bb3a76
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 31 deletions.
diff --git a/src/core/ldap/domain/ldap-client.service.spec.ts b/src/core/ldap/domain/ldap-client.service.spec.ts
@@ -25,6 +25,8 @@ import { ClassLogger } from '../../logging/class-logger.js';
 import { EventService } from '../../eventbus/services/event.service.js';
 import { LdapEmailDomainError } from '../error/ldap-email-domain.error.js';
 import { LdapEmailAddressError } from '../error/ldap-email-address.error.js';
+import { LdapCreateLehrerError } from '../error/ldap-create-lehrer.error.js';
+import { LdapModifyEmailError } from '../error/ldap-modify-email.error.js';
 
 describe('LDAP Client Service', () => {
     let app: INestApplication;
@@ -236,6 +238,30 @@ describe('LDAP Client Service', () => {
                 expect(loggerMock.info).toHaveBeenLastCalledWith(`LDAP: Successfully created lehrer ${lehrerUid}`);
             });
 
+            it('when adding fails should log error', async () => {
+                ldapClientMock.getClient.mockImplementation(() => {
+                    clientMock.bind.mockResolvedValue();
+                    clientMock.search.mockResolvedValueOnce(createMock<SearchResult>({ searchEntries: [] })); //mock: lehrer not present
+                    clientMock.add.mockRejectedValueOnce(new Error('LDAP-Error'));
+
+                    return clientMock;
+                });
+                const testLehrer: PersonData = {
+                    id: faker.string.uuid(),
+                    vorname: faker.person.firstName(),
+                    familienname: faker.person.lastName(),
+                    referrer: faker.lorem.word(),
+                };
+                const lehrerUid: string = 'uid=' + testLehrer.referrer + ',ou=oeffentlicheSchulen,dc=schule-sh,dc=de';
+                const result: Result<PersonData> = await ldapClientService.createLehrer(testLehrer, fakeEmailDomain);
+
+                if (result.ok) throw Error();
+                expect(loggerMock.error).toHaveBeenLastCalledWith(
+                    `LDAP: Creating lehrer FAILED, uid:${lehrerUid}, errMsg:{}`,
+                );
+                expect(result.error).toEqual(new LdapCreateLehrerError());
+            });
+
             it('when called with explicit domain "ersatzschule-sh.de" should return truthy result', async () => {
                 ldapClientMock.getClient.mockImplementation(() => {
                     clientMock.bind.mockResolvedValue();
@@ -290,9 +316,6 @@ describe('LDAP Client Service', () => {
                 ldapClientMock.getClient.mockImplementation(() => {
                     clientMock.bind.mockResolvedValue();
                     clientMock.add.mockResolvedValueOnce();
-                    clientMock.search.mockResolvedValueOnce(
-                        createMock<SearchResult>({ searchEntries: [createMock<Entry>()] }),
-                    ); //mock existsSchule: schule present
                     clientMock.search.mockResolvedValueOnce(createMock<SearchResult>({ searchEntries: [] })); //mock: lehrer not present
 
                     return clientMock;
@@ -498,6 +521,44 @@ describe('LDAP Client Service', () => {
             });
         });
 
+        describe('when person can be found but modification fails', () => {
+            const fakePersonID: string = faker.string.uuid();
+            const fakeDN: string = faker.string.alpha();
+            const newEmailAddress: string = '[email protected]';
+            const currentEmailAddress: string = '[email protected]';
+
+            it('should set mailAlternativeAddress as current mailPrimaryAddress and throw LdapPersonEntryChangedEvent', async () => {
+                ldapClientMock.getClient.mockImplementation(() => {
+                    clientMock.bind.mockResolvedValueOnce();
+                    clientMock.search.mockResolvedValueOnce(
+                        createMock<SearchResult>({
+                            searchEntries: [
+                                createMock<Entry>({
+                                    dn: fakeDN,
+                                    mailPrimaryAddress: currentEmailAddress,
+                                }),
+                            ],
+                        }),
+                    );
+                    clientMock.modify.mockRejectedValueOnce(new Error());
+
+                    return clientMock;
+                });
+
+                const result: Result<PersonID> = await ldapClientService.changeEmailAddressByPersonId(
+                    fakePersonID,
+                    newEmailAddress,
+                );
+
+                if (result.ok) throw Error();
+                expect(result.error).toStrictEqual(new LdapModifyEmailError());
+                expect(loggerMock.error).toHaveBeenLastCalledWith(
+                    `LDAP: Modifying mailPrimaryAddress and mailAlternativeAddress FAILED, errMsg:{}`,
+                );
+                expect(eventServiceMock.publish).toHaveBeenCalledTimes(0);
+            });
+        });
+
         describe('when person can be found and modified', () => {
             let fakePersonID: string;
             let fakeDN: string;

diff --git a/src/core/ldap/domain/ldap-client.service.ts b/src/core/ldap/domain/ldap-client.service.ts
@@ -12,6 +12,8 @@ import { EventService } from '../../eventbus/services/event.service.js';
 import { LdapPersonEntryChangedEvent } from '../../../shared/events/ldap-person-entry-changed.event.js';
 import { LdapEmailAddressError } from '../error/ldap-email-address.error.js';
 import { LdapEmailDomainError } from '../error/ldap-email-domain.error.js';
+import { LdapCreateLehrerError } from '../error/ldap-create-lehrer.error.js';
+import { LdapModifyEmailError } from '../error/ldap-modify-email.error.js';
 
 export type PersonData = {
     vorname: string;
@@ -37,6 +39,12 @@ export class LdapClientService {
 
     public static readonly DC_SCHULE_SH_DC_DE: string = 'dc=schule-sh,dc=de';
 
+    public static readonly GID_NUMBER: string = '100'; //because 0 to 99 are used for statically allocated user groups on Unix-systems
+
+    public static readonly UID_NUMBER: string = '100'; //to match the GID_NUMBER rule above and 0 is reserved for super-user
+
+    public static readonly HOME_DIRECTORY: string = 'none'; //highlight it's a dummy value
+
     private mutex: Mutex;
 
     public constructor(
@@ -131,9 +139,13 @@ export class LdapClientService {
                 return { ok: true, value: person };
             }
             const entry: LdapPersonEntry = {
+                uid: lehrerUid,
+                uidNumber: LdapClientService.UID_NUMBER,
+                gidNumber: LdapClientService.GID_NUMBER,
+                homeDirectory: LdapClientService.HOME_DIRECTORY,
                 cn: person.vorname,
                 sn: person.familienname,
-                objectclass: ['inetOrgPerson', 'univentionMail'],
+                objectclass: ['inetOrgPerson', 'univentionMail', 'posixAccount'],
                 mailPrimaryAddress: mail ?? ``,
                 mailAlternativeAddress: mail ?? ``,
             };
@@ -143,10 +155,17 @@ export class LdapClientService {
             entry.entryUUID = person.ldapEntryUUID ?? person.id;
             controls.push(new Control(relaxRulesControlOID));
 
-            await client.add(lehrerUid, entry, controls);
-            this.logger.info(`LDAP: Successfully created lehrer ${lehrerUid}`);
+            try {
+                await client.add(lehrerUid, entry, controls);
+                this.logger.info(`LDAP: Successfully created lehrer ${lehrerUid}`);
 
-            return { ok: true, value: person };
+                return { ok: true, value: person };
+            } catch (err) {
+                const errMsg: string = JSON.stringify(err);
+                this.logger.error(`LDAP: Creating lehrer FAILED, uid:${lehrerUid}, errMsg:${errMsg}`);
+
+                return { ok: false, error: new LdapCreateLehrerError() };
+            }
         });
     }
 
@@ -268,31 +287,41 @@ export class LdapClientService {
             }
             const currentEmailAddress: string = currentEmailAddressString ?? newEmailAddress;
 
-            await client.modify(searchResult.searchEntries[0].dn, [
-                new Change({
-                    operation: 'replace',
-                    modification: new Attribute({
-                        type: LdapClientService.MAIL_PRIMARY_ADDRESS,
-                        values: [newEmailAddress],
+            try {
+                await client.modify(searchResult.searchEntries[0].dn, [
+                    new Change({
+                        operation: 'replace',
+                        modification: new Attribute({
+                            type: LdapClientService.MAIL_PRIMARY_ADDRESS,
+                            values: [newEmailAddress],
+                        }),
                     }),
-                }),
-            ]);
-            await client.modify(searchResult.searchEntries[0].dn, [
-                new Change({
-                    operation: 'replace',
-                    modification: new Attribute({
-                        type: LdapClientService.MAIL_ALTERNATIVE_ADDRESS,
-                        values: [currentEmailAddress],
+                ]);
+                await client.modify(searchResult.searchEntries[0].dn, [
+                    new Change({
+                        operation: 'replace',
+                        modification: new Attribute({
+                            type: LdapClientService.MAIL_ALTERNATIVE_ADDRESS,
+                            values: [currentEmailAddress],
+                        }),
                     }),
-                }),
-            ]);
-            this.logger.info(
-                `LDAP: Successfully modified mailPrimaryAddress and mailAlternativeAddress for personId:${personId}`,
-            );
-
-            this.eventService.publish(new LdapPersonEntryChangedEvent(personId, newEmailAddress, currentEmailAddress));
-
-            return { ok: true, value: personId };
+                ]);
+                this.logger.info(
+                    `LDAP: Successfully modified mailPrimaryAddress and mailAlternativeAddress for personId:${personId}`,
+                );
+                this.eventService.publish(
+                    new LdapPersonEntryChangedEvent(personId, newEmailAddress, currentEmailAddress),
+                );
+
+                return { ok: true, value: personId };
+            } catch (err) {
+                const errMsg: string = JSON.stringify(err);
+                this.logger.error(
+                    `LDAP: Modifying mailPrimaryAddress and mailAlternativeAddress FAILED, errMsg:${errMsg}`,
+                );
+
+                return { ok: false, error: new LdapModifyEmailError() };
+            }
         });
     }
 }
diff --git a/src/core/ldap/domain/ldap.types.ts b/src/core/ldap/domain/ldap.types.ts
@@ -1,4 +1,8 @@
 export type LdapPersonEntry = {
+    uid: string; //required for posixAccount objectClass
+    uidNumber: string; //required for posixAccount objectClass
+    gidNumber: string; //required for posixAccount objectClass
+    homeDirectory: string; //required for posixAccount objectClass
     cn: string;
     sn: string;
     mailPrimaryAddress?: string;
@@ -9,6 +13,5 @@ export type LdapPersonEntry = {
 };
 
 export enum LdapEntityType {
-    SCHULE = 'SCHULE',
     LEHRER = 'LEHRER',
 }
diff --git a/src/core/ldap/error/ldap-create-lehrer.error.ts b/src/core/ldap/error/ldap-create-lehrer.error.ts
@@ -0,0 +1,7 @@
+import { DomainError } from '../../../shared/error/index.js';
+
+export class LdapCreateLehrerError extends DomainError {
+    public constructor(details?: unknown[] | Record<string, unknown>) {
+        super(`LDAP error: Creating lehrer FAILED`, 'LDAP_LEHRER_CREATION_FAILED', details);
+    }
+}
diff --git a/src/core/ldap/error/ldap-modify-email.error.ts b/src/core/ldap/error/ldap-modify-email.error.ts
@@ -0,0 +1,11 @@
+import { DomainError } from '../../../shared/error/index.js';
+
+export class LdapModifyEmailError extends DomainError {
+    public constructor(details?: unknown[] | Record<string, unknown>) {
+        super(
+            `LDAP error: Modifying mailPrimaryAddress and mailAlternativeAddress FAILED`,
+            'LDAP_EMAIL_MODIFICATION_FAILED',
+            details,
+        );
+    }
+}